diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-node-utilities.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-node-utilities.ts index 85a4a7ab128..5b59431bd5e 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-node-utilities.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-node-utilities.ts @@ -1,11 +1,11 @@ import { useCallback } from 'react' import { createLogger } from '@sim/logger' -import { BLOCK_DIMENSIONS, CONTAINER_DIMENSIONS } from '@sim/workflow-renderer' +import { BLOCK_DIMENSIONS, CONTAINER_DIMENSIONS, getNoteBlockHeight } from '@sim/workflow-renderer' import { useReactFlow } from 'reactflow' +import { getBlockMetrics } from '@/lib/workflows/autolayout/utils' import { calculateContainerDimensions, clampPositionToContainer, - estimateBlockDimensions, } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils' import { useWorkflowStore } from '@/stores/workflows/workflow/store' @@ -26,39 +26,57 @@ export function useNodeUtilities(blocks: Record) { /** * Get the dimensions of a block. - * For regular blocks, uses stored height or estimates based on block config. + * + * Before a card mounts there is no measurement, so the height comes from + * {@link getBlockMetrics}, which estimates it from the block's own state — + * the sub-blocks its values leave visible, the summary sentence it will draw, + * the error row — through the same `calculateWorkflowBlockDimensions` the + * card itself calls. It lands on the height the card goes on to render. + * + * The old estimate read the block's *type* alone and assumed + * `ceil(subBlockCount / 2)` rows, which put a 39-field Gmail card at 276px + * against the 112px it draws. A container sized from that, painted it, then + * got the real height a frame later and visibly resized — on every load, + * because measurements are not persisted. Estimating from state removes the + * gap rather than waiting it out: both passes now produce the same number. */ - const getBlockDimensions = useCallback( - (blockId: string): { width: number; height: number } => { - const block = blocks[blockId] + const dimensionsOfBlock = useCallback( + (block: any): { width: number; height: number } => { 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, + 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) { + /* A note is not a card: it has no sub-block rows and no error row, so the + card estimate does not describe it. Its own height is what it was + measured at, or the height an empty one paints. */ + if (block.type === 'note') { 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), + width: BLOCK_DIMENSIONS.NOTE_WIDTH, + height: block.height || getNoteBlockHeight(true), } } - return estimateBlockDimensions(block.type) + return getBlockMetrics(block) }, - [blocks, isContainerType] + [isContainerType] + ) + + const getBlockDimensions = useCallback( + (blockId: string): { width: number; height: number } => dimensionsOfBlock(blocks[blockId]), + [blocks, dimensionsOfBlock] ) /** @@ -270,6 +288,12 @@ export function useNodeUtilities(blocks: Record) { /** * Calculates appropriate dimensions for a loop or parallel node based on its children + * + * Child heights come from {@link getBlockDimensions}, which estimates from + * block state when a card has not mounted yet and lands on the height it will + * render — so the size computed before the cards report matches the one after, + * and the container does not resize behind the user. + * * @param nodeId ID of the container node * @returns Calculated width and height for the container */ @@ -284,14 +308,21 @@ export function useNodeUtilities(blocks: Record) { .map((childId) => { const child = currentBlocks[childId] if (!child?.position) return null - const { width, height } = getBlockDimensions(childId) + /* Sized from `currentBlocks`, the same snapshot the position came + from. Reading dimensions off the hook's render snapshot instead + mixed two ages of the same store: `resizeLoopNodes` walks + deepest-first, so an inner container resized earlier in the pass + was already updated here but still old there, and the parent sized + against a stale inner box — leaving nested containers to converge + over a second pass. */ + const { width, height } = dimensionsOfBlock(child) return { x: child.position.x, y: child.position.y, width, height } }) - .filter((p): p is NonNullable => p !== null) + .filter((position): position is NonNullable => position !== null) return calculateContainerDimensions(childPositions) }, - [getBlockDimensions] + [dimensionsOfBlock] ) /** diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils.test.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils.test.ts new file mode 100644 index 00000000000..5cf044a9299 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils.test.ts @@ -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, + }) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils.ts index 8b93e302e06..7430229df48 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/node-position-utils.ts @@ -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 } diff --git a/packages/workflow-renderer/src/dimensions.ts b/packages/workflow-renderer/src/dimensions.ts index 8efc5098548..e35fb5cb08f 100644 --- a/packages/workflow-renderer/src/dimensions.ts +++ b/packages/workflow-renderer/src/dimensions.ts @@ -80,16 +80,36 @@ export const estimateNoteBlockHeight = (content: string) => { ) } +/** + * A container's box, and the gutter it keeps around the blocks inside it. + * + * The single source for both halves of that: the layout math that sizes a + * container and clamps its children, and the card's own DOM. `subflow-node-view` + * renders its header and content box straight from these, so the gap the + * geometry reserves is the gap the container actually paints. They used to be + * separate — the same four numbers as Tailwind literals in the view — and had + * already drifted, the view drawing a 40px header against a constant that + * claimed 50. + * + * Each padding is the gap between a child's edge and the container's, counted + * once: the header is accounted for by the child's own position, which + * `clampPositionToContainer` floors at `HEADER_HEIGHT + TOP_PADDING`. + * + * The two edges that carry chrome are wider than the two that are only gutter. + * The container's output handle sits on the right, and the resize grip in the + * bottom-right corner spans 40px in from both — a child at the gutter width + * would sit underneath it. + */ export const CONTAINER_DIMENSIONS = { DEFAULT_WIDTH: 500, DEFAULT_HEIGHT: 300, MIN_WIDTH: 400, MIN_HEIGHT: 200, - HEADER_HEIGHT: 50, - LEFT_PADDING: 16, + HEADER_HEIGHT: 40, + LEFT_PADDING: 24, RIGHT_PADDING: 80, - TOP_PADDING: 16, - BOTTOM_PADDING: 16, + TOP_PADDING: 24, + BOTTOM_PADDING: 80, } as const /** diff --git a/packages/workflow-renderer/src/subflow/subflow-node-view.tsx b/packages/workflow-renderer/src/subflow/subflow-node-view.tsx index b7d25588ae0..40fbafd140a 100644 --- a/packages/workflow-renderer/src/subflow/subflow-node-view.tsx +++ b/packages/workflow-renderer/src/subflow/subflow-node-view.tsx @@ -8,7 +8,7 @@ import { useStoreApi as useReactFlowStoreApi, useUpdateNodeInternals, } from 'reactflow' -import { BLOCK_DIMENSIONS, HANDLE_POSITIONS } from '../dimensions' +import { BLOCK_DIMENSIONS, CONTAINER_DIMENSIONS, HANDLE_POSITIONS } from '../dimensions' import { OverflowSpan } from '../lib/overflow-span' import type { DiffStatus } from '../types' import { @@ -588,8 +588,8 @@ export function SubflowNodeView({ aria-label={`Select ${blockName}`} onClick={onSelect} onKeyDown={(event) => handleKeyboardActivation(event, onSelect)} - className='workflow-drag-handle relative z-20 flex h-[40px] cursor-grab items-center justify-between px-2 [&:active]:cursor-grabbing' - style={{ pointerEvents: 'auto' }} + className='workflow-drag-handle relative z-20 flex cursor-grab items-center justify-between px-2 [&:active]:cursor-grabbing' + style={{ pointerEvents: 'auto', height: CONTAINER_DIMENSIONS.HEADER_HEIGHT }} data-subflow-header='' >
{ ) const header = host.querySelector('[data-subflow-header]') - expect(header).toHaveClass('h-[40px]') + /* Height comes from `CONTAINER_DIMENSIONS`, which the layout math also + measures against — asserting the rendered value rather than a utility + class keeps the two from drifting apart again. */ + expect(header).toHaveStyle({ height: `${CONTAINER_DIMENSIONS.HEADER_HEIGHT}px` }) expect(header).not.toHaveClass('border-b') expect(header).not.toHaveClass('bg-[var(--surface-2)]') expect(host.querySelector('[data-subflow-type-tag="loop"]')).toHaveTextContent('Loop') diff --git a/packages/workflow-renderer/vitest.setup.ts b/packages/workflow-renderer/vitest.setup.ts index 0c78bee3922..1468f99e728 100644 --- a/packages/workflow-renderer/vitest.setup.ts +++ b/packages/workflow-renderer/vitest.setup.ts @@ -1,3 +1,11 @@ +declare global { + /** + * React reads this to decide whether `act` is supported. It is not one of the + * ambient globals, so it has to be declared before it can be assigned. + */ + var IS_REACT_ACT_ENVIRONMENT: boolean +} + /** * jest-dom only registers DOM matchers (`toHaveStyle`, `toHaveClass`, …), so it is * dead weight outside a DOM environment. This package's mount tests opt into jsdom @@ -5,4 +13,11 @@ */ if (typeof document !== 'undefined') { await import('@testing-library/jest-dom/vitest') + /* + * Without this React treats `act` as unsupported, and every render in the + * mount tests logs "The current testing environment is not configured to + * support act(...)" — around forty lines a run, which buries the output that + * matters when one of them fails. + */ + globalThis.IS_REACT_ACT_ENVIRONMENT = true }