Skip to content

Commit f069838

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(workflow): hide idle nested subflow end handles
1 parent 6a5e250 commit f069838

13 files changed

Lines changed: 323 additions & 19 deletions

File tree

apps/docs/components/workflow-preview/docs-container-node.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ interface DocsContainerData {
88
name: string
99
blockType: string
1010
size?: { width: number; height: number }
11+
parentId?: string
1112
}
1213

1314
/**
@@ -24,6 +25,7 @@ export const DocsContainerNode = memo(function DocsContainerNode({
2425
name: data.name,
2526
width: data.size?.width,
2627
height: data.size?.height,
28+
parentId: data.parentId,
2729
isPreview: true,
2830
}
2931

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { BLOCK_Z_BASE, CONTAINER_CHILD_Z_BASE, getEdgeZIndex } from '@sim/workflow-renderer'
5+
import { describe, expect, it } from 'vitest'
6+
import { type PreviewBlock, type PreviewWorkflow, toReactFlowElements } from './workflow-data'
7+
8+
const block = (
9+
overrides: Partial<PreviewBlock> & Pick<PreviewBlock, 'id' | 'type'>
10+
): PreviewBlock => ({
11+
name: overrides.id,
12+
bgColor: '#000000',
13+
rows: [],
14+
position: { x: 0, y: 0 },
15+
...overrides,
16+
})
17+
18+
const workflow: PreviewWorkflow = {
19+
id: 'nested-subflows',
20+
name: 'Nested subflows',
21+
blocks: [
22+
block({ id: 'start', type: 'starter' }),
23+
block({ id: 'loop', type: 'loop', size: { width: 500, height: 300 } }),
24+
block({
25+
id: 'parallel',
26+
type: 'parallel',
27+
parentId: 'loop',
28+
position: { x: 24, y: 64 },
29+
size: { width: 400, height: 200 },
30+
}),
31+
block({ id: 'agent', type: 'agent', parentId: 'loop', position: { x: 24, y: 140 } }),
32+
],
33+
edges: [
34+
{ id: 'start-loop', source: 'start', target: 'loop' },
35+
{ id: 'loop-parallel', source: 'loop', target: 'parallel' },
36+
{ id: 'loop-agent', source: 'loop', target: 'agent' },
37+
],
38+
}
39+
40+
describe('toReactFlowElements layering', () => {
41+
it('places incoming edges on their container target layer', () => {
42+
const { nodes, edges } = toReactFlowElements(workflow, false, {
43+
highlightEdge: 'loop-parallel',
44+
})
45+
const nodeById = new Map(nodes.map((node) => [node.id, node]))
46+
const edgeById = new Map(edges.map((edge) => [edge.id, edge]))
47+
48+
expect(nodeById.get('loop')?.zIndex).toBe(0)
49+
expect(nodeById.get('parallel')?.zIndex).toBe(1)
50+
expect(edgeById.get('start-loop')?.zIndex).toBe(0)
51+
expect(edgeById.get('loop-parallel')?.zIndex).toBe(1)
52+
})
53+
54+
it('keeps ordinary cards above normally layered edges', () => {
55+
const { nodes, edges } = toReactFlowElements(workflow)
56+
const nodeById = new Map(nodes.map((node) => [node.id, node]))
57+
const edgeById = new Map(edges.map((edge) => [edge.id, edge]))
58+
59+
expect(nodeById.get('start')?.zIndex).toBe(BLOCK_Z_BASE)
60+
expect(nodeById.get('agent')?.zIndex).toBe(CONTAINER_CHILD_Z_BASE)
61+
expect(edgeById.get('loop-agent')?.zIndex).toBe(getEdgeZIndex(0))
62+
})
63+
})

apps/docs/components/workflow-preview/workflow-data.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
import {
2+
BLOCK_Z_BASE,
3+
CONTAINER_CHILD_Z_BASE,
4+
getEdgeZIndex,
5+
getEdgeZIndexForTarget,
6+
} from '@sim/workflow-renderer'
17
import { type Edge, type Node, Position } from 'reactflow'
28

39
/**
@@ -61,6 +67,24 @@ export interface HighlightOptions {
6167
selectedBlock?: string
6268
}
6369

70+
/** Semantic container depth used for z-order while docs positions stay flattened. */
71+
function getNestingDepth(block: PreviewBlock, blocksById: Map<string, PreviewBlock>): number {
72+
let depth = 0
73+
let parentId = block.parentId
74+
const visited = new Set<string>()
75+
76+
while (parentId && !visited.has(parentId)) {
77+
const parent = blocksById.get(parentId)
78+
if (!parent) break
79+
80+
visited.add(parentId)
81+
depth += 1
82+
parentId = parent.parentId
83+
}
84+
85+
return depth
86+
}
87+
6488
/**
6589
* Converts a {@link PreviewWorkflow} to React Flow nodes and edges.
6690
*
@@ -81,6 +105,7 @@ export function toReactFlowElements(
81105

82106
const nodes: Node[] = workflow.blocks.map((block, index) => {
83107
const isContainer = Boolean(block.size)
108+
const nestingDepth = getNestingDepth(block, blocksById)
84109
// Nested blocks are authored relative to their container; render them at
85110
// absolute coordinates (not React Flow parentNode children) so the edges
86111
// between a container and its nested blocks render reliably and on top.
@@ -92,7 +117,7 @@ export function toReactFlowElements(
92117
id: block.id,
93118
type: isContainer ? 'previewContainer' : 'previewBlock',
94119
position,
95-
zIndex: isContainer ? 0 : 1,
120+
zIndex: isContainer ? nestingDepth : block.parentId ? CONTAINER_CHILD_Z_BASE : BLOCK_Z_BASE,
96121
...(block.size ? { style: { width: block.size.width, height: block.size.height } } : {}),
97122
data: {
98123
name: block.name,
@@ -103,6 +128,7 @@ export function toReactFlowElements(
103128
tools: block.tools,
104129
hideTargetHandle: block.hideTargetHandle,
105130
size: block.size,
131+
parentId: block.parentId,
106132
index,
107133
animate,
108134
isHighlighted: highlightBlock === block.id || selectedBlock === block.id,
@@ -127,6 +153,14 @@ export function toReactFlowElements(
127153
// so edges into and out of Loop/Parallel containers still connect.
128154
const sourceBlock = blocksById.get(e.source)
129155
const targetBlock = blocksById.get(e.target)
156+
const parentContainer = blocksById.get(sourceBlock?.parentId ?? targetBlock?.parentId ?? '')
157+
const baseZIndex = getEdgeZIndex(
158+
parentContainer ? getNestingDepth(parentContainer, blocksById) : undefined,
159+
{ isHighlighted: isEdgeHighlight }
160+
)
161+
const targetContainerZIndex = targetBlock?.size
162+
? getNestingDepth(targetBlock, blocksById)
163+
: undefined
130164
const sourceHandle =
131165
e.sourceHandle ?? (sourceBlock?.size ? `${sourceBlock.type}-end-source` : 'source')
132166
const targetHandle = targetBlock?.size ? undefined : 'target'
@@ -142,6 +176,7 @@ export function toReactFlowElements(
142176
},
143177
sourceHandle,
144178
targetHandle,
179+
zIndex: getEdgeZIndexForTarget(baseZIndex, targetContainerZIndex),
145180
data: {
146181
animate,
147182
delay: animate ? sourceIndex * BLOCK_STAGGER + BLOCK_STAGGER : 0,

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
EDGE_Z_MAX,
2929
getBlockZIndex,
3030
getEdgeZIndex,
31+
getEdgeZIndexForTarget,
3132
getNoteBlockHeight,
3233
normalizeCursorSourceHandleId,
3334
} from '@sim/workflow-renderer'
@@ -4886,10 +4887,16 @@ const WorkflowContent = React.memo(
48864887
isEdgeSelected: isSelected,
48874888
}),
48884889
})
4890+
const targetContainerZIndex =
4891+
targetNode?.type === 'subflowNode' ? (targetNode.zIndex ?? 0) : undefined
4892+
// The target node paints after an equal-z edge. A nested container is
4893+
// one depth above its parent, so this hides only the segment beneath
4894+
// the target while leaving the route visible over the parent body.
4895+
const zIndex = getEdgeZIndexForTarget(baseZIndex, targetContainerZIndex)
48894896

48904897
return {
48914898
...edge,
4892-
zIndex: baseZIndex,
4899+
zIndex,
48934900
data: {
48944901
...edge.data,
48954902
isSelected,
@@ -4898,6 +4905,7 @@ const WorkflowContent = React.memo(
48984905
parentLoopId,
48994906
sourceHandle: edge.sourceHandle,
49004907
onDelete: handleEdgeDelete,
4908+
...(targetContainerZIndex !== undefined ? { labelZIndex: zIndex } : {}),
49014909
},
49024910
}
49034911
})

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ interface WorkflowPreviewSubflowData {
1212
width?: number
1313
height?: number
1414
kind: 'loop' | 'parallel'
15+
parentId?: string
1516
/** Whether this subflow is enabled */
1617
enabled?: boolean
1718
/** Whether this subflow is selected in preview mode */

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
CONTAINER_DIMENSIONS,
2323
EDGE_Z_BASE,
2424
EDGE_Z_MAX,
25+
getEdgeZIndexForTarget,
2526
} from '@sim/workflow-renderer'
2627
import { normalizeWorkflowEdgeHandles } from '@sim/workflow-types/workflow'
2728
import { WorkflowEdge } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-edge/workflow-edge'
@@ -567,6 +568,14 @@ export function PreviewWorkflow({
567568
return normalizeWorkflowEdgeHandles(workflowState.edges).map((edge) => {
568569
const status = getEdgeExecutionStatus(edge)
569570
const isErrorEdge = edge.sourceHandle === 'error'
571+
const baseZIndex =
572+
status === 'success' ? EDGE_Z_MAX : isErrorEdge ? EDGE_Z_BASE + 2 : EDGE_Z_BASE
573+
const targetBlock = workflowState.blocks[edge.target]
574+
const targetContainerZIndex =
575+
targetBlock?.type === 'loop' || targetBlock?.type === 'parallel'
576+
? calculateNestingDepth(targetBlock, workflowState.blocks)
577+
: undefined
578+
570579
return {
571580
id: edge.id,
572581
source: edge.source,
@@ -580,12 +589,14 @@ export function PreviewWorkflow({
580589
/* Inside the shared edge band, so a line clears the opaque container it
581590
crosses and still passes behind cards. Execution status orders edges
582591
within the band: a successful path draws over an error one, which
583-
draws over an unexecuted one. */
584-
zIndex: status === 'success' ? EDGE_Z_MAX : isErrorEdge ? EDGE_Z_BASE + 2 : EDGE_Z_BASE,
592+
draws over an unexecuted one. A Loop/Parallel target overrides that
593+
ordering so its node paints over the incoming segment. */
594+
zIndex: getEdgeZIndexForTarget(baseZIndex, targetContainerZIndex),
585595
}
586596
})
587597
}, [
588598
edgesStructure,
599+
workflowState.blocks,
589600
workflowState.edges,
590601
isValidWorkflowState,
591602
blockExecutionMap,

packages/workflow-renderer/src/canvas-layers.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
EDGE_Z_MAX,
1010
getBlockZIndex,
1111
getEdgeZIndex,
12+
getEdgeZIndexForTarget,
1213
} from './canvas-layers'
1314

1415
/**
@@ -69,3 +70,36 @@ describe('getEdgeZIndex', () => {
6970
expect(getEdgeZIndex(8)).toBeLessThan(getEdgeZIndex(undefined, { isHighlighted: true }))
7071
})
7172
})
73+
74+
describe('getEdgeZIndexForTarget', () => {
75+
it('shares a container target layer so the node paints over the incoming edge', () => {
76+
const parentZIndex = 0
77+
const targetZIndex = 1
78+
const edgeZIndex = getEdgeZIndex(parentZIndex)
79+
80+
const resolved = getEdgeZIndexForTarget(edgeZIndex, targetZIndex)
81+
82+
expect(resolved).toBe(targetZIndex)
83+
expect(resolved).toBeGreaterThan(parentZIndex)
84+
})
85+
86+
it('places incoming edges beneath top-level container targets', () => {
87+
expect(getEdgeZIndexForTarget(EDGE_Z_BASE, 0)).toBe(0)
88+
})
89+
90+
it('does not let highlighting elevate an edge over its container target', () => {
91+
const highlighted = getEdgeZIndex(undefined, { isHighlighted: true })
92+
93+
expect(getEdgeZIndexForTarget(highlighted, 2)).toBe(2)
94+
})
95+
96+
it('does not let an execution edge elevate over its container target', () => {
97+
expect(getEdgeZIndexForTarget(EDGE_Z_MAX, 2)).toBe(2)
98+
})
99+
100+
it('leaves edges to ordinary blocks unchanged', () => {
101+
const edgeZIndex = getEdgeZIndex(1)
102+
103+
expect(getEdgeZIndexForTarget(edgeZIndex, undefined)).toBe(edgeZIndex)
104+
})
105+
})

packages/workflow-renderer/src/canvas-layers.ts

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@
99
* - {@link CONTAINER_CHILD_Z_BASE} — cards inside a container (same +1 / +10 steps)
1010
* - {@link CONNECTION_PICKER_Z} — the connection block picker
1111
*
12-
* Containers and edges must occupy separate bands. A container paints an opaque
13-
* body, so an edge sharing its z loses the equal-z tiebreak to DOM order — React
14-
* Flow renders the nodes layer after the edges layer — and is drawn *behind* the
15-
* container. That is what hid every line crossing a top-level subflow, whether
16-
* in flight or persisted. Cards then sit above the edge band, so a line still
17-
* passes behind card chrome, knobs, and the action-bar swell.
12+
* Containers and ordinary edges occupy separate bands. A container paints an
13+
* opaque body, so an edge sharing its z loses the equal-z tiebreak to DOM order
14+
* — React Flow renders the nodes layer after the edges layer — and is drawn
15+
* *behind* the container. Incoming container edges deliberately use that rule
16+
* for their target only: the target sits above its parent by one depth, leaving
17+
* the edge over the parent body but beneath the target. Cards then sit above the
18+
* edge band, so every other line still passes behind card chrome, knobs, and the
19+
* action-bar swell.
1820
*
1921
* Shared by the editor canvas and the read-only preview because both render the
2022
* same graph through the same React Flow layering rules; a second scale drifted
@@ -74,3 +76,22 @@ export function getEdgeZIndex(
7476
const depth = containerZIndex === undefined ? 0 : containerZIndex + 1
7577
return Math.min(EDGE_Z_BASE + depth, EDGE_Z_DEPTH_MAX)
7678
}
79+
80+
/**
81+
* Keeps an incoming edge beneath a Loop/Parallel target without hiding it
82+
* behind that target's parent.
83+
*
84+
* Containers use their nesting depth as z-index, so a nested target is exactly
85+
* one layer above its parent. React Flow renders equal-z edges before nodes;
86+
* sharing the target's layer therefore leaves the edge visible over the parent
87+
* body while the target paints over the segment that reaches beneath it.
88+
*
89+
* `targetContainerZIndex` must only be supplied when the edge targets a
90+
* container. Ordinary edges retain their existing depth/highlight ordering.
91+
*/
92+
export function getEdgeZIndexForTarget(
93+
edgeZIndex: number,
94+
targetContainerZIndex: number | undefined
95+
): number {
96+
return targetContainerZIndex ?? edgeZIndex
97+
}

packages/workflow-renderer/src/edge/workflow-edge-view-mount.test.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
11
/**
22
* @vitest-environment jsdom
33
*/
4-
import { act } from 'react'
4+
import { act, type ReactNode } from 'react'
55
import { createRoot, type Root } from 'react-dom/client'
66
import { Position } from 'reactflow'
7-
import { afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'
7+
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
88
import { WorkflowEdgeView, type WorkflowEdgeViewProps } from '../index'
99

10+
vi.mock('reactflow', async (importOriginal) => {
11+
const actual = await importOriginal<typeof import('reactflow')>()
12+
return {
13+
...actual,
14+
EdgeLabelRenderer: ({ children }: { children: ReactNode }) => <>{children}</>,
15+
}
16+
})
17+
1018
const mountedHosts = new Set<HTMLDivElement>()
1119
const mountedRoots = new Set<Root>()
1220

@@ -210,4 +218,12 @@ describe('WorkflowEdgeView', () => {
210218

211219
expect(path?.style.stroke).toBe('var(--text-error)')
212220
})
221+
222+
it('keeps the selected-edge control on a container target occlusion layer', () => {
223+
const { host } = renderEdge({
224+
data: { isSelected: true, labelZIndex: 1 },
225+
})
226+
227+
expect(host.querySelector('button')).toHaveStyle({ zIndex: 1 })
228+
})
213229
})

packages/workflow-renderer/src/edge/workflow-edge-view.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type { EdgeDiffStatus, EdgeRunStatus } from '../types'
77
const EXECUTION_PULSE_LENGTH = 0.32
88
const EXECUTION_PULSE_CYCLE_LENGTH = 2.2
99
const EXECUTION_PULSE_DURATION = '1100ms'
10+
const DEFAULT_EDGE_LABEL_Z_INDEX = 1011
1011

1112
/**
1213
* How far the glow reaches past the path, in user space.
@@ -125,6 +126,8 @@ export function WorkflowEdgeView({
125126
}, [isHorizontal, sourceX, sourceY, targetX, targetY])
126127

127128
const isSelected = data?.isSelected ?? false
129+
const labelZIndex =
130+
(data as { labelZIndex?: number } | undefined)?.labelZIndex ?? DEFAULT_EDGE_LABEL_Z_INDEX
128131

129132
const dataSourceHandle = (data as { sourceHandle?: string } | undefined)?.sourceHandle
130133
const isErrorEdge = (sourceHandle ?? dataSourceHandle) === 'error'
@@ -268,7 +271,7 @@ export function WorkflowEdgeView({
268271
position: 'absolute',
269272
transform: `translate(-50%, -50%) translate(${labelX}px,${labelY}px)`,
270273
pointerEvents: 'all',
271-
zIndex: 1011,
274+
zIndex: labelZIndex,
272275
}}
273276
onClick={(e) => {
274277
e.preventDefault()

0 commit comments

Comments
 (0)