fix(canvas): make collapse reachable + persist for every node type
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.
This commit is contained in:
Pouzor
@@ -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<NodeData> = {
|
||||
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<string, unknown>
|
||||
const cc = api.custom_colors as Record<string, unknown>
|
||||
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<NodeData> = {
|
||||
id: 's1',
|
||||
type: 'server',
|
||||
position: { x: 0, y: 0 },
|
||||
data: { label: 'Server', type: 'server', status: 'online', services: [] },
|
||||
}
|
||||
const api = serializeNode(rf) as Record<string, unknown>
|
||||
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<NodeData> = {
|
||||
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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<NodeData> => ({
|
||||
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<NodeData> => ({
|
||||
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<EdgeData> => (
|
||||
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<string>() // 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([])
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user