mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(autolayout): relocate notes that overlap blocks after layout (#4888)
* fix(autolayout): relocate notes that overlap blocks after layout * fix(autolayout): harden note overlap resolution against resize and non-finite positions
This commit is contained in:
@@ -63,10 +63,15 @@ export const OVERLAP_MARGIN = 30
|
||||
*/
|
||||
export const MAX_OVERLAP_ITERATIONS = 20
|
||||
|
||||
/**
|
||||
* Note block type identifier
|
||||
*/
|
||||
export const NOTE_BLOCK_TYPE = 'note'
|
||||
|
||||
/**
|
||||
* Block types excluded from autolayout
|
||||
*/
|
||||
export const AUTO_LAYOUT_EXCLUDED_TYPES = new Set(['note'])
|
||||
export const AUTO_LAYOUT_EXCLUDED_TYPES = new Set([NOTE_BLOCK_TYPE])
|
||||
|
||||
/**
|
||||
* Container block types that can have children
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
filterLayoutEligibleBlockIds,
|
||||
getBlocksByParent,
|
||||
prepareContainerDimensions,
|
||||
resolveNoteOverlaps,
|
||||
} from '@/lib/workflows/autolayout/utils'
|
||||
import type { BlockState } from '@/stores/workflows/workflow/types'
|
||||
|
||||
@@ -74,6 +75,8 @@ export function applyAutoLayout(
|
||||
|
||||
layoutContainers(blocksCopy, edges, options)
|
||||
|
||||
resolveNoteOverlaps(blocksCopy, verticalSpacing)
|
||||
|
||||
logger.info('Auto layout completed successfully', {
|
||||
blockCount: Object.keys(blocksCopy).length,
|
||||
})
|
||||
|
||||
@@ -11,7 +11,9 @@ import {
|
||||
filterLayoutEligibleBlockIds,
|
||||
getBlockMetrics,
|
||||
getBlocksByParent,
|
||||
hasFinitePosition,
|
||||
prepareContainerDimensions,
|
||||
resolveNoteOverlaps,
|
||||
shouldSkipAutoLayout,
|
||||
snapPositionToGrid,
|
||||
} from '@/lib/workflows/autolayout/utils'
|
||||
@@ -107,6 +109,10 @@ export function applyTargetedLayout(
|
||||
)
|
||||
}
|
||||
|
||||
// Relocate notes only where this pass introduced an overlap, comparing against
|
||||
// the original positions so pre-existing note arrangements are preserved.
|
||||
resolveNoteOverlaps(blocksCopy, verticalSpacing, { previousBlocks: blocks })
|
||||
|
||||
return blocksCopy
|
||||
}
|
||||
|
||||
@@ -184,7 +190,7 @@ function layoutGroup(
|
||||
const invalidPositions = layoutEligibleChildIds.filter((id) => {
|
||||
const block = blocks[id]
|
||||
if (!block) return false
|
||||
return !hasPosition(block)
|
||||
return !hasFinitePosition(block)
|
||||
})
|
||||
const needsLayoutSet = new Set([...requestedLayout, ...invalidPositions])
|
||||
const needsLayout = Array.from(needsLayoutSet)
|
||||
@@ -536,13 +542,3 @@ function updateContainerDimensions(
|
||||
measuredHeight: parentBlock.data.height,
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a block has a valid, finite position.
|
||||
* Returns false for missing, undefined, NaN, or Infinity coordinates.
|
||||
*/
|
||||
function hasPosition(block: BlockState): boolean {
|
||||
if (!block.position) return false
|
||||
const { x, y } = block.position
|
||||
return Number.isFinite(x) && Number.isFinite(y)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,238 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { DEFAULT_VERTICAL_SPACING } from '@/lib/workflows/autolayout/constants'
|
||||
import { resolveNoteOverlaps } from '@/lib/workflows/autolayout/utils'
|
||||
import type { BlockState } from '@/stores/workflows/workflow/types'
|
||||
|
||||
vi.mock('@/blocks', () => ({
|
||||
getBlock: () => null,
|
||||
}))
|
||||
|
||||
function createBlock(
|
||||
id: string,
|
||||
type: string,
|
||||
position: { x: number; y: number },
|
||||
overrides: Partial<BlockState> = {}
|
||||
): BlockState {
|
||||
return {
|
||||
id,
|
||||
type,
|
||||
name: id,
|
||||
position,
|
||||
subBlocks: {},
|
||||
outputs: {},
|
||||
enabled: true,
|
||||
layout: { measuredWidth: 250, measuredHeight: 120 },
|
||||
...overrides,
|
||||
} as BlockState
|
||||
}
|
||||
|
||||
describe('resolveNoteOverlaps', () => {
|
||||
it('relocates a note that overlaps a laid-out block', () => {
|
||||
const blocks: Record<string, BlockState> = {
|
||||
a: createBlock('a', 'agent', { x: 150, y: 150 }),
|
||||
note: createBlock(
|
||||
'note',
|
||||
'note',
|
||||
{ x: 160, y: 160 },
|
||||
{
|
||||
height: 120,
|
||||
layout: { measuredHeight: 120 },
|
||||
}
|
||||
),
|
||||
}
|
||||
|
||||
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING)
|
||||
|
||||
// Block is untouched; note is pushed below the block's bottom edge.
|
||||
expect(blocks.a.position).toEqual({ x: 150, y: 150 })
|
||||
expect(blocks.note.position.x).toBe(150)
|
||||
expect(blocks.note.position.y).toBeGreaterThanOrEqual(150 + 120 + DEFAULT_VERTICAL_SPACING - 1)
|
||||
})
|
||||
|
||||
it('leaves a note that does not overlap any block in place', () => {
|
||||
const blocks: Record<string, BlockState> = {
|
||||
a: createBlock('a', 'agent', { x: 150, y: 150 }),
|
||||
note: createBlock(
|
||||
'note',
|
||||
'note',
|
||||
{ x: 2000, y: 2000 },
|
||||
{
|
||||
height: 120,
|
||||
layout: { measuredHeight: 120 },
|
||||
}
|
||||
),
|
||||
}
|
||||
|
||||
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING)
|
||||
|
||||
expect(blocks.note.position).toEqual({ x: 2000, y: 2000 })
|
||||
})
|
||||
|
||||
it('stacks multiple overlapping notes without overlapping each other', () => {
|
||||
const blocks: Record<string, BlockState> = {
|
||||
a: createBlock('a', 'agent', { x: 150, y: 150 }),
|
||||
note1: createBlock(
|
||||
'note1',
|
||||
'note',
|
||||
{ x: 150, y: 150 },
|
||||
{
|
||||
height: 100,
|
||||
layout: { measuredHeight: 100 },
|
||||
}
|
||||
),
|
||||
note2: createBlock(
|
||||
'note2',
|
||||
'note',
|
||||
{ x: 200, y: 200 },
|
||||
{
|
||||
height: 100,
|
||||
layout: { measuredHeight: 100 },
|
||||
}
|
||||
),
|
||||
}
|
||||
|
||||
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING)
|
||||
|
||||
const n1 = blocks.note1.position
|
||||
const n2 = blocks.note2.position
|
||||
// Both relocated, stacked in reading order with no vertical overlap.
|
||||
expect(n2.y).toBeGreaterThanOrEqual(n1.y + 100)
|
||||
})
|
||||
|
||||
it('does nothing when there are no notes', () => {
|
||||
const blocks: Record<string, BlockState> = {
|
||||
a: createBlock('a', 'agent', { x: 150, y: 150 }),
|
||||
b: createBlock('b', 'agent', { x: 500, y: 150 }),
|
||||
}
|
||||
|
||||
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING)
|
||||
|
||||
expect(blocks.a.position).toEqual({ x: 150, y: 150 })
|
||||
expect(blocks.b.position).toEqual({ x: 500, y: 150 })
|
||||
})
|
||||
|
||||
it('never produces non-finite coordinates when a block has a NaN position', () => {
|
||||
const blocks: Record<string, BlockState> = {
|
||||
bad: createBlock('bad', 'agent', { x: Number.NaN, y: Number.NaN }),
|
||||
a: createBlock('a', 'agent', { x: 150, y: 150 }),
|
||||
note: createBlock(
|
||||
'note',
|
||||
'note',
|
||||
{ x: 150, y: 150 },
|
||||
{
|
||||
height: 120,
|
||||
layout: { measuredHeight: 120 },
|
||||
}
|
||||
),
|
||||
}
|
||||
|
||||
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING)
|
||||
|
||||
// The corrupted block is ignored; the note still relocates off block "a"
|
||||
// using only finite coordinates.
|
||||
expect(Number.isFinite(blocks.note.position.x)).toBe(true)
|
||||
expect(Number.isFinite(blocks.note.position.y)).toBe(true)
|
||||
expect(blocks.note.position.x).toBe(150)
|
||||
expect(blocks.note.position.y).toBeGreaterThan(150)
|
||||
})
|
||||
|
||||
describe('targeted mode (previousBlocks)', () => {
|
||||
it('relocates a note when a block was moved onto it', () => {
|
||||
const previousBlocks: Record<string, BlockState> = {
|
||||
a: createBlock('a', 'agent', { x: 2000, y: 2000 }),
|
||||
note: createBlock(
|
||||
'note',
|
||||
'note',
|
||||
{ x: 150, y: 150 },
|
||||
{
|
||||
height: 120,
|
||||
layout: { measuredHeight: 120 },
|
||||
}
|
||||
),
|
||||
}
|
||||
// Block "a" has been shifted onto the note by the layout pass.
|
||||
const blocks: Record<string, BlockState> = {
|
||||
a: createBlock('a', 'agent', { x: 150, y: 150 }),
|
||||
note: createBlock(
|
||||
'note',
|
||||
'note',
|
||||
{ x: 150, y: 150 },
|
||||
{
|
||||
height: 120,
|
||||
layout: { measuredHeight: 120 },
|
||||
}
|
||||
),
|
||||
}
|
||||
|
||||
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING, { previousBlocks })
|
||||
|
||||
expect(blocks.note.position.x).toBe(150)
|
||||
expect(blocks.note.position.y).toBeGreaterThan(150)
|
||||
})
|
||||
|
||||
it('preserves a pre-existing overlap not introduced by this pass', () => {
|
||||
// The note already overlapped block "a" before the pass; "a" did not move.
|
||||
const previousBlocks: Record<string, BlockState> = {
|
||||
a: createBlock('a', 'agent', { x: 150, y: 150 }),
|
||||
note: createBlock(
|
||||
'note',
|
||||
'note',
|
||||
{ x: 160, y: 160 },
|
||||
{
|
||||
height: 120,
|
||||
layout: { measuredHeight: 120 },
|
||||
}
|
||||
),
|
||||
}
|
||||
const blocks: Record<string, BlockState> = {
|
||||
a: createBlock('a', 'agent', { x: 150, y: 150 }),
|
||||
note: createBlock(
|
||||
'note',
|
||||
'note',
|
||||
{ x: 160, y: 160 },
|
||||
{
|
||||
height: 120,
|
||||
layout: { measuredHeight: 120 },
|
||||
}
|
||||
),
|
||||
}
|
||||
|
||||
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING, { previousBlocks })
|
||||
|
||||
expect(blocks.note.position).toEqual({ x: 160, y: 160 })
|
||||
})
|
||||
|
||||
it('relocates when a newly added block (no prior position) lands on a note', () => {
|
||||
const previousBlocks: Record<string, BlockState> = {
|
||||
note: createBlock(
|
||||
'note',
|
||||
'note',
|
||||
{ x: 150, y: 150 },
|
||||
{
|
||||
height: 120,
|
||||
layout: { measuredHeight: 120 },
|
||||
}
|
||||
),
|
||||
}
|
||||
const blocks: Record<string, BlockState> = {
|
||||
a: createBlock('a', 'agent', { x: 150, y: 150 }),
|
||||
note: createBlock(
|
||||
'note',
|
||||
'note',
|
||||
{ x: 150, y: 150 },
|
||||
{
|
||||
height: 120,
|
||||
layout: { measuredHeight: 120 },
|
||||
}
|
||||
),
|
||||
}
|
||||
|
||||
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING, { previousBlocks })
|
||||
|
||||
expect(blocks.note.position.y).toBeGreaterThan(150)
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
CONTAINER_PADDING,
|
||||
CONTAINER_PADDING_X,
|
||||
CONTAINER_PADDING_Y,
|
||||
NOTE_BLOCK_TYPE,
|
||||
ROOT_PADDING_X,
|
||||
ROOT_PADDING_Y,
|
||||
} from '@/lib/workflows/autolayout/constants'
|
||||
@@ -313,6 +314,158 @@ export function boxesOverlap(box1: BoundingBox, box2: BoundingBox, margin = 0):
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the on-canvas dimensions of a note block.
|
||||
* Notes are fixed-width and use a deterministic height, but fall back to any
|
||||
* stored measurement or data override when present.
|
||||
*/
|
||||
function getNoteDimensions(block: BlockState): { width: number; height: number } {
|
||||
const width = Math.max(
|
||||
resolveNumeric(block.data?.width, 0),
|
||||
block.layout?.measuredWidth ?? 0,
|
||||
BLOCK_DIMENSIONS.FIXED_WIDTH
|
||||
)
|
||||
|
||||
const defaultHeight =
|
||||
BLOCK_DIMENSIONS.HEADER_HEIGHT +
|
||||
BLOCK_DIMENSIONS.NOTE_CONTENT_PADDING +
|
||||
BLOCK_DIMENSIONS.NOTE_BASE_CONTENT_HEIGHT
|
||||
|
||||
const height = Math.max(
|
||||
resolveNumeric(block.data?.height, 0),
|
||||
block.layout?.measuredHeight ?? 0,
|
||||
block.height ?? 0,
|
||||
defaultHeight
|
||||
)
|
||||
|
||||
return { width, height }
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a block has a valid, finite position.
|
||||
* Returns false for missing, undefined, NaN, or Infinity coordinates.
|
||||
*/
|
||||
export function hasFinitePosition(block: BlockState): boolean {
|
||||
return Number.isFinite(block.position?.x) && Number.isFinite(block.position?.y)
|
||||
}
|
||||
|
||||
/**
|
||||
* Determines whether a note and a block overlapped at their pre-layout
|
||||
* positions. Used by targeted layout to distinguish overlaps introduced by the
|
||||
* current pass from arrangements that already existed.
|
||||
*/
|
||||
function noteOverlappedBlockBefore(
|
||||
previousBlocks: Record<string, BlockState>,
|
||||
noteId: string,
|
||||
blockId: string
|
||||
): boolean {
|
||||
const previousNote = previousBlocks[noteId]
|
||||
const previousBlock = previousBlocks[blockId]
|
||||
if (!previousNote || !previousBlock) return false
|
||||
|
||||
// A block without a finite prior position was not yet placed on the canvas,
|
||||
// so it could not have overlapped anything before this pass.
|
||||
if (!hasFinitePosition(previousNote) || !hasFinitePosition(previousBlock)) return false
|
||||
|
||||
// Derive dimensions from the prior blocks so a resize between passes does not
|
||||
// pair new dimensions with the old position.
|
||||
const noteBox = createBoundingBox(previousNote.position, getNoteDimensions(previousNote))
|
||||
const blockBox = createBoundingBox(previousBlock.position, getBlockMetrics(previousBlock))
|
||||
return boxesOverlap(blockBox, noteBox)
|
||||
}
|
||||
|
||||
export interface ResolveNoteOverlapsOptions {
|
||||
/**
|
||||
* Pre-layout block snapshot. When provided, only notes whose overlap with a
|
||||
* block was *introduced* by the current pass are relocated (i.e. they overlap
|
||||
* now but did not at these positions). Targeted layout passes this so that
|
||||
* pre-existing note arrangements are preserved. When omitted (full
|
||||
* auto-layout), any note overlapping a block is relocated.
|
||||
*/
|
||||
previousBlocks?: Record<string, BlockState>
|
||||
}
|
||||
|
||||
/**
|
||||
* Relocates note blocks that overlap the laid-out workflow blocks.
|
||||
*
|
||||
* Notes are excluded from the topological layout because they are free-form
|
||||
* annotations, but repositioned workflow blocks can land on top of them. This
|
||||
* pass keeps notes that already sit in clear space and stacks any relocated
|
||||
* notes in a column beneath their parent group's blocks, preserving the notes'
|
||||
* relative reading order. Notes are grouped by parent so notes inside a
|
||||
* container are resolved against that container's children only.
|
||||
*
|
||||
* Placement is always computed against the full set of blocks and notes in the
|
||||
* group, so a relocated note never collides with anything regardless of which
|
||||
* overlaps triggered the relocation.
|
||||
*/
|
||||
export function resolveNoteOverlaps(
|
||||
blocks: Record<string, BlockState>,
|
||||
verticalSpacing: number,
|
||||
options: ResolveNoteOverlapsOptions = {}
|
||||
): void {
|
||||
const { previousBlocks } = options
|
||||
const { root, children } = getBlocksByParent(blocks)
|
||||
const groups: string[][] = [root, ...Array.from(children.values())]
|
||||
|
||||
for (const groupIds of groups) {
|
||||
const obstacles: Array<{ id: string; box: BoundingBox }> = []
|
||||
const noteIds: string[] = []
|
||||
let maxBottom = Number.NEGATIVE_INFINITY
|
||||
let minX = Number.POSITIVE_INFINITY
|
||||
|
||||
for (const id of groupIds) {
|
||||
const block = blocks[id]
|
||||
// Skip non-finite positions so corrupted coordinates never propagate into
|
||||
// minX / maxBottom (and therefore into relocated note positions).
|
||||
if (!block || !hasFinitePosition(block)) continue
|
||||
|
||||
if (block.type === NOTE_BLOCK_TYPE) {
|
||||
noteIds.push(id)
|
||||
const { height } = getNoteDimensions(block)
|
||||
maxBottom = Math.max(maxBottom, block.position.y + height)
|
||||
continue
|
||||
}
|
||||
|
||||
if (shouldSkipAutoLayout(block)) continue
|
||||
|
||||
const box = createBoundingBox(block.position, getBlockMetrics(block))
|
||||
obstacles.push({ id, box })
|
||||
maxBottom = Math.max(maxBottom, box.y + box.height)
|
||||
minX = Math.min(minX, box.x)
|
||||
}
|
||||
|
||||
if (noteIds.length === 0 || obstacles.length === 0) continue
|
||||
|
||||
noteIds.sort((a, b) => {
|
||||
const posA = blocks[a].position
|
||||
const posB = blocks[b].position
|
||||
return posA.y - posB.y || posA.x - posB.x
|
||||
})
|
||||
|
||||
let stackY = maxBottom + verticalSpacing
|
||||
|
||||
for (const id of noteIds) {
|
||||
const note = blocks[id]
|
||||
const dimensions = getNoteDimensions(note)
|
||||
const noteBox = createBoundingBox(note.position, dimensions)
|
||||
|
||||
const needsRelocation = obstacles.some(({ id: blockId, box }) => {
|
||||
if (!boxesOverlap(box, noteBox)) return false
|
||||
if (previousBlocks) {
|
||||
return !noteOverlappedBlockBefore(previousBlocks, id, blockId)
|
||||
}
|
||||
return true
|
||||
})
|
||||
|
||||
if (!needsRelocation) continue
|
||||
|
||||
note.position = { x: minX, y: stackY }
|
||||
stackY += dimensions.height + verticalSpacing
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Groups blocks by their parent container
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user