Repository navigation
fix(ui): fit the view after React Flow renders the new layout - #523
Merged
Merged
Conversation
Both fitView calls ran synchronously after the store update that replaces the nodes (setLayout / setFlow). React Flow had not rendered or measured the new nodes yet, so the fit was computed against the previous view. Both are now deferred to the next animation frame. Measured in Chromium (Playwright) on the bundled graph.json, 3 runs each: - clicking a node into flow view: main fits 2 of 95 nodes, zoomed to the 2x cap on the clicked node; with the fix all 95 are in view. - initial load: all 94 nodes in view either way, but the fix fits them tighter (scale 0.197 -> 0.257), the old fit having included stale bounds. No console errors in either. useFlowNavigation.test.ts pinned the synchronous call; it now asserts that fitView is not called in the same tick and is called, with the same options, once the frame runs. It fails against the old hook. Found by gemini-code-assist on #522, where it was out of scope for a formatting-only change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uv6LUrB5HCQcqBvmcixYn3
|
Contributor
|
Warning Gemini is experiencing higher than usual traffic and was unable to create the review. Please try again in a few hours by commenting |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Found by gemini-code-assist on #522, where it was out of scope for a formatting-only PR.
Problem
useLayoutComputationanduseFlowNavigationcallfitViewsynchronously, right after the store update that replaces the nodes (setLayout/setFlow). React Flow has not yet rendered or measured the new nodes, so the fit is computed against the previous view.The visible symptom: clicking a node to open its flow view leaves the viewport zoomed to the 2× cap on the clicked node, with almost everything else off-screen.
Fix
Both calls are deferred to the next animation frame:
Measured in the browser
Chromium via Playwright, against
vite devserving the bundledpublic/graph.jsonat a 1280×800 viewport. The script counts the nodes whose bounding box lies fully inside the React Flow pane, 1.2 s after each step (fitViewanimates for 250 ms). Three runs each, and the results were identical across runs:mainmain→ flow viewThe initial load was already fully visible on
main, but the old fit was looser. It had been computed against stale bounds, too.Test changed, deliberately
useFlowNavigation.test.tspinned the synchronous call. It now asserts the fixed contract:fitViewis not called in the same tick assetFlow;The test fails against the old hook.
Test plan
bun run format:check,lint,typecheck,test:run(119 passed),buildall pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01Uv6LUrB5HCQcqBvmcixYn3
Generated by Claude Code