mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-01 14:59:19 +08:00
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.
This commit is contained in:
@@ -25,40 +25,64 @@ export function useNodeUtilities(blocks: Record<string, any>) {
|
||||
}, [])
|
||||
|
||||
/**
|
||||
* Get the dimensions of a block.
|
||||
* For regular blocks, uses stored height or estimates based on block config.
|
||||
* A block's dimensions as the block itself reported them, or null if it has
|
||||
* not reported yet.
|
||||
*
|
||||
* {@link getBlockDimensions} without the estimate fallback — a rough box is
|
||||
* fine for clamping a drag or placing a paste; see
|
||||
* {@link calculateLoopDimensions} for why a container cannot size from one.
|
||||
*
|
||||
* A container's own size is already derived from its children, so it counts
|
||||
* as reported once it has one; an empty container reports its default.
|
||||
*/
|
||||
const getReportedBlockDimensions = useCallback(
|
||||
(blockId: string): { width: number; height: number } | null => {
|
||||
const block = blocks[blockId]
|
||||
if (!block) return null
|
||||
|
||||
if (isContainerType(block.type)) {
|
||||
return {
|
||||
width: Math.max(
|
||||
block.data?.width || CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
|
||||
CONTAINER_DIMENSIONS.MIN_WIDTH
|
||||
),
|
||||
height: Math.max(
|
||||
block.data?.height || CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
|
||||
CONTAINER_DIMENSIONS.MIN_HEIGHT
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
if (!block.height) return null
|
||||
|
||||
return {
|
||||
width: block.type === 'note' ? BLOCK_DIMENSIONS.NOTE_WIDTH : BLOCK_DIMENSIONS.FIXED_WIDTH,
|
||||
height:
|
||||
block.type === 'note'
|
||||
? block.height
|
||||
: Math.max(block.height, BLOCK_DIMENSIONS.MIN_HEIGHT),
|
||||
}
|
||||
},
|
||||
[blocks, isContainerType]
|
||||
)
|
||||
|
||||
/**
|
||||
* Get the dimensions of a block, estimating from its type when it has not
|
||||
* reported a height yet.
|
||||
*/
|
||||
const getBlockDimensions = useCallback(
|
||||
(blockId: string): { width: number; height: number } => {
|
||||
const reported = getReportedBlockDimensions(blockId)
|
||||
if (reported) return reported
|
||||
|
||||
const block = blocks[blockId]
|
||||
if (!block) {
|
||||
return { width: BLOCK_DIMENSIONS.FIXED_WIDTH, height: BLOCK_DIMENSIONS.MIN_HEIGHT }
|
||||
}
|
||||
|
||||
if (isContainerType(block.type)) {
|
||||
return {
|
||||
width: block.data?.width
|
||||
? Math.max(block.data.width, CONTAINER_DIMENSIONS.MIN_WIDTH)
|
||||
: CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
|
||||
height: block.data?.height
|
||||
? Math.max(block.data.height, CONTAINER_DIMENSIONS.MIN_HEIGHT)
|
||||
: CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
|
||||
}
|
||||
}
|
||||
|
||||
if (block.height) {
|
||||
return {
|
||||
width: block.type === 'note' ? BLOCK_DIMENSIONS.NOTE_WIDTH : BLOCK_DIMENSIONS.FIXED_WIDTH,
|
||||
height:
|
||||
block.type === 'note'
|
||||
? block.height
|
||||
: Math.max(block.height, BLOCK_DIMENSIONS.MIN_HEIGHT),
|
||||
}
|
||||
}
|
||||
|
||||
return estimateBlockDimensions(block.type)
|
||||
},
|
||||
[blocks, isContainerType]
|
||||
[blocks, getReportedBlockDimensions]
|
||||
)
|
||||
|
||||
/**
|
||||
@@ -270,28 +294,44 @@ export function useNodeUtilities(blocks: Record<string, any>) {
|
||||
|
||||
/**
|
||||
* Calculates appropriate dimensions for a loop or parallel node based on its children
|
||||
*
|
||||
* Sizes only from heights the children have themselves reported. 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, and it publishes it once it does.
|
||||
*
|
||||
* Guessing in the meantime is what made a container resize on every load:
|
||||
* `estimateBlockDimensions` assumes `ceil(subBlockCount / 2)` rows, so it read
|
||||
* a 39-field Gmail card as 276px tall against the 112px it draws. The
|
||||
* container painted that, then the real height arrived a frame later and it
|
||||
* visibly resized. Returning null holds the container at the size it already
|
||||
* has, so it moves once, to the right answer.
|
||||
*
|
||||
* @param nodeId ID of the container node
|
||||
* @returns Calculated width and height for the container
|
||||
* @returns Calculated dimensions, or null while any child is still unmeasured
|
||||
*/
|
||||
const calculateLoopDimensions = useCallback(
|
||||
(nodeId: string): { width: number; height: number } => {
|
||||
(nodeId: string): { width: number; height: number } | null => {
|
||||
const currentBlocks = useWorkflowStore.getState().blocks
|
||||
const childBlockIds = Object.keys(currentBlocks).filter(
|
||||
(id) => currentBlocks[id]?.data?.parentId === nodeId
|
||||
)
|
||||
|
||||
const childPositions = childBlockIds
|
||||
.map((childId) => {
|
||||
const child = currentBlocks[childId]
|
||||
if (!child?.position) return null
|
||||
const { width, height } = getBlockDimensions(childId)
|
||||
return { x: child.position.x, y: child.position.y, width, height }
|
||||
})
|
||||
.filter((p): p is NonNullable<typeof p> => p !== null)
|
||||
const childPositions: Array<{ x: number; y: number; width: number; height: number }> = []
|
||||
for (const childId of childBlockIds) {
|
||||
const child = currentBlocks[childId]
|
||||
if (!child?.position) continue
|
||||
|
||||
const reported = getReportedBlockDimensions(childId)
|
||||
if (!reported) return null
|
||||
|
||||
childPositions.push({ x: child.position.x, y: child.position.y, ...reported })
|
||||
}
|
||||
|
||||
return calculateContainerDimensions(childPositions)
|
||||
},
|
||||
[getBlockDimensions]
|
||||
[getReportedBlockDimensions]
|
||||
)
|
||||
|
||||
/**
|
||||
@@ -312,6 +352,8 @@ export function useNodeUtilities(blocks: Record<string, any>) {
|
||||
|
||||
for (const { id, block } of containerBlocks) {
|
||||
const dimensions = calculateLoopDimensions(id)
|
||||
if (!dimensions) continue
|
||||
|
||||
const currentWidth = block?.data?.width
|
||||
const currentHeight = block?.data?.height
|
||||
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import { CONTAINER_DIMENSIONS } from '@sim/workflow-renderer'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
calculateContainerDimensions,
|
||||
clampPositionToContainer,
|
||||
} from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils'
|
||||
|
||||
describe('calculateContainerDimensions', () => {
|
||||
it('covers the child it holds plus one trailing padding', () => {
|
||||
/* Child coordinates are relative to the container's own origin, so their
|
||||
far edge is already the distance to cover. */
|
||||
const child = { x: 273, y: 207.5, width: 250, height: 112 }
|
||||
|
||||
expect(calculateContainerDimensions([child])).toEqual({
|
||||
width: child.x + child.width + CONTAINER_DIMENSIONS.RIGHT_PADDING,
|
||||
height: child.y + child.height + CONTAINER_DIMENSIONS.BOTTOM_PADDING,
|
||||
})
|
||||
})
|
||||
|
||||
it('leaves the same gap under a child wherever the child sits', () => {
|
||||
const gapUnder = (y: number) =>
|
||||
calculateContainerDimensions([{ x: 600, y, width: 250, height: 112 }]).height - (y + 112)
|
||||
|
||||
expect(gapUnder(400)).toBe(CONTAINER_DIMENSIONS.BOTTOM_PADDING)
|
||||
expect(gapUnder(700)).toBe(CONTAINER_DIMENSIONS.BOTTOM_PADDING)
|
||||
})
|
||||
|
||||
it('holds a child pinned to the top-left clear of the chrome', () => {
|
||||
/* The floor `clampPositionToContainer` applies is what encodes the header
|
||||
and leading padding into the child's own coordinates — the sizing math
|
||||
reads them from there rather than adding them again. */
|
||||
const pinned = clampPositionToContainer(
|
||||
{ x: -999, y: -999 },
|
||||
{ width: 900, height: 900 },
|
||||
{ width: 250, height: 112 }
|
||||
)
|
||||
|
||||
expect(pinned).toEqual({
|
||||
x: CONTAINER_DIMENSIONS.LEFT_PADDING,
|
||||
y: CONTAINER_DIMENSIONS.HEADER_HEIGHT + CONTAINER_DIMENSIONS.TOP_PADDING,
|
||||
})
|
||||
})
|
||||
|
||||
it('falls back to the default box when it holds nothing', () => {
|
||||
expect(calculateContainerDimensions([])).toEqual({
|
||||
width: CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
|
||||
height: CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
|
||||
})
|
||||
})
|
||||
|
||||
it('never sizes below the default box', () => {
|
||||
expect(calculateContainerDimensions([{ x: 16, y: 66, width: 40, height: 20 }])).toEqual({
|
||||
width: CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
|
||||
height: CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -70,6 +70,15 @@ export function clampPositionToContainer(
|
||||
* Single source of truth for container sizing - ensures consistency between
|
||||
* live drag updates and final dimension calculations.
|
||||
*
|
||||
* Child coordinates are relative to the container's own origin — React Flow
|
||||
* places a child at the parent's origin plus its position, and
|
||||
* {@link clampPositionToContainer} keeps them clear of the chrome by flooring
|
||||
* them at `LEFT_PADDING` and `HEADER_HEIGHT + TOP_PADDING`. A child's far edge
|
||||
* is therefore already the distance the container has to cover, and only the
|
||||
* trailing padding is owed on top. Adding the header and leading padding here
|
||||
* as well counted them twice, leaving every container 66px taller and 16px
|
||||
* wider than its contents.
|
||||
*
|
||||
* @param childPositions - Array of child positions with their dimensions
|
||||
* @returns Calculated width and height for the container
|
||||
*/
|
||||
@@ -93,14 +102,11 @@ export function calculateContainerDimensions(
|
||||
|
||||
const width = Math.max(
|
||||
CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
|
||||
CONTAINER_DIMENSIONS.LEFT_PADDING + maxRight + CONTAINER_DIMENSIONS.RIGHT_PADDING
|
||||
maxRight + CONTAINER_DIMENSIONS.RIGHT_PADDING
|
||||
)
|
||||
const height = Math.max(
|
||||
CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
|
||||
CONTAINER_DIMENSIONS.HEADER_HEIGHT +
|
||||
CONTAINER_DIMENSIONS.TOP_PADDING +
|
||||
maxBottom +
|
||||
CONTAINER_DIMENSIONS.BOTTOM_PADDING
|
||||
maxBottom + CONTAINER_DIMENSIONS.BOTTOM_PADDING
|
||||
)
|
||||
|
||||
return { width, height }
|
||||
|
||||
Reference in New Issue
Block a user