refactor(canvas): promote collapsed to first-class NodeData field + edge rewire
Three follow-ups to PR #158 review: 1. Promote collapsed to NodeData.collapsed The flag was previously stashed inside NodeData.custom_colors, which is a colors/style object — semantically wrong. Move it to a first-class boolean on NodeData. Persistence keeps the existing API shape: serialize writes it into the custom_colors blob (alongside width/height/z_order, matching how groupRect already stashes layout metadata), and deserialize hoists it back. Legacy saves from the original PR shape load correctly. 2. Re-route cross-zone edges to the collapsed ancestor Previously any edge touching a hidden node was dropped, so a Zigbee coordinator outside a collapsed mesh lost all visible links to it. rewireEdgesForCollapse now walks each endpoint up the parentId chain to its nearest visible ancestor, surfaces a single stub edge on the collapsed zone, de-dupes parallel rewires (a 20-device mesh becomes one stub, not twenty), and drops edges that would self-loop on a zone or reference an orphan. 3. Revert package-lock.json churn The 63-line diff from the original PR was npm-version drift (libc arrays stripped from optional deps), unrelated to the feature. Tests: - canvasStore.collapse: updated to assert on data.collapsed. - collapseFilter: 8 cases for visibility + 7 for edge rewire, covering cross-boundary, nested collapse, sibling self-loop, mesh dedup, and orphan endpoints. - canvasSerializer.collapse: round-trip + legacy-shape compat.
This commit is contained in:
@@ -0,0 +1,83 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import type { Node } from '@xyflow/react'
|
||||
import type { NodeData } from '@/types'
|
||||
import { serializeNode, deserializeApiNode, type ApiNode } from '@/utils/canvasSerializer'
|
||||
|
||||
/**
|
||||
* Persistence contract for the collapse flag on groupRect nodes:
|
||||
*
|
||||
* 1. Serialize stashes `data.collapsed` into `custom_colors.collapsed`
|
||||
* so the existing API blob shape can carry it without a schema change.
|
||||
* 2. Deserialize hoists it back to the first-class `data.collapsed` field.
|
||||
* 3. Legacy saves that already had `custom_colors.collapsed` (the original
|
||||
* shape from PR #158 before the field was promoted) still load
|
||||
* correctly.
|
||||
*/
|
||||
|
||||
function makeGroupRectRfNode(collapsed?: boolean): Node<NodeData> {
|
||||
return {
|
||||
id: 'zone-1',
|
||||
type: 'groupRect',
|
||||
position: { x: 0, y: 0 },
|
||||
data: {
|
||||
label: 'Zigbee Mesh',
|
||||
type: 'groupRect',
|
||||
status: 'unknown',
|
||||
services: [],
|
||||
...(collapsed !== undefined ? { collapsed } : {}),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
describe('canvasSerializer — groupRect collapse', () => {
|
||||
it('stashes data.collapsed=true into custom_colors on serialize', () => {
|
||||
const rf = makeGroupRectRfNode(true)
|
||||
const api = serializeNode(rf) as Record<string, unknown>
|
||||
const cc = api.custom_colors as Record<string, unknown>
|
||||
expect(cc.collapsed).toBe(true)
|
||||
})
|
||||
|
||||
it('writes collapsed=false when the flag is missing (explicit default)', () => {
|
||||
const rf = makeGroupRectRfNode(undefined)
|
||||
const api = serializeNode(rf) as Record<string, unknown>
|
||||
const cc = api.custom_colors as Record<string, unknown>
|
||||
expect(cc.collapsed).toBe(false)
|
||||
})
|
||||
|
||||
it('hoists custom_colors.collapsed back to data.collapsed on deserialize', () => {
|
||||
const apiNode: ApiNode = {
|
||||
id: 'zone-1',
|
||||
type: 'groupRect',
|
||||
label: 'Zone',
|
||||
pos_x: 0,
|
||||
pos_y: 0,
|
||||
status: 'unknown',
|
||||
services: [],
|
||||
custom_colors: { collapsed: true, width: 360, height: 240 },
|
||||
}
|
||||
const rf = deserializeApiNode(apiNode, new Map())
|
||||
expect(rf.data.collapsed).toBe(true)
|
||||
})
|
||||
|
||||
it('treats missing custom_colors.collapsed as false on deserialize', () => {
|
||||
const apiNode: ApiNode = {
|
||||
id: 'zone-1',
|
||||
type: 'groupRect',
|
||||
label: 'Zone',
|
||||
pos_x: 0,
|
||||
pos_y: 0,
|
||||
status: 'unknown',
|
||||
services: [],
|
||||
custom_colors: { width: 360, height: 240 },
|
||||
}
|
||||
const rf = deserializeApiNode(apiNode, new Map())
|
||||
expect(rf.data.collapsed).toBe(false)
|
||||
})
|
||||
|
||||
it('round-trips the collapse flag through serialize → deserialize', () => {
|
||||
const rf = makeGroupRectRfNode(true)
|
||||
const api = serializeNode(rf) as unknown as ApiNode
|
||||
const back = deserializeApiNode(api, new Map())
|
||||
expect(back.data.collapsed).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import type { Edge, Node } from '@xyflow/react'
|
||||
import { getVisibleNodeIds, filterVisibleEdges } from '../collapseFilter'
|
||||
import { getVisibleNodeIds, rewireEdgesForCollapse } from '../collapseFilter'
|
||||
import type { EdgeData, NodeData } from '@/types'
|
||||
|
||||
const mkNode = (
|
||||
@@ -16,7 +16,7 @@ const mkNode = (
|
||||
type: parentId ? 'server' : 'groupRect',
|
||||
status: 'online',
|
||||
services: [],
|
||||
...(collapsed !== undefined ? { custom_colors: { collapsed } } : {}),
|
||||
...(collapsed !== undefined ? { collapsed } : {}),
|
||||
},
|
||||
})
|
||||
|
||||
@@ -91,30 +91,82 @@ describe('getVisibleNodeIds', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('filterVisibleEdges', () => {
|
||||
it('keeps edges whose endpoints are both visible', () => {
|
||||
const visible = new Set(['a', 'b'])
|
||||
describe('rewireEdgesForCollapse', () => {
|
||||
it('keeps edges between two visible nodes unchanged (same reference)', () => {
|
||||
const nodes = [mkNode('a'), mkNode('b')]
|
||||
const edges = [mkEdge('e1', 'a', 'b')]
|
||||
expect(filterVisibleEdges(edges, visible)).toHaveLength(1)
|
||||
const out = rewireEdgesForCollapse(edges, nodes, new Set(['a', 'b']))
|
||||
expect(out).toHaveLength(1)
|
||||
expect(out[0]).toBe(edges[0])
|
||||
})
|
||||
|
||||
it('drops an edge whose target is inside a collapsed subtree', () => {
|
||||
const visible = new Set(['outside', 'zone']) // 'leaf' hidden
|
||||
const edges = [
|
||||
mkEdge('e-outside-leaf', 'outside', 'leaf'),
|
||||
mkEdge('e-outside-zone', 'outside', 'zone'),
|
||||
it('reroutes a cross-boundary edge to the collapsed ancestor', () => {
|
||||
const nodes = [
|
||||
mkNode('zone', undefined, true),
|
||||
mkNode('leaf', 'zone'),
|
||||
mkNode('outside'),
|
||||
]
|
||||
const out = filterVisibleEdges(edges, visible)
|
||||
expect(out.map((e) => e.id)).toEqual(['e-outside-zone'])
|
||||
const visible = getVisibleNodeIds(nodes)
|
||||
const edges = [mkEdge('e1', 'outside', 'leaf')]
|
||||
const out = rewireEdgesForCollapse(edges, nodes, visible)
|
||||
expect(out).toHaveLength(1)
|
||||
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 whose source is hidden', () => {
|
||||
const visible = new Set(['b'])
|
||||
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(filterVisibleEdges(edges, visible)).toHaveLength(0)
|
||||
expect(rewireEdgesForCollapse(edges, nodes, visible)).toEqual([])
|
||||
})
|
||||
|
||||
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.
|
||||
const nodes = [
|
||||
mkNode('zone', undefined, true),
|
||||
mkNode('coord'),
|
||||
...Array.from({ length: 5 }, (_, i) => mkNode(`leaf-${i}`, 'zone')),
|
||||
]
|
||||
const visible = getVisibleNodeIds(nodes)
|
||||
const edges = Array.from({ length: 5 }, (_, i) =>
|
||||
mkEdge(`e-${i}`, 'coord', `leaf-${i}`),
|
||||
)
|
||||
const out = rewireEdgesForCollapse(edges, nodes, visible)
|
||||
expect(out).toHaveLength(1)
|
||||
expect(out[0].source).toBe('coord')
|
||||
expect(out[0].target).toBe('zone')
|
||||
})
|
||||
|
||||
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'),
|
||||
]
|
||||
const visible = getVisibleNodeIds(nodes)
|
||||
const edges = [mkEdge('e1', 'outside', 'leaf')]
|
||||
const out = rewireEdgesForCollapse(edges, nodes, visible)
|
||||
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([])
|
||||
})
|
||||
|
||||
it('returns an empty array for empty input', () => {
|
||||
expect(filterVisibleEdges([], new Set(['a']))).toEqual([])
|
||||
expect(rewireEdgesForCollapse([], [], new Set())).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -76,6 +76,9 @@ export function serializeNode(n: Node<NodeData>): Record<string, unknown> {
|
||||
...n.data.custom_colors,
|
||||
width: n.measured?.width ?? n.width ?? 360,
|
||||
height: n.measured?.height ?? n.height ?? 240,
|
||||
// Stash collapse state inside custom_colors so the API/YAML blob does
|
||||
// not need a new column. Hoisted back to `data.collapsed` on load.
|
||||
collapsed: n.data.collapsed ?? false,
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -139,11 +142,15 @@ export function deserializeApiNode(
|
||||
const w = (n.custom_colors?.width as number | undefined) ?? 360
|
||||
const h = (n.custom_colors?.height as number | undefined) ?? 240
|
||||
const z = (n.custom_colors?.z_order as number | undefined) ?? 1
|
||||
// Hoist persisted collapse flag from the custom_colors stash to a
|
||||
// first-class field on NodeData. Tolerates legacy saves that already had
|
||||
// it there from before the type was promoted.
|
||||
const collapsed = Boolean(n.custom_colors?.collapsed)
|
||||
return {
|
||||
id: n.id,
|
||||
type: 'groupRect',
|
||||
position: { x: n.pos_x, y: n.pos_y },
|
||||
data: n as unknown as NodeData,
|
||||
data: { ...(n as unknown as NodeData), collapsed },
|
||||
width: w,
|
||||
height: h,
|
||||
zIndex: z - 10,
|
||||
|
||||
@@ -35,7 +35,7 @@ export function getVisibleNodeIds(nodes: Node<NodeData>[]): Set<string> {
|
||||
const id = queue.shift()!
|
||||
visible.add(id)
|
||||
const node = byId.get(id)
|
||||
if (node && !node.data.custom_colors?.collapsed) {
|
||||
if (node && !node.data.collapsed) {
|
||||
const children = childrenByParent.get(id)
|
||||
if (children) queue.push(...children)
|
||||
}
|
||||
@@ -45,12 +45,58 @@ export function getVisibleNodeIds(nodes: Node<NodeData>[]): Set<string> {
|
||||
}
|
||||
|
||||
/**
|
||||
* Filter edges to those whose source and target are both in `visibleIds`.
|
||||
* Edges crossing into a collapsed subtree are dropped.
|
||||
* 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:
|
||||
*
|
||||
* - 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.
|
||||
*/
|
||||
export function filterVisibleEdges(
|
||||
export function rewireEdgesForCollapse(
|
||||
edges: Edge<EdgeData>[],
|
||||
nodes: Node<NodeData>[],
|
||||
visibleIds: Set<string>,
|
||||
): Edge<EdgeData>[] {
|
||||
return edges.filter((e) => visibleIds.has(e.source) && visibleIds.has(e.target))
|
||||
const parentOf = new Map<string, string | undefined>()
|
||||
for (const n of nodes) parentOf.set(n.id, n.parentId)
|
||||
|
||||
const nearestVisible = (id: string): string | null => {
|
||||
let cur: string | undefined = id
|
||||
// Walk up parentId chain until we hit a visible node or run out.
|
||||
while (cur !== undefined) {
|
||||
if (visibleIds.has(cur)) return cur
|
||||
cur = parentOf.get(cur)
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
const seen = new Set<string>()
|
||||
const out: Edge<EdgeData>[] = []
|
||||
for (const e of edges) {
|
||||
const src = nearestVisible(e.source)
|
||||
const tgt = nearestVisible(e.target)
|
||||
if (src === null || tgt === null) continue
|
||||
if (src === tgt) continue
|
||||
const key = `${src}->${tgt}`
|
||||
if (seen.has(key)) continue
|
||||
seen.add(key)
|
||||
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 })
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user