fix(canvas): move edge waypoints when the connected node or its container is dragged
Waypoints are stored as absolute canvas coords, so they stayed put when a connected node was dragged, kinking a previously clean route. A dragged container moved its children on screen too (their stored parent-relative position is unchanged), so their edges were missed entirely. onNodesChange now translates the waypoints of every edge touching a moved node by that node's delta, propagating a container's delta to all descendants and translating once when both endpoints ride along. ha-relevant: yes Fixes #279
This commit is contained in:
@@ -0,0 +1,105 @@
|
|||||||
|
import { describe, it, expect, beforeEach } from 'vitest'
|
||||||
|
import { useCanvasStore } from '@/stores/canvasStore'
|
||||||
|
import type { NodeChange, Node } from '@xyflow/react'
|
||||||
|
import type { NodeData } from '@/types'
|
||||||
|
import { makeNode, makeEdge } from '@/test/factories'
|
||||||
|
|
||||||
|
function resetStore() {
|
||||||
|
useCanvasStore.setState({
|
||||||
|
nodes: [],
|
||||||
|
edges: [],
|
||||||
|
hasUnsavedChanges: false,
|
||||||
|
selectedNodeId: null,
|
||||||
|
selectedNodeIds: [],
|
||||||
|
editingGroupRectId: null,
|
||||||
|
editingTextId: null,
|
||||||
|
past: [],
|
||||||
|
future: [],
|
||||||
|
clipboard: { nodes: [], edges: [] },
|
||||||
|
serviceStatuses: {},
|
||||||
|
floorMap: null,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
/** A `position` NodeChange as React Flow emits during/after a drag. */
|
||||||
|
function posChange(id: string, x: number, y: number): NodeChange<Node<NodeData>> {
|
||||||
|
return { type: 'position', id, position: { x, y }, dragging: false }
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('canvasStore — edge waypoints follow dragged nodes (#279)', () => {
|
||||||
|
beforeEach(resetStore)
|
||||||
|
|
||||||
|
it('translates waypoints by the delta when a connected node is dragged', () => {
|
||||||
|
useCanvasStore.setState({
|
||||||
|
nodes: [makeNode('a', {}), makeNode('b', {})],
|
||||||
|
edges: [makeEdge('e1', 'a', 'b', { data: { type: 'ethernet', waypoints: [{ x: 100, y: 100 }, { x: 150, y: 120 }] } })],
|
||||||
|
})
|
||||||
|
// node 'a' starts at (0,0), dragged to (40,-10) → delta (40,-10)
|
||||||
|
useCanvasStore.getState().onNodesChange([posChange('a', 40, -10)])
|
||||||
|
|
||||||
|
const wps = useCanvasStore.getState().edges[0].data!.waypoints
|
||||||
|
expect(wps).toEqual([{ x: 140, y: 90 }, { x: 190, y: 110 }])
|
||||||
|
})
|
||||||
|
|
||||||
|
it('leaves waypoints untouched when an unrelated node is dragged', () => {
|
||||||
|
useCanvasStore.setState({
|
||||||
|
nodes: [makeNode('a', {}), makeNode('b', {}), makeNode('c', {})],
|
||||||
|
edges: [makeEdge('e1', 'a', 'b', { data: { type: 'ethernet', waypoints: [{ x: 100, y: 100 }] } })],
|
||||||
|
})
|
||||||
|
useCanvasStore.getState().onNodesChange([posChange('c', 40, 40)])
|
||||||
|
|
||||||
|
expect(useCanvasStore.getState().edges[0].data!.waypoints).toEqual([{ x: 100, y: 100 }])
|
||||||
|
})
|
||||||
|
|
||||||
|
it('translates waypoints of edges between children when their container is dragged', () => {
|
||||||
|
// Children are parent-relative; their stored position does NOT change when
|
||||||
|
// the container moves, so the fix must propagate the container delta down.
|
||||||
|
useCanvasStore.setState({
|
||||||
|
nodes: [
|
||||||
|
makeNode({ id: 'box', type: 'group', position: { x: 0, y: 0 }, data: { label: 'box', type: 'lxc', container_mode: true } }),
|
||||||
|
makeNode({ id: 'a', position: { x: 20, y: 20 }, parentId: 'box' }),
|
||||||
|
makeNode({ id: 'b', position: { x: 80, y: 60 }, parentId: 'box' }),
|
||||||
|
],
|
||||||
|
edges: [makeEdge('e1', 'a', 'b', { data: { type: 'ethernet', waypoints: [{ x: 200, y: 200 }] } })],
|
||||||
|
})
|
||||||
|
// Drag the container from (0,0) to (30,15) → delta (30,15)
|
||||||
|
useCanvasStore.getState().onNodesChange([posChange('box', 30, 15)])
|
||||||
|
|
||||||
|
expect(useCanvasStore.getState().edges[0].data!.waypoints).toEqual([{ x: 230, y: 215 }])
|
||||||
|
})
|
||||||
|
|
||||||
|
it('translates once (not doubled) when both endpoints move by the same delta', () => {
|
||||||
|
// A container drag reports the container moving; both children ride along.
|
||||||
|
useCanvasStore.setState({
|
||||||
|
nodes: [
|
||||||
|
makeNode({ id: 'box', type: 'group', position: { x: 0, y: 0 }, data: { label: 'box', type: 'lxc', container_mode: true } }),
|
||||||
|
makeNode({ id: 'a', position: { x: 20, y: 20 }, parentId: 'box' }),
|
||||||
|
makeNode({ id: 'b', position: { x: 80, y: 60 }, parentId: 'box' }),
|
||||||
|
],
|
||||||
|
edges: [makeEdge('e1', 'a', 'b', { data: { type: 'ethernet', waypoints: [{ x: 100, y: 100 }] } })],
|
||||||
|
})
|
||||||
|
useCanvasStore.getState().onNodesChange([posChange('box', 10, 10)])
|
||||||
|
|
||||||
|
// +10 once, not +20
|
||||||
|
expect(useCanvasStore.getState().edges[0].data!.waypoints).toEqual([{ x: 110, y: 110 }])
|
||||||
|
})
|
||||||
|
|
||||||
|
it('does not alter edges without waypoints', () => {
|
||||||
|
useCanvasStore.setState({
|
||||||
|
nodes: [makeNode('a', {}), makeNode('b', {})],
|
||||||
|
edges: [makeEdge('e1', 'a', 'b')],
|
||||||
|
})
|
||||||
|
const before = useCanvasStore.getState().edges[0]
|
||||||
|
useCanvasStore.getState().onNodesChange([posChange('a', 40, 40)])
|
||||||
|
expect(useCanvasStore.getState().edges[0]).toBe(before)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('ignores select-only changes (no movement)', () => {
|
||||||
|
useCanvasStore.setState({
|
||||||
|
nodes: [makeNode('a', {}), makeNode('b', {})],
|
||||||
|
edges: [makeEdge('e1', 'a', 'b', { data: { type: 'ethernet', waypoints: [{ x: 100, y: 100 }] } })],
|
||||||
|
})
|
||||||
|
useCanvasStore.getState().onNodesChange([{ type: 'select', id: 'a', selected: true }])
|
||||||
|
expect(useCanvasStore.getState().edges[0].data!.waypoints).toEqual([{ x: 100, y: 100 }])
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -3,6 +3,7 @@ import {
|
|||||||
type Node,
|
type Node,
|
||||||
type Edge,
|
type Edge,
|
||||||
type NodeChange,
|
type NodeChange,
|
||||||
|
type NodePositionChange,
|
||||||
type EdgeChange,
|
type EdgeChange,
|
||||||
type Connection,
|
type Connection,
|
||||||
applyNodeChanges,
|
applyNodeChanges,
|
||||||
@@ -21,6 +22,69 @@ type Clipboard = { nodes: Node<NodeData>[]; edges: Edge<EdgeData>[] }
|
|||||||
/** Resolve a node's effective parent id from either the RF field or domain data. */
|
/** Resolve a node's effective parent id from either the RF field or domain data. */
|
||||||
const parentIdOf = (n: Node<NodeData>): string | undefined => n.parentId ?? n.data.parent_id ?? undefined
|
const parentIdOf = (n: Node<NodeData>): string | undefined => n.parentId ?? n.data.parent_id ?? undefined
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Keep manually-routed edge waypoints attached to their nodes on drag (#279).
|
||||||
|
*
|
||||||
|
* Waypoints live in absolute canvas coords, so they don't move when a connected
|
||||||
|
* node is dragged. For every node that moved on screen we translate the
|
||||||
|
* waypoints of edges touching it by the same delta. A dragged container moves
|
||||||
|
* its children on screen too — their stored (parent-relative) position is
|
||||||
|
* unchanged, so we propagate the container's delta down to every descendant.
|
||||||
|
*/
|
||||||
|
function translateWaypointsForMovedNodes(
|
||||||
|
changes: NodeChange<Node<NodeData>>[],
|
||||||
|
prevNodes: Node<NodeData>[],
|
||||||
|
nextNodes: Node<NodeData>[],
|
||||||
|
edges: Edge<EdgeData>[],
|
||||||
|
): Edge<EdgeData>[] {
|
||||||
|
const positionChanges = changes.filter(
|
||||||
|
(c): c is NodePositionChange => c.type === 'position' && !!c.position,
|
||||||
|
)
|
||||||
|
if (positionChanges.length === 0) return edges
|
||||||
|
|
||||||
|
const prevById = new Map(prevNodes.map((n) => [n.id, n]))
|
||||||
|
// node id -> absolute screen delta it moved by
|
||||||
|
const deltaById = new Map<string, { dx: number; dy: number }>()
|
||||||
|
for (const ch of positionChanges) {
|
||||||
|
const prev = prevById.get(ch.id)
|
||||||
|
if (!prev) continue
|
||||||
|
const dx = ch.position!.x - prev.position.x
|
||||||
|
const dy = ch.position!.y - prev.position.y
|
||||||
|
if (dx === 0 && dy === 0) continue
|
||||||
|
deltaById.set(ch.id, { dx, dy })
|
||||||
|
}
|
||||||
|
if (deltaById.size === 0) return edges
|
||||||
|
|
||||||
|
const childrenByParent = new Map<string, string[]>()
|
||||||
|
for (const n of nextNodes) {
|
||||||
|
const pid = parentIdOf(n)
|
||||||
|
if (!pid) continue
|
||||||
|
const arr = childrenByParent.get(pid) ?? []
|
||||||
|
arr.push(n.id)
|
||||||
|
childrenByParent.set(pid, arr)
|
||||||
|
}
|
||||||
|
const propagate = (id: string, d: { dx: number; dy: number }) => {
|
||||||
|
for (const childId of childrenByParent.get(id) ?? []) {
|
||||||
|
// A directly-dragged child keeps its own delta; don't overwrite it.
|
||||||
|
if (!deltaById.has(childId)) deltaById.set(childId, d)
|
||||||
|
propagate(childId, d)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for (const [id, d] of [...deltaById.entries()]) propagate(id, d)
|
||||||
|
|
||||||
|
return edges.map((e) => {
|
||||||
|
const data = e.data
|
||||||
|
if (!data?.waypoints?.length) return e
|
||||||
|
// Both endpoints may have moved (container drag): translate once.
|
||||||
|
const d = deltaById.get(e.source) ?? deltaById.get(e.target)
|
||||||
|
if (!d) return e
|
||||||
|
return {
|
||||||
|
...e,
|
||||||
|
data: { ...data, waypoints: data.waypoints.map((wp) => ({ x: wp.x + d.dx, y: wp.y + d.dy })) },
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
/** Key for the live per-service status overlay. */
|
/** Key for the live per-service status overlay. */
|
||||||
export const serviceStatusKey = (nodeId: string, port?: number, protocol?: string): string =>
|
export const serviceStatusKey = (nodeId: string, port?: number, protocol?: string): string =>
|
||||||
`${nodeId}:${port ?? ''}/${protocol ?? ''}`
|
`${nodeId}:${port ?? ''}/${protocol ?? ''}`
|
||||||
@@ -251,8 +315,13 @@ export const useCanvasStore = create<CanvasState>((set) => ({
|
|||||||
set((state) => {
|
set((state) => {
|
||||||
const nodes = applyNodeChanges(changes, state.nodes)
|
const nodes = applyNodeChanges(changes, state.nodes)
|
||||||
const selectedNodeIds = nodes.filter((n) => n.selected).map((n) => n.id)
|
const selectedNodeIds = nodes.filter((n) => n.selected).map((n) => n.id)
|
||||||
|
// Manually-placed edge waypoints are stored as absolute canvas coords, so
|
||||||
|
// they don't follow a moved node on their own. Translate them by the same
|
||||||
|
// delta the node moved so a clean routing stays clean after a drag (#279).
|
||||||
|
const edges = translateWaypointsForMovedNodes(changes, state.nodes, nodes, state.edges)
|
||||||
return {
|
return {
|
||||||
nodes,
|
nodes,
|
||||||
|
edges,
|
||||||
selectedNodeIds,
|
selectedNodeIds,
|
||||||
hasUnsavedChanges: state.hasUnsavedChanges || changes.some((c) => c.type !== 'select'),
|
hasUnsavedChanges: state.hasUnsavedChanges || changes.some((c) => c.type !== 'select'),
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user