From e93aef3871d3208f7df397ebc5888c0fc517a3fa Mon Sep 17 00:00:00 2001 From: Pouzor Date: Thu, 13 Aug 2026 00:49:51 +0200 Subject: [PATCH] fix(canvas): show alignment guides for nodes inside groups and containers Alignment guides and snapping were skipped for any node with a parentId, so dragging a node inside a group or a container_mode node (Proxmox, docker host) gave no visual help at all. Boxes are now expressed in absolute canvas coordinates by walking the parentId chain, so a child aligns against its siblings, its parent group, and top-level nodes outside the group. A node whose ancestor is also being dragged follows that ancestor: it is excluded from the snap (its parent-relative position would otherwise be shifted twice) and from the candidate set. Fixes #328 ha-relevant: yes --- .../__tests__/useAlignmentGuides.test.ts | 148 ++++++++++++++++++ frontend/src/hooks/useAlignmentGuides.ts | 78 +++++++-- 2 files changed, 211 insertions(+), 15 deletions(-) create mode 100644 frontend/src/hooks/__tests__/useAlignmentGuides.test.ts diff --git a/frontend/src/hooks/__tests__/useAlignmentGuides.test.ts b/frontend/src/hooks/__tests__/useAlignmentGuides.test.ts new file mode 100644 index 0000000..25df876 --- /dev/null +++ b/frontend/src/hooks/__tests__/useAlignmentGuides.test.ts @@ -0,0 +1,148 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { act, renderHook } from '@testing-library/react' +import type { Node } from '@xyflow/react' +import type { NodeData } from '@/types' +import { useAlignmentGuides } from '../useAlignmentGuides' + +const nodesRef: { current: Node[] } = { current: [] } +const setNodes = vi.fn() + +vi.mock('@xyflow/react', async () => (await import('@/test/mocks')).mockReactFlow({ + useReactFlow: () => ({ + getNodes: () => nodesRef.current, + setNodes, + }), +})) + +function makeFlowNode( + id: string, + x: number, + y: number, + over: Partial> = {}, +): Node { + return { + id, + type: 'custom', + position: { x, y }, + data: { label: id, type: 'server' } as NodeData, + measured: { width: 100, height: 50 }, + ...over, + } +} + +const evt = {} as MouseEvent + +describe('useAlignmentGuides — parented nodes', () => { + beforeEach(() => { + localStorage.clear() + setNodes.mockClear() + nodesRef.current = [] + }) + + it('emits guides for a node dragged inside a group (parent-relative coords)', () => { + // Group at (500, 500); two children in parent-relative coordinates. + // Child "b" sits 3px off "a" horizontally → within the default threshold. + nodesRef.current = [ + makeFlowNode('group', 500, 500, { type: 'group', measured: { width: 400, height: 400 } }), + makeFlowNode('a', 20, 20, { parentId: 'group' }), + makeFlowNode('b', 23, 200, { parentId: 'group' }), + ] + const { result } = renderHook(() => useAlignmentGuides()) + + act(() => { + result.current.onNodeDrag(evt, nodesRef.current[2], [nodesRef.current[2]]) + }) + + // Guides are in absolute canvas space: group.x + a.x = 520. + const xGuides = result.current.guides.filter((g) => g.axis === 'x') + expect(xGuides.length).toBeGreaterThan(0) + expect(xGuides.map((g) => g.position)).toContain(520) + }) + + it('snaps the dragged child on drag stop, keeping its parent-relative position', () => { + nodesRef.current = [ + makeFlowNode('group', 500, 500, { type: 'group', measured: { width: 400, height: 400 } }), + makeFlowNode('a', 20, 20, { parentId: 'group' }), + makeFlowNode('b', 23, 200, { parentId: 'group' }), + ] + const { result } = renderHook(() => useAlignmentGuides()) + + act(() => { + result.current.onNodeDrag(evt, nodesRef.current[2], [nodesRef.current[2]]) + result.current.onNodeDragStop(evt, nodesRef.current[2], [nodesRef.current[2]]) + }) + + expect(setNodes).toHaveBeenCalledTimes(1) + const updated = setNodes.mock.calls[0][0](nodesRef.current) as Node[] + // -3 applied to the relative position: 23 → 20, absolute 523 → 520. + expect(updated.find((n) => n.id === 'b')!.position.x).toBe(20) + expect(updated.find((n) => n.id === 'a')!.position.x).toBe(20) + }) + + it('aligns a child against a top-level node outside its group', () => { + nodesRef.current = [ + makeFlowNode('group', 500, 500, { type: 'group', measured: { width: 400, height: 400 } }), + makeFlowNode('outside', 520, 50), + makeFlowNode('child', 24, 100, { parentId: 'group' }), + ] + const { result } = renderHook(() => useAlignmentGuides()) + + act(() => { + result.current.onNodeDrag(evt, nodesRef.current[2], [nodesRef.current[2]]) + }) + + expect(result.current.guides.some((g) => g.axis === 'x' && g.position === 520)).toBe(true) + }) + + it('resolves nested container chains (container in a group)', () => { + nodesRef.current = [ + makeFlowNode('group', 100, 100, { type: 'group', measured: { width: 600, height: 600 } }), + makeFlowNode('host', 50, 50, { parentId: 'group', measured: { width: 300, height: 300 } }), + makeFlowNode('vm1', 10, 10, { parentId: 'host' }), + makeFlowNode('vm2', 14, 120, { parentId: 'host' }), + ] + const { result } = renderHook(() => useAlignmentGuides()) + + act(() => { + result.current.onNodeDrag(evt, nodesRef.current[3], [nodesRef.current[3]]) + }) + + // vm1 absolute x = 100 + 50 + 10 = 160. + expect(result.current.guides.some((g) => g.axis === 'x' && g.position === 160)).toBe(true) + }) + + it('does not snap a child whose parent is dragged too (no double shift)', () => { + nodesRef.current = [ + makeFlowNode('group', 497, 500, { type: 'group', measured: { width: 400, height: 400 } }), + makeFlowNode('child', 23, 20, { parentId: 'group' }), + makeFlowNode('other', 500, 1200), + ] + const { result } = renderHook(() => useAlignmentGuides()) + const dragged = [nodesRef.current[0], nodesRef.current[1]] + + act(() => { + result.current.onNodeDrag(evt, dragged[0], dragged) + result.current.onNodeDragStop(evt, dragged[0], dragged) + }) + + const updated = setNodes.mock.calls[0][0](nodesRef.current) as Node[] + // The group snaps by +3; the child follows it, so its relative x is untouched. + expect(updated.find((n) => n.id === 'group')!.position.x).toBe(500) + expect(updated.find((n) => n.id === 'child')!.position.x).toBe(23) + }) + + it('ignores nodes that follow the drag as alignment candidates', () => { + // Only the group is dragged; its child must not act as a candidate for itself. + nodesRef.current = [ + makeFlowNode('group', 500, 500, { type: 'group', measured: { width: 400, height: 400 } }), + makeFlowNode('child', 2, 20, { parentId: 'group' }), + ] + const { result } = renderHook(() => useAlignmentGuides()) + + act(() => { + result.current.onNodeDrag(evt, nodesRef.current[0], [nodesRef.current[0]]) + }) + + expect(result.current.guides).toHaveLength(0) + }) +}) diff --git a/frontend/src/hooks/useAlignmentGuides.ts b/frontend/src/hooks/useAlignmentGuides.ts index d60ed09..9c8a6d9 100644 --- a/frontend/src/hooks/useAlignmentGuides.ts +++ b/frontend/src/hooks/useAlignmentGuides.ts @@ -11,14 +11,54 @@ import type { NodeData } from '@/types' type NodeDrag = OnNodeDrag> -function nodeBox(n: Node): Box | null { - // Skip parented nodes — alignment between absolute and parent-relative - // coordinates is misleading. Top-level nodes only for v1. - if (n.parentId) return null +// Absolute canvas position of a node: React Flow stores a parented node's +// position relative to its parent, so we walk the parentId chain and sum. +// `seen` guards against a corrupted cycle in the hierarchy. +function absolutePosition( + n: Node, + byId: Map>, +): { x: number; y: number } { + let x = n.position.x + let y = n.position.y + const seen = new Set([n.id]) + let parentId = n.parentId + while (parentId && !seen.has(parentId)) { + seen.add(parentId) + const parent = byId.get(parentId) + if (!parent) break + x += parent.position.x + y += parent.position.y + parentId = parent.parentId + } + return { x, y } +} + +// Boxes are expressed in absolute canvas coordinates so a node inside a group +// or a container_mode node aligns against siblings *and* outside nodes. +function nodeBox(n: Node, byId: Map>): Box | null { const width = n.measured?.width ?? n.width ?? null const height = n.measured?.height ?? n.height ?? null if (width == null || height == null) return null - return { id: n.id, x: n.position.x, y: n.position.y, width, height } + const { x, y } = absolutePosition(n, byId) + return { id: n.id, x, y, width, height } +} + +// True when any ancestor of `n` is itself being dragged — such a node follows +// its parent, so it must not be snapped again (double-shift) nor act as an +// alignment candidate (it moves with the drag). +function hasDraggedAncestor( + n: Node, + byId: Map>, + draggedIds: Set, +): boolean { + const seen = new Set([n.id]) + let parentId = n.parentId + while (parentId && !seen.has(parentId)) { + if (draggedIds.has(parentId)) return true + seen.add(parentId) + parentId = byId.get(parentId)?.parentId + } + return false } /** @@ -74,27 +114,35 @@ export function useAlignmentGuides() { return } const all = getNodes() + const byId = new Map(all.map((n) => [n.id, n])) const draggedIds = new Set((dragNodes.length > 0 ? dragNodes : [dragNode]).map((n) => n.id)) - // Restrict the snap to top-level dragged nodes. nodeBox returns null for - // parented nodes; if we kept their ids in pendingSnap, onNodeDragStop - // would shift their parent-relative position by the same delta the parent - // already moves by, double-snapping the child. Children follow the parent - // automatically; no extra shift needed. + // A dragged node whose parent is also dragged already moves with that + // parent; snapping it too would shift its parent-relative position by the + // same delta twice. Its box still joins the union bbox (it is part of what + // the user sees moving), it just never lands in pendingSnap.ids. const draggedBoxEntries = all .filter((n) => draggedIds.has(n.id)) - .map((n) => ({ id: n.id, box: nodeBox(n) })) - .filter((e): e is { id: string; box: Box } => e.box !== null) + .map((n) => ({ + id: n.id, + box: nodeBox(n, byId), + follows: hasDraggedAncestor(n, byId, draggedIds), + })) + .filter((e): e is { id: string; box: Box; follows: boolean } => e.box !== null) if (draggedBoxEntries.length === 0) { clearState() return } - const snapIds = new Set(draggedBoxEntries.map((e) => e.id)) + const snapIds = new Set(draggedBoxEntries.filter((e) => !e.follows).map((e) => e.id)) + if (snapIds.size === 0) { + clearState() + return + } const boxes = draggedBoxEntries.map((e) => e.box) const dragged = boxes.length === 1 ? boxes[0] : unionBox(boxes) if (!dragged) return const candidates = all - .filter((n) => !draggedIds.has(n.id)) - .map(nodeBox) + .filter((n) => !draggedIds.has(n.id) && !hasDraggedAncestor(n, byId, draggedIds)) + .map((n) => nodeBox(n, byId)) .filter((b): b is Box => b !== null) const result = computeSnap(dragged, candidates, settings.threshold) setGuides(result.guides)