From 5e9963db4dafab636c13716ce5c72aa028939fcf Mon Sep 17 00:00:00 2001 From: Pouzor Date: Mon, 11 May 2026 01:14:47 +0200 Subject: [PATCH] refactor(alignment): defer snap to drag stop, stabilise callback deps Code review flagged a drag race + deps issues: - Race: setNodes during onNodeDrag fights React Flow's internal drag handler, which derives positions from the cursor offset captured at drag start. Snap delta could be overwritten on the next frame. - Deps: useCallback closed over guides.length, causing recreation on every guide change and pulling stale references into onNodeDrag. Fix: compute guides live during drag for visual feedback, store the pending snap delta in a ref, and apply it via setNodes only on onNodeDragStop. Replace the guides.length dep with a ref mirror updated via useEffect (not during render). --- frontend/src/hooks/useAlignmentGuides.ts | 53 +++++++++++++++++------- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/frontend/src/hooks/useAlignmentGuides.ts b/frontend/src/hooks/useAlignmentGuides.ts index cbcca08..751f123 100644 --- a/frontend/src/hooks/useAlignmentGuides.ts +++ b/frontend/src/hooks/useAlignmentGuides.ts @@ -12,7 +12,7 @@ import type { NodeData } from '@/types' type NodeDrag = OnNodeDrag> function nodeBox(n: Node): Box | null { - // Skip nodes with a parent — alignment between absolute & parent-relative + // Skip parented nodes — alignment between absolute and parent-relative // coordinates is misleading. Top-level nodes only for v1. if (n.parentId) return null const width = n.measured?.width ?? n.width ?? null @@ -21,9 +21,24 @@ function nodeBox(n: Node): Box | null { return { id: n.id, x: n.position.x, y: n.position.y, width, height } } +/** + * Drag-time alignment guides + snap (draw.io / Figma style). + * + * Snap is applied on drag stop (not during drag) to avoid racing React Flow's + * internal drag handler, which computes positions from the cursor offset + * captured at drag start. Guides update live for visual feedback. + * + * Settings (enabled, threshold) live in localStorage and propagate same-tab + * via a CustomEvent so the panel and the hook stay in sync. + */ export function useAlignmentGuides() { const [settings, setSettings] = useState(readAlignmentSettings) const [guides, setGuides] = useState([]) + // ref mirror of guides so callbacks don't need it in their deps + const guidesRef = useRef([]) + useEffect(() => { guidesRef.current = guides }, [guides]) + // latest snap deltas, applied on drag stop + const pendingSnapRef = useRef<{ deltaX: number; deltaY: number; ids: Set } | null>(null) const altDownRef = useRef(false) const { setNodes, getNodes } = useReactFlow>() @@ -48,40 +63,50 @@ export function useAlignmentGuides() { }) }, []) + const clearState = useCallback(() => { + if (guidesRef.current.length > 0) setGuides([]) + pendingSnapRef.current = null + }, []) + const onNodeDrag: NodeDrag = useCallback((_event, dragNode, dragNodes) => { if (!settings.enabled || altDownRef.current) { - if (guides.length > 0) setGuides([]) + clearState() return } const all = getNodes() - const draggedSet = new Set((dragNodes.length > 0 ? dragNodes : [dragNode]).map((n) => n.id)) - const draggedBoxes = all.filter((n) => draggedSet.has(n.id)).map(nodeBox).filter((b): b is Box => b !== null) + const ids = new Set((dragNodes.length > 0 ? dragNodes : [dragNode]).map((n) => n.id)) + const draggedBoxes = all.filter((n) => ids.has(n.id)).map(nodeBox).filter((b): b is Box => b !== null) if (draggedBoxes.length === 0) { - if (guides.length > 0) setGuides([]) + clearState() return } const dragged = draggedBoxes.length === 1 ? draggedBoxes[0] : unionBox(draggedBoxes) if (!dragged) return const candidates = all - .filter((n) => !draggedSet.has(n.id)) + .filter((n) => !ids.has(n.id)) .map(nodeBox) .filter((b): b is Box => b !== null) const result = computeSnap(dragged, candidates, settings.threshold) setGuides(result.guides) - if (result.deltaX !== 0 || result.deltaY !== 0) { + pendingSnapRef.current = + result.deltaX !== 0 || result.deltaY !== 0 + ? { deltaX: result.deltaX, deltaY: result.deltaY, ids } + : null + }, [settings.enabled, settings.threshold, getNodes, clearState]) + + const onNodeDragStop: NodeDrag = useCallback(() => { + const pending = pendingSnapRef.current + if (pending) { setNodes((ns) => ns.map((n) => - draggedSet.has(n.id) - ? { ...n, position: { x: n.position.x + result.deltaX, y: n.position.y + result.deltaY } } + pending.ids.has(n.id) + ? { ...n, position: { x: n.position.x + pending.deltaX, y: n.position.y + pending.deltaY } } : n, ), ) } - }, [settings.enabled, settings.threshold, getNodes, setNodes, guides.length]) - - const onNodeDragStop: NodeDrag = useCallback(() => { - if (guides.length > 0) setGuides([]) - }, [guides.length]) + clearState() + }, [setNodes, clearState]) return { guides, settings, update, onNodeDrag, onNodeDragStop } }