Merge pull request #288 from Pouzor/fix/edge-waypoints-follow-dragged-node

fix(canvas): move edge waypoints when the connected node or its container is dragged
2026-07-16 17:36:24 +02:00
parent 9e088ca7ac 3cab115a80
commit 6ae8b91934
2 changed files with 174 additions and 0 deletions
@@ -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 Edge,
  type NodeChange,
+  type NodePositionChange,
  type EdgeChange,
  type Connection,
  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. */
 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. */
 export const serviceStatusKey = (nodeId: string, port?: number, protocol?: string): string =>
  `${nodeId}:${port ?? ''}/${protocol ?? ''}`
@@ -251,8 +315,13 @@ export const useCanvasStore = create<CanvasState>((set) => ({
    set((state) => {
      const nodes = applyNodeChanges(changes, state.nodes)
      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 {
        nodes,
+        edges,
        selectedNodeIds,
        hasUnsavedChanges: state.hasUnsavedChanges || changes.some((c) => c.type !== 'select'),
      }