Repository navigation
fix(ui): keep the graph zoom when expanding or collapsing groups - #1282
mkceichelbeck merged 10 commits into
Conversation
 [Medium impact] The PR appears safe to merge; no new actionable issues were found. SummaryExpanding or collapsing a group keeps its screen position and zoom. Fit requests made during expansion use the completed graph’s drawn sizes.
Reviews (6) · Last reviewed commit: "test(ui): await the requested viewport b..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
mkceichelbeck
left a comment
mkceichelbeck
left a comment
There was a problem hiding this comment.
Thanks for working on this! I checked it in Chromium and found a few things to address before merging:
- Expanding centres the group, but #1281 asks to keep it in the same screen position. The new test currently expects centring too.
- Placement sometimes reads temporary React Flow geometry. I reproduced collapse drift in Profiles and expanded groups ending partly offscreen on narrow views. Please use the completed layout coordinates, or wait for that exact layout to reach the store; another fixed frame delay would leave the race.
- Clicking Fit while an expansion is laying out gets overridden when the layout completes. Clearing the pending toggle on Fit and consuming it inside the guarded callback passed a base/head regression probe.
Using the same anchoring path for expand and collapse should also let us drop the centring-specific bounds traversal. Please add regressions for these cases and remove the generated-by footer from the PR description.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The anchoring added for group toggles took WorkflowCanvas to 343 lines, over the 300-line function limit the Opcore source check enforces. The collapse, expand and collapse-all handlers and the view placement after a toggle now live in a useGroupToggles hook beside useCanvasView. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only the viewport helpers were tested, not the wiring that places the view once a toggle's layout is drawn. These tests mount the read-only run graph in jsdom with ELK running in process. They check that Expand centres the group and Collapse keeps its card at the same screen spot, both at the saved zoom, and that Collapse all clicked while an Expand is still laying out ends in a whole-graph fit. The ResizeObserver stand-in reports an element again when its inline size changes, so React Flow measures nodes as a browser would and its fitView runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Expanding a group now keeps it where it was on screen, as the-open-engine#1281 asks, instead of centring it. Expand and collapse share one anchoring path, so the member-bounds traversal goes. The anchor and the group's new position are read from the nodes the canvas draws, in an effect on the layout state, rather than from React Flow's store two frames after the layout. The store learns a layout only after a render, so that read could still see temporary positions: collapse drift in Profiles, and expanded groups landing partly offscreen on narrow views. Fit while a toggle is laying out marks the pending toggle, which then fits the finished layout instead of overriding the fit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cd32834 to
cb960cd
Compare
|
Hey Michael, I've made the following edits to reflect #1281 Addressed in cb960cd:
|
Calculate a pending Fit from the completed canvas rectangles so stale React Flow measurements cannot clip expanded groups. Cover four- and six-agent parallel profiles in wide and narrow canvases while ResizeObserver reports are delayed.
mkceichelbeck
left a comment
mkceichelbeck
left a comment
There was a problem hiding this comment.
The feedback is addressed. Verified the built UI inside zero-cloud's Profiles and run-history frames with fixture-backed workspace APIs, including narrow panes, nested group toggles, and Fit requested while ELK layout is pending. The four- and six-agent clipping cases now fit the completed rectangles, including while measurements remain stale. All 412 UI tests, the UI build, and the current head's CI checks pass.
Wait for saved-view restoration and completed-layout fitting before treating a quiet canvas as settled. Cover delayed animation frames explicitly.
mkceichelbeck
left a comment
mkceichelbeck
left a comment
There was a problem hiding this comment.
Verified the viewport fix and added regressions. All 413 UI tests pass, including stale-measurement fitting at both pane widths and delayed viewport restoration. All ten Chromium scenarios pass in zero-cloud’s local embedded Profiles and run-history UI, using the built PR assets and fixture-backed APIs, including nested group toggles, a narrow inspector pane, and Fit during a pending layout. Required CI checks pass for this commit.
Summary
Expanding or collapsing a workflow group preserves its screen position and zoom. Clicking Fit while a group is laying out fits the completed graph from its rendered positions and sizes, so stale React Flow measurements cannot clip the expanded group.
Fixes #1281
Validation
npm --prefix ui test: 413 passing. New regressions cover four- and six-agent parallel profiles in 1106 px and 722 px canvases, with ResizeObserver measurements held stale through the completed layout. A delayed-frame case also verifies saved-view restoration and Collapse all, with explicit waits for the expected viewport.npm --prefix ui run build.60937562, using the built PR assets and fixture-backed workspace APIs: six Profiles/run-history scenarios at wide and narrow widths, including nested toggles and an open inspector; four native-validated parallel-profile clipping cases. All ten pass, including Fit clicked while ELK layout is held pending.