From b71c96897aa246188b78b33ad31762f2a27b4aae Mon Sep 17 00:00:00 2001 From: Pouzor Date: Fri, 29 May 2026 08:58:36 +0200 Subject: [PATCH] fix(canvas): make collapse reachable + persist for every node type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three connected bugs in PR #158's collapse feature: 1. Toggle wired to the wrong component The chevron was on GroupRectNode and computed children via React Flow parentId. But in this codebase parentId is set by createGroup() on type=group nodes, not on groupRect zones — zones are decorative rectangles. Result: childrenCount was always 0 on every zone and the button never rendered, so the feature was unreachable from the UI. Fix: - Add the same chevron toggle to GroupNode (the actual container). parentId children are already known there, so the existing BFS in computeCollapseInfo hides them when data.collapsed flips. - For GroupRectNode, switch childrenCount to spatial containment so drawn zones also work: hit-test other nodes' bbox centres against the zone bbox. 2. Visibility filter ignored spatial zones Extend computeCollapseInfo with a second pass that hides every node whose centre lies inside a collapsed groupRect, plus the parentId subtrees of those nodes (so a Proxmox host inside a collapsed zone takes its VMs/LXCs with it). Edge rewiring routes vanished endpoints to the same visible zone via a unified hiddenBy map populated by both passes. 3. Save dropped data.collapsed for every type except groupRect The DevTools payload was the smoking gun: for a type=group node the serializer wrote custom_colors: {show_border: true} with no collapsed key, so the backend stored a stale false on every save. Only the groupRect branch of serializeNode/deserializeApiNode stashed and hoisted the flag. Move the stash + hoist to the general branch too (backend's custom_colors is dict[str, Any] so no schema change). Tests: 11 new cases for spatial containment + GroupNode toggle UI, and 4 round-trip cases for collapse on non-groupRect types. --- .../src/components/canvas/CanvasContainer.tsx | 12 +- .../canvas/__tests__/GroupNode.test.tsx | 48 ++++ .../src/components/canvas/nodes/GroupNode.tsx | 27 +- .../components/canvas/nodes/GroupRectNode.tsx | 9 +- .../canvasSerializer.collapse.test.ts | 68 +++++ .../utils/__tests__/collapseFilter.test.ts | 244 ++++++++++++------ frontend/src/utils/canvasSerializer.ts | 17 +- frontend/src/utils/collapseFilter.ts | 206 +++++++++++---- 8 files changed, 486 insertions(+), 145 deletions(-) diff --git a/frontend/src/components/canvas/CanvasContainer.tsx b/frontend/src/components/canvas/CanvasContainer.tsx index 9bdcf83..be30641 100644 --- a/frontend/src/components/canvas/CanvasContainer.tsx +++ b/frontend/src/components/canvas/CanvasContainer.tsx @@ -17,7 +17,7 @@ import '@xyflow/react/dist/style.css' import { useCanvasStore } from '@/stores/canvasStore' import { useThemeStore } from '@/stores/themeStore' import { THEMES } from '@/utils/themes' -import { getVisibleNodeIds, rewireEdgesForCollapse } from '@/utils/collapseFilter' +import { computeCollapseInfo, rewireEdgesForCollapse } from '@/utils/collapseFilter' import { nodeTypes } from './nodes/nodeTypes' import { edgeTypes } from './edges/edgeTypes' import { SearchBar } from './SearchBar' @@ -57,14 +57,14 @@ export function CanvasContainer({ onConnect: onConnectProp, onEdgeDoubleClick, o const theme = THEMES[activeTheme] // Filter nodes and edges based on collapsed state (memoized — O(n)). - const visibleNodeIds = useMemo(() => getVisibleNodeIds(nodes), [nodes]) + const collapseInfo = useMemo(() => computeCollapseInfo(nodes), [nodes]) const visibleNodes = useMemo( - () => nodes.filter((n) => visibleNodeIds.has(n.id)), - [nodes, visibleNodeIds], + () => nodes.filter((n) => collapseInfo.visibleIds.has(n.id)), + [nodes, collapseInfo], ) const visibleEdges = useMemo( - () => rewireEdgesForCollapse(edges, nodes, visibleNodeIds), - [edges, nodes, visibleNodeIds], + () => rewireEdgesForCollapse(edges, nodes, collapseInfo.visibleIds, collapseInfo.hiddenBy), + [edges, nodes, collapseInfo], ) const onNodeClick = useCallback((e: React.MouseEvent, node: Node) => { diff --git a/frontend/src/components/canvas/__tests__/GroupNode.test.tsx b/frontend/src/components/canvas/__tests__/GroupNode.test.tsx index 926e4c8..b39f219 100644 --- a/frontend/src/components/canvas/__tests__/GroupNode.test.tsx +++ b/frontend/src/components/canvas/__tests__/GroupNode.test.tsx @@ -42,6 +42,7 @@ function renderGroupNode(props: Partial[0]> = {}, s nodes: storeNodes, updateNode: vi.fn(), snapshotHistory: vi.fn(), + toggleNodeCollapsed: vi.fn(), } as unknown as ReturnType) return render( @@ -134,4 +135,51 @@ describe('GroupNode', () => { renderGroupNode() expect(screen.queryByText(/●/)).toBeNull() }) + + it('renders a collapse toggle when the group has parentId children', () => { + const storeNodes = [ + { id: 'c1', parentId: 'g1', data: { status: 'online' } }, + { id: 'c2', parentId: 'g1', data: { status: 'online' } }, + ] + renderGroupNode({}, storeNodes) + expect(screen.getByTitle('Hide 2 items')).toBeDefined() + }) + + it('flips the toggle title when collapsed', () => { + const storeNodes = [ + { id: 'c1', parentId: 'g1', data: { status: 'online' } }, + ] + renderGroupNode({ data: makeGroupNode({ collapsed: true }).data }, storeNodes) + expect(screen.getByTitle('Show 1 hidden items')).toBeDefined() + }) + + it('calls toggleNodeCollapsed when the toggle is clicked', () => { + const toggleNodeCollapsed = vi.fn() + const storeNodes = [{ id: 'c1', parentId: 'g1', data: { status: 'online' } }] + vi.mocked(canvasStore.useCanvasStore).mockReturnValue({ + nodes: storeNodes, + updateNode: vi.fn(), + snapshotHistory: vi.fn(), + toggleNodeCollapsed, + } as unknown as ReturnType) + render( + , + ) + fireEvent.click(screen.getByTitle('Hide 1 items')) + expect(toggleNodeCollapsed).toHaveBeenCalledWith('g1') + }) + + it('does not render the toggle when the group has no children', () => { + renderGroupNode() + expect(screen.queryByTitle(/Hide.*items|Show.*hidden/)).toBeNull() + }) }) diff --git a/frontend/src/components/canvas/nodes/GroupNode.tsx b/frontend/src/components/canvas/nodes/GroupNode.tsx index 0b3c058..34fa0ce 100644 --- a/frontend/src/components/canvas/nodes/GroupNode.tsx +++ b/frontend/src/components/canvas/nodes/GroupNode.tsx @@ -1,13 +1,14 @@ import { useState } from 'react' import { type NodeProps, type Node, NodeResizer, Handle, Position } from '@xyflow/react' -import { Layers, Pencil, Check, X } from 'lucide-react' +import { Layers, Pencil, Check, X, ChevronDown } from 'lucide-react' import { useCanvasStore } from '@/stores/canvasStore' import { useThemeStore } from '@/stores/themeStore' import { THEMES } from '@/utils/themes' import { STATUS_COLORS, type NodeData } from '@/types' export function GroupNode({ id, data, selected }: NodeProps>) { - const { nodes, updateNode, snapshotHistory } = useCanvasStore() + const { nodes, updateNode, snapshotHistory, toggleNodeCollapsed } = useCanvasStore() + const isCollapsed = data.collapsed ?? false const activeTheme = useThemeStore((s) => s.activeTheme) const theme = THEMES[activeTheme] const showBorder = data.custom_colors?.show_border !== false @@ -138,6 +139,28 @@ export function GroupNode({ id, data, selected }: NodeProps>) { )} + {/* Collapse / expand toggle */} + {children.length > 0 && ( + + )} + {/* Status summary */} {children.length > 0 && (
diff --git a/frontend/src/components/canvas/nodes/GroupRectNode.tsx b/frontend/src/components/canvas/nodes/GroupRectNode.tsx index 113b01d..7a780a2 100644 --- a/frontend/src/components/canvas/nodes/GroupRectNode.tsx +++ b/frontend/src/components/canvas/nodes/GroupRectNode.tsx @@ -2,6 +2,7 @@ import { useState } from 'react' import { Handle, Position, NodeResizer, type NodeProps, type Node } from '@xyflow/react' import { ChevronDown } from 'lucide-react' import { useCanvasStore } from '@/stores/canvasStore' +import { getZoneSpatialChildren } from '@/utils/collapseFilter' import type { NodeData, TextPosition } from '@/types' const FONT_FAMILIES: Record = { @@ -54,8 +55,12 @@ export function GroupRectNode({ id, data, selected }: NodeProps>) const textPos = (rc.text_position ?? 'top-left') as TextPosition const posStyle = POSITION_STYLES[textPos] - // Count children for collapse badge - const childrenCount = (nodes ?? []).filter((n) => n.parentId === id).length + // Count children for collapse badge — groupRect zones don't parent their + // contents via React Flow parentId, so we hit-test by spatial containment. + const selfNode = (nodes ?? []).find((n) => n.id === id) + const childrenCount = selfNode + ? getZoneSpatialChildren(selfNode, nodes ?? []).length + : 0 const outsideJustify = textPos.includes('right') ? 'flex-end' : (textPos.includes('center') || textPos === 'center') ? 'center' diff --git a/frontend/src/utils/__tests__/canvasSerializer.collapse.test.ts b/frontend/src/utils/__tests__/canvasSerializer.collapse.test.ts index 03aca1d..7b5b6f2 100644 --- a/frontend/src/utils/__tests__/canvasSerializer.collapse.test.ts +++ b/frontend/src/utils/__tests__/canvasSerializer.collapse.test.ts @@ -81,3 +81,71 @@ describe('canvasSerializer — groupRect collapse', () => { expect(back.data.collapsed).toBe(true) }) }) + +describe('canvasSerializer — collapse on non-groupRect node types', () => { + it('stashes data.collapsed into custom_colors for a group container', () => { + const rf: Node = { + id: 'g1', + type: 'group', + position: { x: 0, y: 0 }, + data: { + label: 'Container', + type: 'group', + status: 'unknown', + services: [], + custom_colors: { show_border: true }, + collapsed: true, + }, + } + const api = serializeNode(rf) as Record + const cc = api.custom_colors as Record + expect(cc.collapsed).toBe(true) + // Existing custom_colors keys are preserved alongside the stash. + expect(cc.show_border).toBe(true) + }) + + it('leaves custom_colors null when neither flag nor colors are set', () => { + const rf: Node = { + id: 's1', + type: 'server', + position: { x: 0, y: 0 }, + data: { label: 'Server', type: 'server', status: 'online', services: [] }, + } + const api = serializeNode(rf) as Record + expect(api.custom_colors).toBeNull() + }) + + it('hoists custom_colors.collapsed to data.collapsed for a group container', () => { + const apiNode: ApiNode = { + id: 'g1', + type: 'group', + label: 'Container', + pos_x: 0, + pos_y: 0, + status: 'unknown', + services: [], + custom_colors: { show_border: true, collapsed: true }, + } + const rf = deserializeApiNode(apiNode, new Map()) + expect(rf.data.collapsed).toBe(true) + }) + + it('round-trips collapse on a group container', () => { + const rf: Node = { + id: 'g1', + type: 'group', + position: { x: 0, y: 0 }, + data: { + label: 'Container', + type: 'group', + status: 'unknown', + services: [], + custom_colors: { show_border: true }, + collapsed: true, + }, + } + const api = serializeNode(rf) as unknown as ApiNode + const back = deserializeApiNode(api, new Map()) + expect(back.data.collapsed).toBe(true) + }) +}) diff --git a/frontend/src/utils/__tests__/collapseFilter.test.ts b/frontend/src/utils/__tests__/collapseFilter.test.ts index c91eae6..aa2fe54 100644 --- a/frontend/src/utils/__tests__/collapseFilter.test.ts +++ b/frontend/src/utils/__tests__/collapseFilter.test.ts @@ -1,22 +1,38 @@ import { describe, it, expect } from 'vitest' import type { Edge, Node } from '@xyflow/react' -import { getVisibleNodeIds, rewireEdgesForCollapse } from '../collapseFilter' +import { + getVisibleNodeIds, + rewireEdgesForCollapse, + getZoneSpatialChildren, + computeCollapseInfo, +} from '../collapseFilter' import type { EdgeData, NodeData } from '@/types' -const mkNode = ( - id: string, - parentId?: string, - collapsed?: boolean, -): Node => ({ +interface MkOpts { + parentId?: string + collapsed?: boolean + position?: { x: number; y: number } + width?: number + height?: number + type?: NodeData['type'] +} + +// Outside the default 360x240 zone bbox at origin — used by tests that need +// a node that must NOT be spatially captured by a collapsed zone. +const FAR = { x: 10000, y: 0 } + +const mkNode = (id: string, opts: MkOpts = {}): Node => ({ id, - position: { x: 0, y: 0 }, - ...(parentId ? { parentId } : {}), + position: opts.position ?? { x: 0, y: 0 }, + ...(opts.width !== undefined ? { width: opts.width } : {}), + ...(opts.height !== undefined ? { height: opts.height } : {}), + ...(opts.parentId ? { parentId: opts.parentId } : {}), data: { label: id, - type: parentId ? 'server' : 'groupRect', + type: opts.type ?? (opts.parentId ? 'server' : 'groupRect'), status: 'online', services: [], - ...(collapsed !== undefined ? { collapsed } : {}), + ...(opts.collapsed !== undefined ? { collapsed: opts.collapsed } : {}), }, }) @@ -26,121 +42,193 @@ const mkEdge = (id: string, source: string, target: string): Edge => ( target, }) -describe('getVisibleNodeIds', () => { +describe('getVisibleNodeIds — parentId cascade', () => { it('returns all nodes when nothing is collapsed', () => { const nodes = [ mkNode('zone'), - mkNode('child-a', 'zone'), - mkNode('child-b', 'zone'), + mkNode('child-a', { parentId: 'zone' }), + mkNode('child-b', { parentId: 'zone' }), ] expect(getVisibleNodeIds(nodes)).toEqual(new Set(['zone', 'child-a', 'child-b'])) }) - it('hides direct children of a collapsed zone but keeps the zone itself', () => { + it('hides direct children of a collapsed parent but keeps the parent itself', () => { const nodes = [ - mkNode('zone', undefined, true), - mkNode('child-a', 'zone'), - mkNode('child-b', 'zone'), - mkNode('outside'), + mkNode('zone', { collapsed: true }), + mkNode('child-a', { parentId: 'zone' }), + mkNode('child-b', { parentId: 'zone' }), + mkNode('outside', { position: FAR }), ] expect(getVisibleNodeIds(nodes)).toEqual(new Set(['zone', 'outside'])) }) it('hides the entire subtree when an ancestor is collapsed (multi-level)', () => { const nodes = [ - mkNode('root', undefined, true), - mkNode('mid', 'root', false), // expanded but parent collapsed → still hidden - mkNode('leaf', 'mid'), + mkNode('root', { collapsed: true }), + mkNode('mid', { parentId: 'root', collapsed: false }), + mkNode('leaf', { parentId: 'mid' }), ] - const visible = getVisibleNodeIds(nodes) - expect(visible.has('root')).toBe(true) - expect(visible.has('mid')).toBe(false) - expect(visible.has('leaf')).toBe(false) + const v = getVisibleNodeIds(nodes) + expect(v.has('root')).toBe(true) + expect(v.has('mid')).toBe(false) + expect(v.has('leaf')).toBe(false) }) it('hides only the nested subtree when an inner zone is collapsed', () => { const nodes = [ - mkNode('root', undefined, false), - mkNode('inner', 'root', true), - mkNode('leaf', 'inner'), - mkNode('sibling', 'root'), + mkNode('root', { collapsed: false }), + mkNode('inner', { parentId: 'root', collapsed: true }), + mkNode('leaf', { parentId: 'inner' }), + mkNode('sibling', { parentId: 'root' }), ] expect(getVisibleNodeIds(nodes)).toEqual(new Set(['root', 'inner', 'sibling'])) }) it('handles a zone with no children', () => { - const nodes = [mkNode('empty-zone', undefined, true)] - expect(getVisibleNodeIds(nodes)).toEqual(new Set(['empty-zone'])) + expect(getVisibleNodeIds([mkNode('empty-zone', { collapsed: true })])) + .toEqual(new Set(['empty-zone'])) }) it('returns an empty set for empty input', () => { expect(getVisibleNodeIds([])).toEqual(new Set()) }) - it('treats nodes with no custom_colors as expanded', () => { - const nodes = [mkNode('zone'), mkNode('child', 'zone')] + it('treats nodes with no collapsed flag as expanded', () => { + const nodes = [mkNode('zone'), mkNode('child', { parentId: 'zone' })] expect(getVisibleNodeIds(nodes)).toEqual(new Set(['zone', 'child'])) }) it('is independent of insertion order (children declared before parent)', () => { const nodes = [ - mkNode('child', 'zone'), - mkNode('zone', undefined, true), + mkNode('child', { parentId: 'zone' }), + mkNode('zone', { collapsed: true }), ] expect(getVisibleNodeIds(nodes)).toEqual(new Set(['zone'])) }) }) +describe('getZoneSpatialChildren', () => { + it('picks up top-level nodes whose centre lies inside the zone bbox', () => { + const zone = mkNode('zone', { position: { x: 0, y: 0 }, width: 400, height: 300 }) + const inside = mkNode('inside', { position: { x: 100, y: 50 }, type: 'server' }) + const outside = mkNode('outside', { position: { x: 500, y: 0 }, type: 'server' }) + expect(getZoneSpatialChildren(zone, [zone, inside, outside])).toEqual(['inside']) + }) + + it('ignores the zone itself', () => { + const zone = mkNode('zone', { width: 400, height: 300 }) + expect(getZoneSpatialChildren(zone, [zone])).toEqual([]) + }) + + it('ignores nodes with a parentId (handled via parentId cascade)', () => { + const zone = mkNode('zone', { width: 400, height: 300 }) + const child = mkNode('child', { parentId: 'other', type: 'server' }) + expect(getZoneSpatialChildren(zone, [zone, child])).toEqual([]) + }) + + it('uses fallback dimensions for nodes with no width/height set', () => { + const zone = mkNode('zone', { width: 400, height: 300 }) + // No width/height → defaults (200, 80). Centre at (100, 40), inside. + const n = mkNode('n', { type: 'server' }) + expect(getZoneSpatialChildren(zone, [zone, n])).toEqual(['n']) + }) +}) + +describe('computeCollapseInfo — spatial collapse via groupRect zones', () => { + it('hides nodes spatially inside a collapsed zone and records hiddenBy', () => { + const zone = mkNode('zone', { collapsed: true, width: 400, height: 300 }) + const inside = mkNode('inside', { position: { x: 50, y: 50 }, type: 'server' }) + const outside = mkNode('outside', { position: FAR, type: 'server' }) + const info = computeCollapseInfo([zone, inside, outside]) + expect(info.visibleIds).toEqual(new Set(['zone', 'outside'])) + expect(info.hiddenBy.get('inside')).toBe('zone') + }) + + it('cascades parentId descendants of spatially-hidden nodes', () => { + // Proxmox host sitting inside a collapsed zone — its VMs (parentId) + // must also be hidden even though they live at relative coords. + const zone = mkNode('zone', { collapsed: true, width: 400, height: 300 }) + const px = mkNode('px', { position: { x: 50, y: 50 }, type: 'proxmox' }) + const vm = mkNode('vm', { parentId: 'px', type: 'vm' }) + const info = computeCollapseInfo([zone, px, vm]) + expect(info.visibleIds).toEqual(new Set(['zone'])) + expect(info.hiddenBy.get('vm')).toBe('zone') + }) + + it('a nested groupRect inside a collapsed outer zone is also hidden', () => { + const outer = mkNode('outer', { collapsed: true, width: 600, height: 400 }) + const inner = mkNode('inner', { position: { x: 100, y: 100 }, width: 200, height: 150 }) + const leaf = mkNode('leaf', { position: { x: 150, y: 150 }, type: 'server' }) + const info = computeCollapseInfo([outer, inner, leaf]) + expect(info.visibleIds).toEqual(new Set(['outer'])) + }) + + it('does not affect nodes outside every collapsed zone', () => { + const a = mkNode('a', { collapsed: true, width: 300, height: 200 }) + const b = mkNode('b', { position: { x: 1000, y: 1000 }, width: 300, height: 200 }) + const free = mkNode('free', { position: { x: 2000, y: 2000 }, type: 'server' }) + const info = computeCollapseInfo([a, b, free]) + expect(info.visibleIds.has('free')).toBe(true) + }) +}) + describe('rewireEdgesForCollapse', () => { it('keeps edges between two visible nodes unchanged (same reference)', () => { - const nodes = [mkNode('a'), mkNode('b')] + const nodes = [mkNode('a'), mkNode('b', { position: FAR })] const edges = [mkEdge('e1', 'a', 'b')] - const out = rewireEdgesForCollapse(edges, nodes, new Set(['a', 'b'])) + const info = computeCollapseInfo(nodes) + const out = rewireEdgesForCollapse(edges, nodes, info.visibleIds, info.hiddenBy) expect(out).toHaveLength(1) expect(out[0]).toBe(edges[0]) }) - it('reroutes a cross-boundary edge to the collapsed ancestor', () => { + it('reroutes a cross-boundary edge to the collapsed parentId ancestor', () => { const nodes = [ - mkNode('zone', undefined, true), - mkNode('leaf', 'zone'), - mkNode('outside'), + mkNode('zone', { collapsed: true }), + mkNode('leaf', { parentId: 'zone' }), + mkNode('outside', { position: FAR }), ] - const visible = getVisibleNodeIds(nodes) + const info = computeCollapseInfo(nodes) const edges = [mkEdge('e1', 'outside', 'leaf')] - const out = rewireEdgesForCollapse(edges, nodes, visible) - expect(out).toHaveLength(1) + const out = rewireEdgesForCollapse(edges, nodes, info.visibleIds, info.hiddenBy) expect(out[0].source).toBe('outside') expect(out[0].target).toBe('zone') - // Handle hints stripped when the endpoint moved. expect(out[0].sourceHandle).toBeNull() expect(out[0].targetHandle).toBeNull() }) - it('drops an edge between two siblings inside the same collapsed zone (self-loop)', () => { - const nodes = [ - mkNode('zone', undefined, true), - mkNode('a', 'zone'), - mkNode('b', 'zone'), - ] - const visible = getVisibleNodeIds(nodes) - const edges = [mkEdge('e1', 'a', 'b')] - expect(rewireEdgesForCollapse(edges, nodes, visible)).toEqual([]) + it('reroutes a cross-boundary edge to a collapsed groupRect zone (spatial)', () => { + const zone = mkNode('zone', { collapsed: true, width: 400, height: 300 }) + const inside = mkNode('inside', { position: { x: 50, y: 50 }, type: 'server' }) + const outside = mkNode('outside', { position: FAR, type: 'server' }) + const nodes = [zone, inside, outside] + const info = computeCollapseInfo(nodes) + const edges = [mkEdge('e1', 'outside', 'inside')] + const out = rewireEdgesForCollapse(edges, nodes, info.visibleIds, info.hiddenBy) + expect(out[0].source).toBe('outside') + expect(out[0].target).toBe('zone') }) - it('de-dupes multiple cross-boundary edges that rewire to the same pair', () => { - // 20-device Zigbee mesh: many edges from outside coordinator to leaves - // inside a collapsed zone should collapse to a single stub. + it('drops an edge between two siblings inside the same collapsed zone (self-loop)', () => { const nodes = [ - mkNode('zone', undefined, true), - mkNode('coord'), - ...Array.from({ length: 5 }, (_, i) => mkNode(`leaf-${i}`, 'zone')), + mkNode('zone', { collapsed: true }), + mkNode('a', { parentId: 'zone' }), + mkNode('b', { parentId: 'zone' }), ] - const visible = getVisibleNodeIds(nodes) - const edges = Array.from({ length: 5 }, (_, i) => - mkEdge(`e-${i}`, 'coord', `leaf-${i}`), - ) - const out = rewireEdgesForCollapse(edges, nodes, visible) + const info = computeCollapseInfo(nodes) + expect(rewireEdgesForCollapse([mkEdge('e1', 'a', 'b')], nodes, info.visibleIds, info.hiddenBy)) + .toEqual([]) + }) + + it('de-dupes parallel cross-boundary edges that rewire to the same pair', () => { + const nodes = [ + mkNode('zone', { collapsed: true }), + mkNode('coord', { position: FAR }), + ...Array.from({ length: 5 }, (_, i) => mkNode(`leaf-${i}`, { parentId: 'zone' })), + ] + const info = computeCollapseInfo(nodes) + const edges = Array.from({ length: 5 }, (_, i) => mkEdge(`e-${i}`, 'coord', `leaf-${i}`)) + const out = rewireEdgesForCollapse(edges, nodes, info.visibleIds, info.hiddenBy) expect(out).toHaveLength(1) expect(out[0].source).toBe('coord') expect(out[0].target).toBe('zone') @@ -148,25 +236,27 @@ describe('rewireEdgesForCollapse', () => { it('walks the chain to the nearest visible ancestor (nested collapse)', () => { const nodes = [ - mkNode('root', undefined, true), - mkNode('mid', 'root'), - mkNode('leaf', 'mid'), - mkNode('outside'), + mkNode('root', { collapsed: true }), + mkNode('mid', { parentId: 'root' }), + mkNode('leaf', { parentId: 'mid' }), + mkNode('outside', { position: FAR }), ] - const visible = getVisibleNodeIds(nodes) - const edges = [mkEdge('e1', 'outside', 'leaf')] - const out = rewireEdgesForCollapse(edges, nodes, visible) + const info = computeCollapseInfo(nodes) + const out = rewireEdgesForCollapse( + [mkEdge('e1', 'outside', 'leaf')], + nodes, + info.visibleIds, + info.hiddenBy, + ) expect(out[0].target).toBe('root') }) it('drops an edge whose endpoint has no visible ancestor', () => { - const nodes = [mkNode('orphan-parent', undefined, true)] - const visible = new Set() // nothing visible at all - const edges = [mkEdge('e1', 'ghost', 'orphan-parent')] - expect(rewireEdgesForCollapse(edges, nodes, visible)).toEqual([]) + const edges = [mkEdge('e1', 'ghost', 'also-ghost')] + expect(rewireEdgesForCollapse(edges, [], new Set(), new Map())).toEqual([]) }) it('returns an empty array for empty input', () => { - expect(rewireEdgesForCollapse([], [], new Set())).toEqual([]) + expect(rewireEdgesForCollapse([], [], new Set(), new Map())).toEqual([]) }) }) diff --git a/frontend/src/utils/canvasSerializer.ts b/frontend/src/utils/canvasSerializer.ts index 78d678c..435e06b 100644 --- a/frontend/src/utils/canvasSerializer.ts +++ b/frontend/src/utils/canvasSerializer.ts @@ -97,7 +97,13 @@ export function serializeNode(n: Node): Record { notes: n.data.notes ?? null, parent_id: n.data.parent_id ?? null, container_mode: n.data.container_mode ?? false, - custom_colors: n.data.custom_colors ?? null, + // Stash collapse state inside the custom_colors blob so the backend's + // dict[str, Any] column carries it without a schema change. Hoisted + // back to `data.collapsed` on load. Applies to every node type — group + // containers, Proxmox hosts, etc. — not just groupRect zones. + custom_colors: n.data.collapsed !== undefined + ? { ...(n.data.custom_colors ?? {}), collapsed: n.data.collapsed } + : (n.data.custom_colors ?? null), custom_icon: n.data.custom_icon ?? null, cpu_count: n.data.cpu_count ?? null, cpu_model: n.data.cpu_model ?? null, @@ -162,7 +168,14 @@ export function deserializeApiNode( id: n.id, type: normalizedType, position: { x: n.pos_x, y: n.pos_y }, - data: { ...n, type: normalizedType, bottom_handles: clampBottomHandles(n.bottom_handles ?? 1) } as unknown as NodeData, + // Hoist persisted collapse flag from the custom_colors stash (matches + // the symmetric serialize step). Applies to every node type. + data: { + ...n, + type: normalizedType, + bottom_handles: clampBottomHandles(n.bottom_handles ?? 1), + collapsed: Boolean(n.custom_colors?.collapsed), + } as unknown as NodeData, ...(n.parent_id && parentIsContainer ? { parentId: n.parent_id, extent: 'parent' as const } : {}), ...(['proxmox', 'vm', 'lxc', 'docker_host'].includes(normalizedType) && n.container_mode !== false ? { width: n.width ?? 300, height: n.height ?? 200 } diff --git a/frontend/src/utils/collapseFilter.ts b/frontend/src/utils/collapseFilter.ts index 8db0659..ed66b5a 100644 --- a/frontend/src/utils/collapseFilter.ts +++ b/frontend/src/utils/collapseFilter.ts @@ -2,80 +2,176 @@ import type { Edge, Node } from '@xyflow/react' import type { EdgeData, NodeData } from '@/types' /** - * Compute the set of node IDs that should be visible on the canvas given the - * current collapse state of group/zone nodes. + * Collapse model + * ────────────── + * Two ways a node can collapse and hide what it "contains": * - * A node is hidden if any ancestor (via `parentId`) has - * `data.custom_colors.collapsed === true`. Root nodes (no `parentId`) are - * always visible. + * 1. parentId hierarchy — `type: 'group'` containers (createGroup) and + * Proxmox container_mode children. Setting `data.collapsed = true` on + * such a node hides every node in its parentId subtree. * - * O(n) — builds a `parentId -> children[]` index once, then BFS from roots. + * 2. Spatial containment — `type: 'groupRect'` decorative zones drawn + * around nodes. Zones do not parent their contents in React Flow, so + * we hit-test every top-level node's centre against the zone bbox to + * decide what is "inside". Collapsing a zone hides every node whose + * centre lies inside the zone (plus the parentId subtrees of those + * nodes, so e.g. a Proxmox host inside a collapsed zone also takes its + * VMs/LXCs with it). + * + * `hiddenBy` records which collapsed ancestor hid each node — used by edge + * rewiring to redirect a vanished endpoint to the visible zone the user is + * actually looking at. */ -export function getVisibleNodeIds(nodes: Node[]): Set { - const childrenByParent = new Map() - for (const n of nodes) { - if (n.parentId) { - const arr = childrenByParent.get(n.parentId) - if (arr) arr.push(n.id) - else childrenByParent.set(n.parentId, [n.id]) - } + +interface BBox { x: number; y: number; w: number; h: number } + +const DEFAULT_NODE_W = 200 +const DEFAULT_NODE_H = 80 +const DEFAULT_ZONE_W = 360 +const DEFAULT_ZONE_H = 240 + +function bboxOf(n: Node, fallbackW: number, fallbackH: number): BBox { + return { + x: n.position.x, + y: n.position.y, + w: n.width ?? fallbackW, + h: n.height ?? fallbackH, } +} - // Fast lookup for collapse flag. - const byId = new Map>() - for (const n of nodes) byId.set(n.id, n) - - const visible = new Set() - const queue: string[] = [] - for (const n of nodes) { - if (!n.parentId) queue.push(n.id) - } - - while (queue.length > 0) { - const id = queue.shift()! - visible.add(id) - const node = byId.get(id) - if (node && !node.data.collapsed) { - const children = childrenByParent.get(id) - if (children) queue.push(...children) - } - } - - return visible +function centerInside(n: Node, b: BBox): boolean { + const w = n.width ?? DEFAULT_NODE_W + const h = n.height ?? DEFAULT_NODE_H + const cx = n.position.x + w / 2 + const cy = n.position.y + h / 2 + return cx >= b.x && cx <= b.x + b.w && cy >= b.y && cy <= b.y + b.h } /** - * Rewire edges so that any endpoint inside a collapsed subtree is replaced - * with the nearest visible ancestor (the collapsed zone the user actually - * sees). Behaviour: + * Node ids whose centre lies inside the given zone, excluding the zone + * itself and any node that is a React Flow child (parentId set — those are + * positioned relative to their parent, not in absolute canvas coordinates). + */ +export function getZoneSpatialChildren( + zone: Node, + nodes: Node[], +): string[] { + const zb = bboxOf(zone, DEFAULT_ZONE_W, DEFAULT_ZONE_H) + const out: string[] = [] + for (const n of nodes) { + if (n.id === zone.id) continue + if (n.parentId) continue + if (centerInside(n, zb)) out.push(n.id) + } + return out +} + +function buildChildrenByParent(nodes: Node[]): Map { + const m = new Map() + for (const n of nodes) { + if (!n.parentId) continue + const arr = m.get(n.parentId) + if (arr) arr.push(n.id) + else m.set(n.parentId, [n.id]) + } + return m +} + +export interface CollapseInfo { + /** Ids the canvas should render. */ + visibleIds: Set + /** For each hidden id, the id of the collapsed ancestor that hid it. */ + hiddenBy: Map +} + +/** + * Single source of truth for visibility under collapse. O(n) over nodes + * (the spatial pass is O(z·n) where z is the number of collapsed zones). + */ +export function computeCollapseInfo(nodes: Node[]): CollapseInfo { + const childrenByParent = buildChildrenByParent(nodes) + const hidden = new Set() + const hiddenBy = new Map() + + const hideSubtree = (rootId: string, hider: string) => { + const queue = [...(childrenByParent.get(rootId) ?? [])] + while (queue.length > 0) { + const id = queue.shift()! + if (hidden.has(id)) continue + hidden.add(id) + if (!hiddenBy.has(id)) hiddenBy.set(id, hider) + const sub = childrenByParent.get(id) + if (sub) queue.push(...sub) + } + } + + // Pass 1 — parentId-based collapse (real containers). + for (const n of nodes) { + if (n.data.collapsed) hideSubtree(n.id, n.id) + } + + // Pass 2 — spatial collapse (groupRect zones). + for (const n of nodes) { + if (n.data.type !== 'groupRect') continue + if (!n.data.collapsed) continue + const contained = getZoneSpatialChildren(n, nodes) + for (const id of contained) { + if (!hidden.has(id)) { + hidden.add(id) + if (!hiddenBy.has(id)) hiddenBy.set(id, n.id) + } + hideSubtree(id, n.id) + } + } + + const visibleIds = new Set() + for (const n of nodes) { + if (!hidden.has(n.id)) visibleIds.add(n.id) + } + return { visibleIds, hiddenBy } +} + +/** + * Convenience wrapper kept for call sites that only need the visible set. + */ +export function getVisibleNodeIds(nodes: Node[]): Set { + return computeCollapseInfo(nodes).visibleIds +} + +/** + * Rewire edges so that any endpoint inside a collapsed subtree (parentId or + * spatial) is replaced with the nearest visible ancestor. See module + * docstring for the full rationale. * - * - Both endpoints visible → edge kept as-is. - * - One endpoint hidden → endpoint replaced by its nearest visible - * ancestor; edge surfaces as a "stub" on the - * collapsed zone so the connection is not lost. - * - Both endpoints hidden under the *same* collapsed ancestor → dropped - * (would be a self-loop on the zone). - * - Multiple original edges that rewire to the same (source, target) pair - * are de-duplicated; only the first is kept. Prevents a 20-device Zigbee - * mesh from rendering 20 stacked stub edges on the collapsed parent. - * - * Edges with an endpoint whose ancestor chain never reaches a visible node - * (orphaned reference) are dropped. + * - Both endpoints visible → edge kept as-is. + * - One endpoint hidden → endpoint replaced by its nearest + * visible ancestor; edge surfaces + * as a stub on the collapsed zone. + * - Both endpoints hidden under the + * same visible ancestor → dropped (would be a self-loop). + * - Parallel rewires to the same pair → de-duplicated; one stub kept. + * (Prevents a 20-device mesh from rendering 20 stacked stubs.) + * - Endpoint with no visible ancestor → dropped. */ export function rewireEdgesForCollapse( edges: Edge[], nodes: Node[], visibleIds: Set, + hiddenBy?: Map, ): Edge[] { - const parentOf = new Map() - for (const n of nodes) parentOf.set(n.id, n.parentId) + // If the caller already computed hiddenBy (CanvasContainer path), reuse + // it. Otherwise recompute — keeps the helper callable from tests without + // forcing them to thread the second map through. + const hb = hiddenBy ?? computeCollapseInfo(nodes).hiddenBy const nearestVisible = (id: string): string | null => { let cur: string | undefined = id - // Walk up parentId chain until we hit a visible node or run out. + const guard = new Set() while (cur !== undefined) { if (visibleIds.has(cur)) return cur - cur = parentOf.get(cur) + if (guard.has(cur)) return null + guard.add(cur) + cur = hb.get(cur) } return null } @@ -93,8 +189,6 @@ export function rewireEdgesForCollapse( if (src === e.source && tgt === e.target) { out.push(e) } else { - // Endpoint moved → strip handle hints that referred to the original - // (hidden) node; let React Flow pick defaults on the visible ancestor. out.push({ ...e, source: src, target: tgt, sourceHandle: null, targetHandle: null }) } }