Skip to content

Commit 128054e

Browse files
authored
fix(workflow): stop subflows resizing themselves after every load (#6639)
* fix(workflow): stop subflows resizing themselves after every load A container sized itself from its children, and when a child had not yet reported a height it used `estimateBlockDimensions` in its place — a guess of `ceil(subBlockCount / 2)` rows, which read a 39-field Gmail card as 276px tall against the 112px it draws. The container painted that number, the real height arrived a frame later, and it visibly resized between the two. Nothing is persisted, so it happened on every refresh. A card's height depends on what it actually renders — which rows survive its conditions, whether it draws a summary sentence, and for a reactive field even a credential it has to fetch — so the card is the only thing that can know it. Size only from heights the children have themselves reported, and hold the container at its current size until they have. `getBlockDimensions` keeps the estimate for the callers that only need a rough box (clamping a drag, placing a paste) and is now that same lookup plus the fallback. Also stop `calculateContainerDimensions` counting the container's chrome twice. Child coordinates are relative to the container's own origin and are already held clear of the header by `clampPositionToContainer`, so a child's far edge is the distance to cover and only the trailing padding is owed on top. Adding the header and leading padding again left every container 66px taller and 16px wider than its contents. * fix(workflow): gate container sizing on this session's reported layout Two holes in the measurement gate, both from reading the wrong field. `height` is a persisted column and `data.width` / `data.height` persist a container's last size, so a block that has not reported yet can still carry last session's numbers — reachable through paste, import, and checkpoint restore. The gate treated those as reported and sized from them. Nested containers had it worse: an inner container with an unreported descendant handed back its 500x300 default as though it were measured, so the outer container sized to that and resized again once the descendant filled in — the same two-step this change exists to remove. `layout` is in-memory only and written by exactly the two places that know: a card through `updateBlockLayoutMetrics`, a container through `updateNodeDimensions`. Reading it means "reported during this session" and nothing else, and an inner container that is still waiting reports null, so the outer one waits with it. `getBlockDimensions` keeps the persisted height and the estimate as fallbacks — its callers only need a rough box, where a stale height still beats a guess. * Revert "fix(workflow): gate container sizing on this session's reported layout" This reverts commit 3cce724. * fix(workflow): size containers from a state-aware child estimate The gate in the reverted commit held a container at its current size until its children reported. That is worse than it sounds: the size it holds is the persisted default of 300, the child needs 335, and so the child hung outside the container until something forced a resize. Estimate accurately instead of waiting. `getBlockMetrics` derives a card's height from the block's own state — the sub-blocks its values leave visible, the summary sentence, the error row — through the same `calculateWorkflowBlockDimensions` the card calls, and lands on the height the card goes on to render: 112px for the Gmail card the type-only estimate put at 276px. The pass before the cards report and the pass after now produce the same container, so there is nothing to gate and nothing to correct. This also fixes the guess everywhere else it was painted rather than only in the container path — `estimateBlockDimensions` fed React Flow's node height for unmeasured blocks, so selection bounds were 276px around a 112px card. * improvement(workflow): even out the gutter inside a container Left, top and bottom were 16 and the bottom read tighter than either, because the 50px header sits above the top gap and gives that edge visual weight the other two do not have. Taking them to 24 leaves the three gutter-only edges matching and the bottom no longer pinched. Right stays 80. The container's output handle sits on that edge, so a child needs clearance there it does not need anywhere else — chrome rather than gutter, now said so in the type. Only reachable as a single constant each because the paddings mean what they say: each is the gap between a child's edge and the container's, counted once. While the sizing math added the header and leading padding a second time, the effective bottom gap was spread across three constants and tuning it meant reasoning about all of them. * improvement(workflow): give a container one source for its own gutter The four paddings and the header height existed twice: as `CONTAINER_DIMENSIONS`, which sizes a container and clamps its children, and again as Tailwind literals in `subflow-node-view`, which draws the header and the content box. Nothing kept them in step and they had already drifted — the view rendering a 40px header against a constant claiming 50, so children were clamped 10px below where the header actually ends. The view now renders from the constants, and the constant follows the DOM at 40. Match the bottom gutter to the right at 80. The two edges that carry chrome are now the two that are wider: the container's output handle sits on the right, and the resize grip in the bottom-right corner spans 40px in from both, so a child at the 24px gutter width could sit underneath it. Left and top are only gutter and stay at 24. * test(workflow-renderer): assert the subflow header's height, not its class The header renders from `CONTAINER_DIMENSIONS.HEADER_HEIGHT` now, so the class it used to carry is gone. Assert the rendered height against the same constant the layout math measures against — the two drifting apart is what this whole change is about, and a utility-class assertion cannot catch that. Also set `IS_REACT_ACT_ENVIRONMENT`, which these tests have always needed. React only treats `act` as supported when it can see the flag, so every render logged "The current testing environment is not configured to support act(...)" — around forty lines of it per run, burying the actual failure output. * fix(workflow-renderer): declare the act-environment global `vitest.setup.ts` is inside the package's tsconfig, so assigning an undeclared property on `globalThis` failed type-check (TS7017) even though the tests ran. * fix(workflow): size a container from one snapshot of the store `calculateLoopDimensions` took child positions from the live store but child dimensions from the hook's render snapshot, so it was reading two ages of the same data. `resizeLoopNodes` walks deepest-first: an inner container resized earlier in the pass was already updated in the live snapshot and still stale in the closed-over one, so its parent sized against the old inner box and only caught up on a later render — a nested container visibly resizing twice, which is the symptom this branch set out to remove. Take both from the snapshot the function already reads. * fix(workflow): size an unmeasured note as a note Routing every non-container block through `getBlockMetrics` sent notes through the workflow-card estimate, which counts sub-block rows and an error row a note does not have. A note that had not reported a height yet got a card's box, so a container holding one sized itself around the wrong shape. Give a note its own branch, as the estimate it replaced did: measured height when there is one, and the height an empty note paints when there is not.
1 parent 2805a8d commit 128054e

7 files changed

Lines changed: 181 additions & 38 deletions

File tree

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

Lines changed: 54 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { useCallback } from 'react'
22
import { createLogger } from '@sim/logger'
3-
import { BLOCK_DIMENSIONS, CONTAINER_DIMENSIONS } from '@sim/workflow-renderer'
3+
import { BLOCK_DIMENSIONS, CONTAINER_DIMENSIONS, getNoteBlockHeight } from '@sim/workflow-renderer'
44
import { useReactFlow } from 'reactflow'
5+
import { getBlockMetrics } from '@/lib/workflows/autolayout/utils'
56
import {
67
calculateContainerDimensions,
78
clampPositionToContainer,
8-
estimateBlockDimensions,
99
} from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils'
1010
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
1111

@@ -26,39 +26,57 @@ export function useNodeUtilities(blocks: Record<string, any>) {
2626

2727
/**
2828
* Get the dimensions of a block.
29-
* For regular blocks, uses stored height or estimates based on block config.
29+
*
30+
* Before a card mounts there is no measurement, so the height comes from
31+
* {@link getBlockMetrics}, which estimates it from the block's own state —
32+
* the sub-blocks its values leave visible, the summary sentence it will draw,
33+
* the error row — through the same `calculateWorkflowBlockDimensions` the
34+
* card itself calls. It lands on the height the card goes on to render.
35+
*
36+
* The old estimate read the block's *type* alone and assumed
37+
* `ceil(subBlockCount / 2)` rows, which put a 39-field Gmail card at 276px
38+
* against the 112px it draws. A container sized from that, painted it, then
39+
* got the real height a frame later and visibly resized — on every load,
40+
* because measurements are not persisted. Estimating from state removes the
41+
* gap rather than waiting it out: both passes now produce the same number.
3042
*/
31-
const getBlockDimensions = useCallback(
32-
(blockId: string): { width: number; height: number } => {
33-
const block = blocks[blockId]
43+
const dimensionsOfBlock = useCallback(
44+
(block: any): { width: number; height: number } => {
3445
if (!block) {
3546
return { width: BLOCK_DIMENSIONS.FIXED_WIDTH, height: BLOCK_DIMENSIONS.MIN_HEIGHT }
3647
}
3748

3849
if (isContainerType(block.type)) {
3950
return {
40-
width: block.data?.width
41-
? Math.max(block.data.width, CONTAINER_DIMENSIONS.MIN_WIDTH)
42-
: CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
43-
height: block.data?.height
44-
? Math.max(block.data.height, CONTAINER_DIMENSIONS.MIN_HEIGHT)
45-
: CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
51+
width: Math.max(
52+
block.data?.width || CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
53+
CONTAINER_DIMENSIONS.MIN_WIDTH
54+
),
55+
height: Math.max(
56+
block.data?.height || CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
57+
CONTAINER_DIMENSIONS.MIN_HEIGHT
58+
),
4659
}
4760
}
4861

49-
if (block.height) {
62+
/* A note is not a card: it has no sub-block rows and no error row, so the
63+
card estimate does not describe it. Its own height is what it was
64+
measured at, or the height an empty one paints. */
65+
if (block.type === 'note') {
5066
return {
51-
width: block.type === 'note' ? BLOCK_DIMENSIONS.NOTE_WIDTH : BLOCK_DIMENSIONS.FIXED_WIDTH,
52-
height:
53-
block.type === 'note'
54-
? block.height
55-
: Math.max(block.height, BLOCK_DIMENSIONS.MIN_HEIGHT),
67+
width: BLOCK_DIMENSIONS.NOTE_WIDTH,
68+
height: block.height || getNoteBlockHeight(true),
5669
}
5770
}
5871

59-
return estimateBlockDimensions(block.type)
72+
return getBlockMetrics(block)
6073
},
61-
[blocks, isContainerType]
74+
[isContainerType]
75+
)
76+
77+
const getBlockDimensions = useCallback(
78+
(blockId: string): { width: number; height: number } => dimensionsOfBlock(blocks[blockId]),
79+
[blocks, dimensionsOfBlock]
6280
)
6381

6482
/**
@@ -270,6 +288,12 @@ export function useNodeUtilities(blocks: Record<string, any>) {
270288

271289
/**
272290
* Calculates appropriate dimensions for a loop or parallel node based on its children
291+
*
292+
* Child heights come from {@link getBlockDimensions}, which estimates from
293+
* block state when a card has not mounted yet and lands on the height it will
294+
* render — so the size computed before the cards report matches the one after,
295+
* and the container does not resize behind the user.
296+
*
273297
* @param nodeId ID of the container node
274298
* @returns Calculated width and height for the container
275299
*/
@@ -284,14 +308,21 @@ export function useNodeUtilities(blocks: Record<string, any>) {
284308
.map((childId) => {
285309
const child = currentBlocks[childId]
286310
if (!child?.position) return null
287-
const { width, height } = getBlockDimensions(childId)
311+
/* Sized from `currentBlocks`, the same snapshot the position came
312+
from. Reading dimensions off the hook's render snapshot instead
313+
mixed two ages of the same store: `resizeLoopNodes` walks
314+
deepest-first, so an inner container resized earlier in the pass
315+
was already updated here but still old there, and the parent sized
316+
against a stale inner box — leaving nested containers to converge
317+
over a second pass. */
318+
const { width, height } = dimensionsOfBlock(child)
288319
return { x: child.position.x, y: child.position.y, width, height }
289320
})
290-
.filter((p): p is NonNullable<typeof p> => p !== null)
321+
.filter((position): position is NonNullable<typeof position> => position !== null)
291322

292323
return calculateContainerDimensions(childPositions)
293324
},
294-
[getBlockDimensions]
325+
[dimensionsOfBlock]
295326
)
296327

297328
/**
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { CONTAINER_DIMENSIONS } from '@sim/workflow-renderer'
5+
import { describe, expect, it } from 'vitest'
6+
import {
7+
calculateContainerDimensions,
8+
clampPositionToContainer,
9+
} from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils'
10+
11+
describe('calculateContainerDimensions', () => {
12+
it('covers the child it holds plus one trailing padding', () => {
13+
/* Child coordinates are relative to the container's own origin, so their
14+
far edge is already the distance to cover. */
15+
const child = { x: 273, y: 207.5, width: 250, height: 112 }
16+
17+
expect(calculateContainerDimensions([child])).toEqual({
18+
width: child.x + child.width + CONTAINER_DIMENSIONS.RIGHT_PADDING,
19+
height: child.y + child.height + CONTAINER_DIMENSIONS.BOTTOM_PADDING,
20+
})
21+
})
22+
23+
it('leaves the same gap under a child wherever the child sits', () => {
24+
const gapUnder = (y: number) =>
25+
calculateContainerDimensions([{ x: 600, y, width: 250, height: 112 }]).height - (y + 112)
26+
27+
expect(gapUnder(400)).toBe(CONTAINER_DIMENSIONS.BOTTOM_PADDING)
28+
expect(gapUnder(700)).toBe(CONTAINER_DIMENSIONS.BOTTOM_PADDING)
29+
})
30+
31+
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. */
35+
const pinned = clampPositionToContainer(
36+
{ x: -999, y: -999 },
37+
{ width: 900, height: 900 },
38+
{ width: 250, height: 112 }
39+
)
40+
41+
expect(pinned).toEqual({
42+
x: CONTAINER_DIMENSIONS.LEFT_PADDING,
43+
y: CONTAINER_DIMENSIONS.HEADER_HEIGHT + CONTAINER_DIMENSIONS.TOP_PADDING,
44+
})
45+
})
46+
47+
it('falls back to the default box when it holds nothing', () => {
48+
expect(calculateContainerDimensions([])).toEqual({
49+
width: CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
50+
height: CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
51+
})
52+
})
53+
54+
it('never sizes below the default box', () => {
55+
expect(calculateContainerDimensions([{ x: 16, y: 66, width: 40, height: 20 }])).toEqual({
56+
width: CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
57+
height: CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
58+
})
59+
})
60+
})

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,15 @@ export function clampPositionToContainer(
7070
* Single source of truth for container sizing - ensures consistency between
7171
* live drag updates and final dimension calculations.
7272
*
73+
* Child coordinates are relative to the container's own origin — React Flow
74+
* places a child at the parent's origin plus its position, and
75+
* {@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.
81+
*
7382
* @param childPositions - Array of child positions with their dimensions
7483
* @returns Calculated width and height for the container
7584
*/
@@ -93,14 +102,11 @@ export function calculateContainerDimensions(
93102

94103
const width = Math.max(
95104
CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
96-
CONTAINER_DIMENSIONS.LEFT_PADDING + maxRight + CONTAINER_DIMENSIONS.RIGHT_PADDING
105+
maxRight + CONTAINER_DIMENSIONS.RIGHT_PADDING
97106
)
98107
const height = Math.max(
99108
CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
100-
CONTAINER_DIMENSIONS.HEADER_HEIGHT +
101-
CONTAINER_DIMENSIONS.TOP_PADDING +
102-
maxBottom +
103-
CONTAINER_DIMENSIONS.BOTTOM_PADDING
109+
maxBottom + CONTAINER_DIMENSIONS.BOTTOM_PADDING
104110
)
105111

106112
return { width, height }

packages/workflow-renderer/src/dimensions.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,36 @@ export const estimateNoteBlockHeight = (content: string) => {
8080
)
8181
}
8282

83+
/**
84+
* A container's box, and the gutter it keeps around the blocks inside it.
85+
*
86+
* The single source for both halves of that: the layout math that sizes a
87+
* container and clamps its children, and the card's own DOM. `subflow-node-view`
88+
* renders its header and content box straight from these, so the gap the
89+
* geometry reserves is the gap the container actually paints. They used to be
90+
* separate — the same four numbers as Tailwind literals in the view — and had
91+
* already drifted, the view drawing a 40px header against a constant that
92+
* claimed 50.
93+
*
94+
* Each padding is the gap between a child's edge and the container's, counted
95+
* once: the header is accounted for by the child's own position, which
96+
* `clampPositionToContainer` floors at `HEADER_HEIGHT + TOP_PADDING`.
97+
*
98+
* The two edges that carry chrome are wider than the two that are only gutter.
99+
* The container's output handle sits on the right, and the resize grip in the
100+
* bottom-right corner spans 40px in from both — a child at the gutter width
101+
* would sit underneath it.
102+
*/
83103
export const CONTAINER_DIMENSIONS = {
84104
DEFAULT_WIDTH: 500,
85105
DEFAULT_HEIGHT: 300,
86106
MIN_WIDTH: 400,
87107
MIN_HEIGHT: 200,
88-
HEADER_HEIGHT: 50,
89-
LEFT_PADDING: 16,
108+
HEADER_HEIGHT: 40,
109+
LEFT_PADDING: 24,
90110
RIGHT_PADDING: 80,
91-
TOP_PADDING: 16,
92-
BOTTOM_PADDING: 16,
111+
TOP_PADDING: 24,
112+
BOTTOM_PADDING: 80,
93113
} as const
94114

95115
/**

packages/workflow-renderer/src/subflow/subflow-node-view.tsx

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
useStoreApi as useReactFlowStoreApi,
99
useUpdateNodeInternals,
1010
} from 'reactflow'
11-
import { BLOCK_DIMENSIONS, HANDLE_POSITIONS } from '../dimensions'
11+
import { BLOCK_DIMENSIONS, CONTAINER_DIMENSIONS, HANDLE_POSITIONS } from '../dimensions'
1212
import { OverflowSpan } from '../lib/overflow-span'
1313
import type { DiffStatus } from '../types'
1414
import {
@@ -588,8 +588,8 @@ export function SubflowNodeView({
588588
aria-label={`Select ${blockName}`}
589589
onClick={onSelect}
590590
onKeyDown={(event) => handleKeyboardActivation(event, onSelect)}
591-
className='workflow-drag-handle relative z-20 flex h-[40px] cursor-grab items-center justify-between px-2 [&:active]:cursor-grabbing'
592-
style={{ pointerEvents: 'auto' }}
591+
className='workflow-drag-handle relative z-20 flex cursor-grab items-center justify-between px-2 [&:active]:cursor-grabbing'
592+
style={{ pointerEvents: 'auto', height: CONTAINER_DIMENSIONS.HEADER_HEIGHT }}
593593
data-subflow-header=''
594594
>
595595
<div
@@ -647,9 +647,16 @@ export function SubflowNodeView({
647647
)}
648648

649649
<div
650-
className='relative z-20 h-[calc(100%-40px)] pt-4 pr-[80px] pb-4 pl-4'
650+
className='relative z-20'
651651
data-dragarea='true'
652-
style={{ pointerEvents: 'none' }}
652+
style={{
653+
pointerEvents: 'none',
654+
height: `calc(100% - ${CONTAINER_DIMENSIONS.HEADER_HEIGHT}px)`,
655+
paddingTop: CONTAINER_DIMENSIONS.TOP_PADDING,
656+
paddingRight: CONTAINER_DIMENSIONS.RIGHT_PADDING,
657+
paddingBottom: CONTAINER_DIMENSIONS.BOTTOM_PADDING,
658+
paddingLeft: CONTAINER_DIMENSIONS.LEFT_PADDING,
659+
}}
653660
>
654661
<SubflowStartView
655662
parentId={id}

packages/workflow-renderer/src/workflow-block/workflow-block-border-mount.test.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import { createRoot, type Root } from 'react-dom/client'
1414
import { ReactFlowProvider } from 'reactflow'
1515
import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'
16+
import { CONTAINER_DIMENSIONS } from '../dimensions'
1617
import {
1718
ERROR_SOURCE_HANDLE_POSITION,
1819
getCursorBranchSourceHandleId,
@@ -959,7 +960,10 @@ describe('WorkflowBlockBorder mount', () => {
959960
)
960961

961962
const header = host.querySelector('[data-subflow-header]')
962-
expect(header).toHaveClass('h-[40px]')
963+
/* Height comes from `CONTAINER_DIMENSIONS`, which the layout math also
964+
measures against — asserting the rendered value rather than a utility
965+
class keeps the two from drifting apart again. */
966+
expect(header).toHaveStyle({ height: `${CONTAINER_DIMENSIONS.HEADER_HEIGHT}px` })
963967
expect(header).not.toHaveClass('border-b')
964968
expect(header).not.toHaveClass('bg-[var(--surface-2)]')
965969
expect(host.querySelector('[data-subflow-type-tag="loop"]')).toHaveTextContent('Loop')
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,23 @@
1+
declare global {
2+
/**
3+
* React reads this to decide whether `act` is supported. It is not one of the
4+
* ambient globals, so it has to be declared before it can be assigned.
5+
*/
6+
var IS_REACT_ACT_ENVIRONMENT: boolean
7+
}
8+
19
/**
210
* jest-dom only registers DOM matchers (`toHaveStyle`, `toHaveClass`, …), so it is
311
* dead weight outside a DOM environment. This package's mount tests opt into jsdom
412
* per file, so load it only when one is actually running — mirroring `apps/sim`.
513
*/
614
if (typeof document !== 'undefined') {
715
await import('@testing-library/jest-dom/vitest')
16+
/*
17+
* Without this React treats `act` as unsupported, and every render in the
18+
* mount tests logs "The current testing environment is not configured to
19+
* support act(...)" — around forty lines a run, which buries the output that
20+
* matters when one of them fails.
21+
*/
22+
globalThis.IS_REACT_ACT_ENVIRONMENT = true
823
}

0 commit comments

Comments
 (0)