Skip to content

Commit 67019b0

Browse files
waleedlatif1waleedlatif1
committed
fix(autolayout): relocate notes that overlap blocks after layout
1 parent e761043 commit 67019b0

5 files changed

Lines changed: 379 additions & 12 deletions

File tree

‎apps/sim/lib/workflows/autolayout/constants.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,15 @@ export const OVERLAP_MARGIN = 30
6363
*/
6464
export const MAX_OVERLAP_ITERATIONS = 20
6565

66+
/**
67+
* Note block type identifier
68+
*/
69+
export const NOTE_BLOCK_TYPE = 'note'
70+
6671
/**
6772
* Block types excluded from autolayout
6873
*/
69-
export const AUTO_LAYOUT_EXCLUDED_TYPES = new Set(['note'])
74+
export const AUTO_LAYOUT_EXCLUDED_TYPES = new Set([NOTE_BLOCK_TYPE])
7075

7176
/**
7277
* Container block types that can have children

‎apps/sim/lib/workflows/autolayout/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
filterLayoutEligibleBlockIds,
1313
getBlocksByParent,
1414
prepareContainerDimensions,
15+
resolveNoteOverlaps,
1516
} from '@/lib/workflows/autolayout/utils'
1617
import type { BlockState } from '@/stores/workflows/workflow/types'
1718

@@ -74,6 +75,8 @@ export function applyAutoLayout(
7475

7576
layoutContainers(blocksCopy, edges, options)
7677

78+
resolveNoteOverlaps(blocksCopy, verticalSpacing)
79+
7780
logger.info('Auto layout completed successfully', {
7881
blockCount: Object.keys(blocksCopy).length,
7982
})

‎apps/sim/lib/workflows/autolayout/targeted.ts‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ import {
1111
filterLayoutEligibleBlockIds,
1212
getBlockMetrics,
1313
getBlocksByParent,
14+
hasFinitePosition,
1415
prepareContainerDimensions,
16+
resolveNoteOverlaps,
1517
shouldSkipAutoLayout,
1618
snapPositionToGrid,
1719
} from '@/lib/workflows/autolayout/utils'
@@ -107,6 +109,10 @@ export function applyTargetedLayout(
107109
)
108110
}
109111

112+
// Relocate notes only where this pass introduced an overlap, comparing against
113+
// the original positions so pre-existing note arrangements are preserved.
114+
resolveNoteOverlaps(blocksCopy, verticalSpacing, { previousBlocks: blocks })
115+
110116
return blocksCopy
111117
}
112118

@@ -184,7 +190,7 @@ function layoutGroup(
184190
const invalidPositions = layoutEligibleChildIds.filter((id) => {
185191
const block = blocks[id]
186192
if (!block) return false
187-
return !hasPosition(block)
193+
return !hasFinitePosition(block)
188194
})
189195
const needsLayoutSet = new Set([...requestedLayout, ...invalidPositions])
190196
const needsLayout = Array.from(needsLayoutSet)
@@ -536,13 +542,3 @@ function updateContainerDimensions(
536542
measuredHeight: parentBlock.data.height,
537543
}
538544
}
539-
540-
/**
541-
* Checks if a block has a valid, finite position.
542-
* Returns false for missing, undefined, NaN, or Infinity coordinates.
543-
*/
544-
function hasPosition(block: BlockState): boolean {
545-
if (!block.position) return false
546-
const { x, y } = block.position
547-
return Number.isFinite(x) && Number.isFinite(y)
548-
}

