Skip to content

Commit af79fcb

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

11 files changed

Lines changed: 333 additions & 157 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

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ export function toReactFlowElements(
103103
tools: block.tools,
104104
hideTargetHandle: block.hideTargetHandle,
105105
size: block.size,
106+
parentId: block.parentId,
106107
index,
107108
animate,
108109
isHighlighted: highlightBlock === block.id || selectedBlock === block.id,

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-node-utilities.test.tsx

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
import { act } from 'react'
6-
import { CONTAINER_DIMENSIONS } from '@sim/workflow-renderer'
6+
import { CONTAINER_CHILD_MIN_POSITION } from '@sim/workflow-renderer'
77
import { createRoot } from 'react-dom/client'
88
import { beforeEach, describe, expect, it, vi } from 'vitest'
99

@@ -38,10 +38,7 @@ function renderNodeUtilities(blockMap: Parameters<typeof useNodeUtilities>[0]) {
3838
* body — exactly where `clampPositionToContainer` floors a child.
3939
*/
4040
const CONTAINER_POSITION = { x: 1000, y: 500 }
41-
const CHILD_POSITION = {
42-
x: CONTAINER_DIMENSIONS.LEFT_PADDING,
43-
y: CONTAINER_DIMENSIONS.HEADER_HEIGHT + CONTAINER_DIMENSIONS.TOP_PADDING,
44-
}
41+
const CHILD_POSITION = CONTAINER_CHILD_MIN_POSITION
4542

4643
const blocks: Parameters<typeof useNodeUtilities>[0] = {
4744
loop: { id: 'loop', type: 'loop', position: CONTAINER_POSITION, data: {} },
@@ -93,3 +90,33 @@ describe('getNodeAbsolutePosition', () => {
9390
expect({ x: absolute.x - container.x, y: absolute.y - container.y }).toEqual(CHILD_POSITION)
9491
})
9592
})
93+
94+
describe('updateNodeParent', () => {
95+
beforeEach(() => {
96+
vi.clearAllMocks()
97+
mockGetNodes.mockReturnValue(nodes)
98+
})
99+
100+
it('uses one prepared position for display and parent persistence updates', () => {
101+
const api = renderNodeUtilities(blocks)
102+
const position = CONTAINER_CHILD_MIN_POSITION
103+
const batchUpdatePositions = vi.fn()
104+
const batchUpdateBlocksWithParent = vi.fn()
105+
const resize = vi.fn()
106+
107+
api.updateNodeParent(
108+
'root',
109+
'loop',
110+
position,
111+
batchUpdatePositions,
112+
batchUpdateBlocksWithParent,
113+
resize
114+
)
115+
116+
expect(batchUpdatePositions).toHaveBeenCalledWith([{ id: 'root', position }])
117+
expect(batchUpdateBlocksWithParent).toHaveBeenCalledWith([
118+
{ id: 'root', position, parentId: 'loop' },
119+
])
120+
expect(resize).toHaveBeenCalledOnce()
121+
})
122+
})

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-node-utilities.ts

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -146,14 +146,12 @@ export function useNodeUtilities(blocks: Record<string, BlockState>) {
146146
* A child's position is relative to its container's own origin — React Flow
147147
* places it at the parent's origin plus its position, and
148148
* `clampPositionToContainer` is what holds it clear of the chrome, flooring it
149-
* at `LEFT_PADDING` and `HEADER_HEIGHT + TOP_PADDING`. The container's header
150-
* and padding are therefore already inside the child's coordinates, and
151-
* adding them again here counted them twice: a nested node reported 16px
152-
* right and 66px below where it actually is.
149+
* at `CONTAINER_CHILD_MIN_POSITION`. The container's Start lane, header, and
150+
* padding are therefore already inside the child's coordinates, and adding
151+
* them again here would count them twice.
153152
*
154-
* That is why callers wanting a relative position had to subtract the same
155-
* three constants straight back off, and why `positionAbsolute` — React
156-
* Flow's own answer, which carries no offset — disagreed with this one.
153+
* React Flow's `positionAbsolute` follows the same rule and carries no extra
154+
* chrome offset.
157155
*
158156
* @param nodeId ID of the node to check
159157
* @returns Absolute position coordinates {x, y}
@@ -368,6 +366,7 @@ export function useNodeUtilities(blocks: Record<string, BlockState>) {
368366
* Updates a node's parent with proper position calculation
369367
* @param nodeId ID of the node being reparented
370368
* @param newParentId ID of the new parent (or null to remove parent)
369+
* @param newPosition Final position in the new parent's coordinate space
371370
* @param batchUpdatePositions Function to batch update positions of blocks
372371
* @param batchUpdateBlocksWithParent Function to batch update blocks with parent info
373372
* @param resizeCallback Function to resize loop nodes after parent update
@@ -376,6 +375,7 @@ export function useNodeUtilities(blocks: Record<string, BlockState>) {
376375
(
377376
nodeId: string,
378377
newParentId: string | null,
378+
newPosition: { x: number; y: number },
379379
batchUpdatePositions: (
380380
updates: Array<{ id: string; position: { x: number; y: number } }>
381381
) => void,
@@ -391,22 +391,16 @@ export function useNodeUtilities(blocks: Record<string, BlockState>) {
391391
if (newParentId === currentParentId) return
392392

393393
if (newParentId) {
394-
const relativePosition = calculateRelativePosition(nodeId, newParentId)
395-
396-
batchUpdatePositions([{ id: nodeId, position: relativePosition }])
397-
batchUpdateBlocksWithParent([
398-
{ id: nodeId, position: relativePosition, parentId: newParentId },
399-
])
394+
batchUpdatePositions([{ id: nodeId, position: newPosition }])
395+
batchUpdateBlocksWithParent([{ id: nodeId, position: newPosition, parentId: newParentId }])
400396
} else if (currentParentId) {
401-
const absolutePosition = getNodeAbsolutePosition(nodeId)
402-
403-
batchUpdatePositions([{ id: nodeId, position: absolutePosition }])
404-
batchUpdateBlocksWithParent([{ id: nodeId, position: absolutePosition, parentId: '' }])
397+
batchUpdatePositions([{ id: nodeId, position: newPosition }])
398+
batchUpdateBlocksWithParent([{ id: nodeId, position: newPosition, parentId: '' }])
405399
}
406400

407401
resizeCallback()
408402
},
409-
[getNodes, blocks, calculateRelativePosition, getNodeAbsolutePosition]
403+
[getNodes, blocks]
410404
)
411405

412406
/**

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils.test.ts

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { CONTAINER_DIMENSIONS } from '@sim/workflow-renderer'
4+
import {
5+
BLOCK_DIMENSIONS,
6+
CONTAINER_CHILD_MIN_POSITION,
7+
CONTAINER_DIMENSIONS,
8+
} from '@sim/workflow-renderer'
59
import { describe, expect, it } from 'vitest'
610
import {
711
calculateContainerDimensions,
812
clampPositionToContainer,
13+
getContainerChildExtent,
14+
shiftPositionsToContainerBounds,
915
} from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils'
1016

1117
describe('calculateContainerDimensions', () => {
@@ -29,21 +35,53 @@ describe('calculateContainerDimensions', () => {
2935
})
3036

3137
it('holds a child pinned to the top-left clear of the chrome', () => {
32-
/* The floor `clampPositionToContainer` applies is what encodes the header
33-
and leading padding into the child's own coordinates — the sizing math
34-
reads them from there rather than adding them again. */
38+
expect(CONTAINER_CHILD_MIN_POSITION.x).toBe(
39+
BLOCK_DIMENSIONS.SUBFLOW_START_LEFT_OFFSET +
40+
BLOCK_DIMENSIONS.SUBFLOW_START_WIDTH +
41+
CONTAINER_DIMENSIONS.LEFT_PADDING
42+
)
43+
expect(CONTAINER_CHILD_MIN_POSITION.x).toBe(98)
44+
3545
const pinned = clampPositionToContainer(
3646
{ x: -999, y: -999 },
3747
{ width: 900, height: 900 },
3848
{ width: 250, height: 112 }
3949
)
4050

4151
expect(pinned).toEqual({
42-
x: CONTAINER_DIMENSIONS.LEFT_PADDING,
43-
y: CONTAINER_DIMENSIONS.HEADER_HEIGHT + CONTAINER_DIMENSIONS.TOP_PADDING,
52+
x: CONTAINER_CHILD_MIN_POSITION.x,
53+
y: CONTAINER_CHILD_MIN_POSITION.y,
4454
})
4555
})
4656

57+
it('uses the same leading floor for runtime dragging while leaving growth unconstrained', () => {
58+
expect(getContainerChildExtent()).toEqual([
59+
[CONTAINER_CHILD_MIN_POSITION.x, CONTAINER_CHILD_MIN_POSITION.y],
60+
[Number.POSITIVE_INFINITY, Number.POSITIVE_INFINITY],
61+
])
62+
})
63+
64+
it('moves a batch clear of the Start lane without changing its arrangement', () => {
65+
const updates = [
66+
{ blockId: 'parallel', newPosition: { x: 24, y: 40 } },
67+
{ blockId: 'agent', newPosition: { x: 224, y: 140 } },
68+
]
69+
70+
expect(shiftPositionsToContainerBounds(updates)).toEqual([
71+
{
72+
blockId: 'parallel',
73+
newPosition: CONTAINER_CHILD_MIN_POSITION,
74+
},
75+
{
76+
blockId: 'agent',
77+
newPosition: {
78+
x: 224 + (CONTAINER_CHILD_MIN_POSITION.x - 24),
79+
y: 140 + (CONTAINER_CHILD_MIN_POSITION.y - 40),
80+
},
81+
},
82+
])
83+
})
84+
4785
it('falls back to the default box when it holds nothing', () => {
4886
expect(calculateContainerDimensions([])).toEqual({
4987
width: CONTAINER_DIMENSIONS.DEFAULT_WIDTH,

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils.ts

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
import { BLOCK_DIMENSIONS, CONTAINER_DIMENSIONS, getNoteBlockHeight } from '@sim/workflow-renderer'
1+
import {
2+
BLOCK_DIMENSIONS,
3+
CONTAINER_CHILD_MIN_POSITION,
4+
CONTAINER_DIMENSIONS,
5+
getNoteBlockHeight,
6+
} from '@sim/workflow-renderer'
27
import { showsCanvasErrorRow } from '@/lib/workflows/blocks/canvas-rows'
38
import { calculateWorkflowBlockDimensions } from '@/lib/workflows/blocks/deterministic-dimensions'
49
import { getBlock } from '@/blocks/registry'
@@ -54,8 +59,7 @@ export function clampPositionToContainer(
5459
const { width: containerWidth, height: containerHeight } = containerDimensions
5560
const { width: blockWidth, height: blockHeight } = blockDimensions
5661

57-
const minX = CONTAINER_DIMENSIONS.LEFT_PADDING
58-
const minY = CONTAINER_DIMENSIONS.HEADER_HEIGHT + CONTAINER_DIMENSIONS.TOP_PADDING
62+
const { x: minX, y: minY } = CONTAINER_CHILD_MIN_POSITION
5963
const maxX = containerWidth - CONTAINER_DIMENSIONS.RIGHT_PADDING - blockWidth
6064
const maxY = containerHeight - CONTAINER_DIMENSIONS.BOTTOM_PADDING - blockHeight
6165

@@ -65,6 +69,42 @@ export function clampPositionToContainer(
6569
}
6670
}
6771

72+
/**
73+
* Runtime React Flow bounds for a nested node.
74+
*
75+
* The finite leading edge protects the header and Start lane. The trailing
76+
* edge stays open because moving a child right or down expands its container.
77+
* Persisted workflow data continues using `extent: 'parent'`.
78+
*/
79+
export function getContainerChildExtent(): [[number, number], [number, number]] {
80+
return [
81+
[CONTAINER_CHILD_MIN_POSITION.x, CONTAINER_CHILD_MIN_POSITION.y],
82+
[Number.POSITIVE_INFINITY, Number.POSITIVE_INFINITY],
83+
]
84+
}
85+
86+
/** Keeps a batch's relative arrangement while moving it clear of container chrome. */
87+
export function shiftPositionsToContainerBounds<
88+
T extends { newPosition: { x: number; y: number } },
89+
>(rawUpdates: T[]): T[] {
90+
if (rawUpdates.length === 0) return rawUpdates
91+
92+
const minX = Math.min(...rawUpdates.map((update) => update.newPosition.x))
93+
const minY = Math.min(...rawUpdates.map((update) => update.newPosition.y))
94+
const shiftX = Math.max(0, CONTAINER_CHILD_MIN_POSITION.x - minX)
95+
const shiftY = Math.max(0, CONTAINER_CHILD_MIN_POSITION.y - minY)
96+
97+
if (shiftX === 0 && shiftY === 0) return rawUpdates
98+
99+
return rawUpdates.map((update) => ({
100+
...update,
101+
newPosition: {
102+
x: update.newPosition.x + shiftX,
103+
y: update.newPosition.y + shiftY,
104+
},
105+
}))
106+
}
107+
68108
/**
69109
* Calculates container dimensions based on child block positions.
70110
* Single source of truth for container sizing - ensures consistency between
@@ -73,11 +113,10 @@ export function clampPositionToContainer(
73113
* Child coordinates are relative to the container's own origin — React Flow
74114
* places a child at the parent's origin plus its position, and
75115
* {@link clampPositionToContainer} keeps them clear of the chrome by flooring
76-
* them at `LEFT_PADDING` and `HEADER_HEIGHT + TOP_PADDING`. A child's far edge
77-
* is therefore already the distance the container has to cover, and only the
78-
* trailing padding is owed on top. Adding the header and leading padding here
79-
* as well counted them twice, leaving every container 66px taller and 16px
80-
* wider than its contents.
116+
* them at `CONTAINER_CHILD_MIN_POSITION`. A child's far edge is therefore
117+
* already the distance the container has to cover, and only the trailing
118+
* padding is owed on top. Adding the header and leading lane here as well would
119+
* count them twice.
81120
*
82121
* @param childPositions - Array of child positions with their dimensions
83122
* @returns Calculated width and height for the container

0 commit comments

Comments
 (0)