fix(workflow): stop a nested block jumping when it leaves its container - #6644
Conversation
`getNodeAbsolutePosition` added the container's header and padding to a child's position. Those are already in the position: React Flow places a child at its parent's origin plus its own coordinates, and `clampPositionToContainer` is what holds it clear of the chrome, flooring it at `LEFT_PADDING` and `HEADER_HEIGHT + TOP_PADDING`. Counting them twice put every nested node 16px right and 66px below where it actually renders. Visible as a block dropping down-right the moment it is dragged out of a Loop, and as a block landing off-target when dragged into one from the canvas. Also skewed container hit-testing during a drag and the bounds `fitView` focuses on. Two callers already knew: both subtracted the same three constants straight back off to recover a relative position. They now take the difference of two absolutes, which is what a relative position is. A third place, React Flow's child `extent`, had its own copy of the numbers — a fourth distinct header height, 42, against the 40 the card renders — and now reads the same constants as the clamp, so a drag stops where a drop would put it. `positionAbsolute ?? getNodeAbsolutePosition(...)` in the fit-view path can also stop disagreeing with itself: React Flow's own answer carries no offset, so the two branches returned points 66px apart for the same node.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview
Adds Reviewed by Cursor Bugbot for commit 1c7fc31. Configure here. |
Greptile SummaryThe PR aligns nested-node coordinates with React Flow by removing duplicate container chrome offsets and using shared container constants.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported fixture typing issue is fixed by typing the hook and deriving the fixture type from its parameter.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-node-utilities.ts | Removes duplicated header and padding offsets, safely reads optional parent links, and replaces the untyped block map with BlockState. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx | Updates reparenting calculations and drag extents to use one consistent container coordinate convention. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-node-utilities.test.tsx | Adds typed regression coverage for child absolute positions, root positions, and stable container removal. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Child[Child-relative position] --> Absolute[Parent absolute + child position]
Parent[Parent absolute position] --> Absolute
Absolute --> Reparent[Subtract destination parent absolute]
Reparent --> Clamp[Clamp to shared container padding and header]
Clamp --> Render[Stable rendered position]
Reviews (2): Last reviewed commit: "refactor(workflow): type the node-utilit..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
`useNodeUtilities` took `Record<string, any>`, so the test fixtures had to be cast to reach it and nothing in the hook was checked against a real block. Typing it as `Record<string, BlockState>` surfaced an unsafe read straight away: the cycle walk re-read `blocks[currentId].data.parentId` after the `while` condition had tested the same optional chain, on a map where both links are optional. It now reads the value once and breaks on absence, which is what the condition was trying to express. The fixtures follow the hook's own parameter type, so they stay honest without a cast on either side.
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1c7fc31. Configure here.
1 participant
Summary
getNodeAbsolutePositionadded the container's header and padding to a child's position. Those are already in the position — React Flow places a child at its parent's origin plus its own coordinates, andclampPositionToContaineris what holds it clear of the chrome, flooring it atLEFT_PADDINGandHEADER_HEIGHT + TOP_PADDING. Counting them twice reported every nested node 16px right and 66px below where it actually renders.What that looks like:
fitViewfocuses on, are both skewed for nested nodes.Two callers already knew something was wrong: each subtracted the same three constants straight back off to recover a relative position. They now take the difference of two absolutes, which is what a relative position is.
A third place had its own copy — React Flow's child
extent, with a fourth distinct header height (42, against the 40 the card renders and the 50 this helper used). It now reads the same constants as the clamp, so a drag stops where a drop would put it.And
positionAbsolute ?? getNodeAbsolutePosition(...)in the fit-view path stops disagreeing with itself: React Flow's own answer carries no offset, so its two branches returned points 66px apart for the same node.Type of Change
Testing
New unit tests on the hook pinning the convention — a child sits at parent-plus-position, a root node is untouched, and popping a child out round-trips to exactly where it was. Two fail on the old helper, with the double-count visible in the diff:
That is the pop-out case: the block landed 16px right and 66px down instead of staying put.
Blast radius is bounded by the helper's own early return — a node with no parent returns its position unchanged, so nothing about top-level blocks changes. Only nested nodes move, and only onto the position they were already being drawn at.
Full
[workflowId]andlib/workflowssuites green (1756 tests), type-check and lint clean across all 23 packages.Not verified on a running canvas. Worth dragging a block out of a Loop and back in before merging — that is the exact interaction this changes.
Notes
extentchange tightens the drag floor from42toHEADER_HEIGHT + TOP_PADDING. Programmatic placement already used the larger floor, so this makes dragging agree with dropping rather than loosening anything.mcp-dynamic-argsrow estimate, and writing a container's derived size back so the first painted frame is not the stored default.Checklist