‎apps/sim/lib/workflows/autolayout/utils.test.ts‎

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it, vi } from 'vitest'
5+
import { DEFAULT_VERTICAL_SPACING } from '@/lib/workflows/autolayout/constants'
6+
import { resolveNoteOverlaps } from '@/lib/workflows/autolayout/utils'
7+
import type { BlockState } from '@/stores/workflows/workflow/types'
8+
9+
vi.mock('@/blocks', () => ({
10+
getBlock: () => null,
11+
}))
12+
13+
function createBlock(
14+
id: string,
15+
type: string,
16+
position: { x: number; y: number },
17+
overrides: Partial<BlockState> = {}
18+
): BlockState {
19+
return {
20+
id,
21+
type,
22+
name: id,
23+
position,
24+
subBlocks: {},
25+
outputs: {},
26+
enabled: true,
27+
layout: { measuredWidth: 250, measuredHeight: 120 },
28+
...overrides,
29+
} as BlockState
30+
}
31+
32+
describe('resolveNoteOverlaps', () => {
33+
it('relocates a note that overlaps a laid-out block', () => {
34+
const blocks: Record<string, BlockState> = {
35+
a: createBlock('a', 'agent', { x: 150, y: 150 }),
36+
note: createBlock(
37+
'note',
38+
'note',
39+
{ x: 160, y: 160 },
40+
{
41+
height: 120,
42+
layout: { measuredHeight: 120 },
43+
}
44+
),
45+
}
46+
47+
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING)
48+
49+
// Block is untouched; note is pushed below the block's bottom edge.
50+
expect(blocks.a.position).toEqual({ x: 150, y: 150 })
51+
expect(blocks.note.position.x).toBe(150)
52+
expect(blocks.note.position.y).toBeGreaterThanOrEqual(150 + 120 + DEFAULT_VERTICAL_SPACING - 1)
53+
})
54+
55+
it('leaves a note that does not overlap any block in place', () => {
56+
const blocks: Record<string, BlockState> = {
57+
a: createBlock('a', 'agent', { x: 150, y: 150 }),
58+
note: createBlock(
59+
'note',
60+
'note',
61+
{ x: 2000, y: 2000 },
62+
{
63+
height: 120,
64+
layout: { measuredHeight: 120 },
65+
}
66+
),
67+
}
68+
69+
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING)
70+
71+
expect(blocks.note.position).toEqual({ x: 2000, y: 2000 })
72+
})
73+
74+
it('stacks multiple overlapping notes without overlapping each other', () => {
75+
const blocks: Record<string, BlockState> = {
76+
a: createBlock('a', 'agent', { x: 150, y: 150 }),
77+
note1: createBlock(
78+
'note1',
79+
'note',
80+
{ x: 150, y: 150 },
81+
{
82+
height: 100,
83+
layout: { measuredHeight: 100 },
84+
}
85+
),
86+
note2: createBlock(
87+
'note2',
88+
'note',
89+
{ x: 200, y: 200 },
90+
{
91+
height: 100,
92+
layout: { measuredHeight: 100 },
93+
}
94+
),
95+
}
96+
97+
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING)
98+
99+
const n1 = blocks.note1.position
100+
const n2 = blocks.note2.position
101+
// Both relocated, stacked in reading order with no vertical overlap.
102+
expect(n2.y).toBeGreaterThanOrEqual(n1.y + 100)
103+
})
104+
105+
it('does nothing when there are no notes', () => {
106+
const blocks: Record<string, BlockState> = {
107+
a: createBlock('a', 'agent', { x: 150, y: 150 }),
108+
b: createBlock('b', 'agent', { x: 500, y: 150 }),
109+
}
110+
111+
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING)
112+
113+
expect(blocks.a.position).toEqual({ x: 150, y: 150 })
114+
expect(blocks.b.position).toEqual({ x: 500, y: 150 })
115+
})
116+
117+
describe('targeted mode (previousBlocks)', () => {
118+
it('relocates a note when a block was moved onto it', () => {
119+
const previousBlocks: Record<string, BlockState> = {
120+
a: createBlock('a', 'agent', { x: 2000, y: 2000 }),
121+
note: createBlock(
122+
'note',
123+
'note',
124+
{ x: 150, y: 150 },
125+
{
126+
height: 120,
127+
layout: { measuredHeight: 120 },
128+
}
129+
),
130+
}
131+
// Block "a" has been shifted onto the note by the layout pass.
132+
const blocks: Record<string, BlockState> = {
133+
a: createBlock('a', 'agent', { x: 150, y: 150 }),
134+
note: createBlock(
135+
'note',
136+
'note',
137+
{ x: 150, y: 150 },
138+
{
139+
height: 120,
140+
layout: { measuredHeight: 120 },
141+
}
142+
),
143+
}
144+
145+
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING, { previousBlocks })
146+
147+
expect(blocks.note.position.x).toBe(150)
148+
expect(blocks.note.position.y).toBeGreaterThan(150)
149+
})
150+
151+
it('preserves a pre-existing overlap not introduced by this pass', () => {
152+
// The note already overlapped block "a" before the pass; "a" did not move.
153+
const previousBlocks: Record<string, BlockState> = {
154+
a: createBlock('a', 'agent', { x: 150, y: 150 }),
155+
note: createBlock(
156+
'note',
157+
'note',
158+
{ x: 160, y: 160 },
159+
{
160+
height: 120,
161+
layout: { measuredHeight: 120 },
162+
}
163+
),
164+
}
165+
const blocks: Record<string, BlockState> = {
166+
a: createBlock('a', 'agent', { x: 150, y: 150 }),
167+
note: createBlock(
168+
'note',
169+
'note',
170+
{ x: 160, y: 160 },
171+
{
172+
height: 120,
173+
layout: { measuredHeight: 120 },
174+
}
175+
),
176+
}
177+
178+
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING, { previousBlocks })
179+
180+
expect(blocks.note.position).toEqual({ x: 160, y: 160 })
181+
})
182+
183+
it('relocates when a newly added block (no prior position) lands on a note', () => {
184+
const previousBlocks: Record<string, BlockState> = {
185+
note: createBlock(
186+
'note',
187+
'note',
188+
{ x: 150, y: 150 },
189+
{
190+
height: 120,
191+
layout: { measuredHeight: 120 },
192+
}
193+
),
194+
}
195+
const blocks: Record<string, BlockState> = {
196+
a: createBlock('a', 'agent', { x: 150, y: 150 }),
197+
note: createBlock(
198+
'note',
199+
'note',
200+
{ x: 150, y: 150 },
201+
{
202+
height: 120,
203+
layout: { measuredHeight: 120 },
204+
}
205+
),
206+
}
207+
208+
resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING, { previousBlocks })
209+
210+
expect(blocks.note.position.y).toBeGreaterThan(150)
211+
})
212+
})
213+
})

0 commit comments

Comments
 (0)