Skip to content

Commit 46e7e32

Browse files
committed
fix(canvas): order subflow containers ahead of their children before render
React Flow v12 resolves a child node's absolute position in a single pass over the nodes array, against parents it has already adopted. A child listed before its container has its container-relative offset treated as absolute, so a block nested in a Loop or Parallel jumped out of the container on any click and only returned once a re-measurement corrected it. Blocks arrive in database row order, so a block created before the container it was later dragged into precedes that container. Sort parents first on the array each ReactFlow mount adopts, and reuse the helper for the preview's own sort.
1 parent 915833b commit 46e7e32

5 files changed

Lines changed: 140 additions & 9 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
getEdgeZIndexForTarget,
3434
getNoteBlockHeight,
3535
normalizeCursorSourceHandleId,
36+
sortNodesParentsFirst,
3637
useCanvasColorMode,
3738
} from '@sim/workflow-renderer'
3839
import {
@@ -4830,9 +4831,14 @@ const WorkflowContent = React.memo(
48304831
*
48314832
* Subflow containers are skipped: their depth-based zIndex is what orders
48324833
* them against their own children, and bumping it would break that.
4834+
*
4835+
* Containers are moved ahead of their children first: React Flow v12 places
4836+
* a child that precedes its parent at its parent-relative offset. Sorting
4837+
* here rather than in `displayNodes` covers the in-place patches too, since
4838+
* `nodesForRender` is the only array handed to React Flow.
48334839
*/
48344840
const nodesForRender = useMemo(() => {
4835-
const elevatedNodes = displayNodes.map((node) => {
4841+
const elevatedNodes = sortNodesParentsFirst(displayNodes).map((node) => {
48364842
if (node.type === 'subflowNode') return node
48374843
const target = getBlockZIndex(node.zIndex ?? BLOCK_Z_BASE, {
48384844
isSelected: node.selected,

apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/preview-workflow.tsx

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
EDGE_Z_BASE,
2626
EDGE_Z_MAX,
2727
getEdgeZIndexForTarget,
28+
sortNodesParentsFirst,
2829
useCanvasColorMode,
2930
} from '@sim/workflow-renderer'
3031
import { normalizeWorkflowEdgeHandles } from '@sim/workflow-types/workflow'
@@ -402,13 +403,7 @@ export function PreviewWorkflow({
402403
const nodeArray: Node[] = []
403404
const blocksWithErrorEdge = new Set(errorSourceBlockKey ? errorSourceBlockKey.split(',') : [])
404405

405-
const sortedBlocks = Object.entries(workflowState.blocks || {}).sort(
406-
([, left], [, right]) =>
407-
calculateNestingDepth(left, workflowState.blocks) -
408-
calculateNestingDepth(right, workflowState.blocks)
409-
)
410-
411-
sortedBlocks.forEach(([blockId, block]) => {
406+
Object.entries(workflowState.blocks || {}).forEach(([blockId, block]) => {
412407
if (!block || !block.type) {
413408
logger.warn(`Skipping invalid block: ${blockId}`)
414409
return
@@ -501,7 +496,7 @@ export function PreviewWorkflow({
501496
})
502497
})
503498

504-
return nodeArray
499+
return sortNodesParentsFirst(nodeArray)
505500
}, [
506501
blocksStructure,
507502
loopsStructure,

packages/workflow-renderer/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export {
1717
type WorkflowEdgeViewProps,
1818
} from './edge/workflow-edge-view'
1919
export { humanizeBlockName } from './lib/humanize-block-name'
20+
export { sortNodesParentsFirst } from './node-order'
2021
export {
2122
NOTE_MARKDOWN_FLOW,
2223
NoteBlockView,
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { sortNodesParentsFirst } from './node-order'
6+
7+
const node = (id: string, parentId?: string) => (parentId ? { id, parentId } : { id })
8+
const ids = (nodes: Array<{ id: string }>) => nodes.map((n) => n.id)
9+
10+
describe('sortNodesParentsFirst', () => {
11+
it('moves a child that precedes its container behind it', () => {
12+
/* The reported bug: a card created before the loop it was later dragged
13+
into sits ahead of the loop in row order, so React Flow v12 placed it at
14+
its loop-relative offset on every click. */
15+
const nodes = [node('start'), node('earlier'), node('sink', 'loop'), node('loop')]
16+
17+
expect(ids(sortNodesParentsFirst(nodes))).toEqual(['start', 'earlier', 'loop', 'sink'])
18+
})
19+
20+
it('returns the same array when every parent already precedes its children', () => {
21+
const nodes = [node('start'), node('loop'), node('sink', 'loop'), node('later')]
22+
23+
expect(sortNodesParentsFirst(nodes)).toBe(nodes)
24+
})
25+
26+
it('leaves a parents-first array alone even when depth drops between siblings', () => {
27+
const nodes = [node('loopA'), node('a1', 'loopA'), node('loopB'), node('b1', 'loopB')]
28+
29+
expect(sortNodesParentsFirst(nodes)).toBe(nodes)
30+
})
31+
32+
it('orders every level of a nested chain and keeps siblings in their original order', () => {
33+
const nodes = [
34+
node('grandchild', 'inner'),
35+
node('inner', 'outer'),
36+
node('second', 'outer'),
37+
node('first', 'outer'),
38+
node('outer'),
39+
node('top'),
40+
]
41+
42+
expect(ids(sortNodesParentsFirst(nodes))).toEqual([
43+
'outer',
44+
'top',
45+
'inner',
46+
'second',
47+
'first',
48+
'grandchild',
49+
])
50+
})
51+
52+
it('does not move a child whose parent is not in the array', () => {
53+
const nodes = [node('orphan', 'missing'), node('top')]
54+
55+
expect(sortNodesParentsFirst(nodes)).toBe(nodes)
56+
})
57+
58+
it('terminates on a parent cycle', () => {
59+
const nodes = [node('b', 'a'), node('a', 'b'), node('c', 'a')]
60+
61+
expect(ids(sortNodesParentsFirst(nodes))).toHaveLength(3)
62+
})
63+
64+
it('does not mutate its input when it has to reorder', () => {
65+
const nodes = [node('sink', 'loop'), node('loop')]
66+
const before = [...nodes]
67+
68+
sortNodesParentsFirst(nodes)
69+
70+
expect(nodes).toEqual(before)
71+
})
72+
})
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
interface OrderableNode {
2+
id: string
3+
parentId?: string
4+
}
5+
6+
/**
7+
* Returns the nodes with every container ahead of its descendants, otherwise
8+
* in their original order. An array that already satisfies that is returned
9+
* as-is, and a reorder sorts a copy — the caller's array is never sorted in
10+
* place, which matters because the editor hands React state straight in.
11+
*
12+
* React Flow v12 adopts nodes in one pass over the array and resolves a child's
13+
* absolute position against the parent it has *already* adopted; a child that
14+
* precedes its parent is placed at its parent-relative offset as if that were
15+
* absolute (and logged as "Parent node not found"). Adoption reruns whenever a
16+
* node object changes identity — every click rebuilds them — and the misplaced
17+
* card is only corrected by the next measurement pass, so it sits over the
18+
* top-left of the canvas until something resizes it. v11 resolved positions in
19+
* a second pass and never cared about order. The editor's block record is in
20+
* database row order, which puts a block ahead of a container it was later
21+
* dragged into.
22+
*/
23+
export function sortNodesParentsFirst<T extends OrderableNode>(nodes: T[]): T[] {
24+
const indexById = new Map<string, number>()
25+
for (let index = 0; index < nodes.length; index++) {
26+
indexById.set(nodes[index].id, index)
27+
}
28+
29+
let ordered = true
30+
for (let index = 0; index < nodes.length && ordered; index++) {
31+
const parentId = nodes[index].parentId
32+
if (!parentId) continue
33+
const parentIndex = indexById.get(parentId)
34+
if (parentIndex !== undefined && parentIndex > index) ordered = false
35+
}
36+
if (ordered) return nodes
37+
38+
const depthById = new Map<string, number>()
39+
const depthOf = (node: T): number => {
40+
const cached = depthById.get(node.id)
41+
if (cached !== undefined) return cached
42+
let depth = 0
43+
const visited = new Set<string>([node.id])
44+
let parentId = node.parentId
45+
while (parentId && !visited.has(parentId)) {
46+
const parentIndex = indexById.get(parentId)
47+
if (parentIndex === undefined) break
48+
visited.add(parentId)
49+
depth++
50+
parentId = nodes[parentIndex].parentId
51+
}
52+
depthById.set(node.id, depth)
53+
return depth
54+
}
55+
56+
return [...nodes].sort((a, b) => depthOf(a) - depthOf(b))
57+
}

0 commit comments

Comments
 (0)