handleNodeDrag pushes history on every incremental mouse-move (~200 entries per 100px drag); no drag-start/drag-end signal #90

New issue

Closed

opened 2026-09-14 14:53:01 +00:00 by Mark · 0 comments

Mark commented

2026-09-14 14:53:01 +00:00

Owner

Where: src/pages/Editor.tsx:243-258 (handleNodeDrag), src/components/editor/EditorCanvas.tsx:184-223 (handleMouseMove).

Bug: EditorCanvas.handleMouseMove calls onNodeDrag(dragNodeId, dx, dy) whenever the mouse moves more than ~0.5 px from the last drag sample (EditorCanvas.tsx:199-205), passing the incremental delta and resetting dragStart to the current position. Each call lands in Editor.handleNodeDrag:

// src/pages/Editor.tsx:243
const handleNodeDrag = useCallback((nodeId, deltaX, deltaY) => {
  const next = paths.map((path) => { ... });
  const mutated = next.some((p, i) => p !== paths[i]);
  if (mutated) pushHistory(paths);   // ← fires on EVERY ~0.5 px move
  setPaths(next);
}, [paths, pushHistory]);

A 100 px drag fires ~200 onNodeDrag calls. Each one pushes the pre-drag-step paths to history. So a single gesture creates ~200 history entries. With HISTORY_LIMIT = 50 (Editor.tsx:182), the first 150 entries get shifted off, leaving the last 50 to be intermediate-step snapshots. Undo (when #89 lands) would step through ~50 sub-pixel-offset states per gesture.

There is no drag-start/drag-end signal between EditorCanvas and Editor. EditorCanvas.handleMouseUp (EditorCanvas.tsx:225-232) only resets local state — it never calls a onDragEnd callback. Editor therefore cannot distinguish "first pixel of a drag" from "200th pixel of the same drag."

Symptom (once #89 is fixed): dragging a node 100 px right, then pressing Ctrl+Z once, would only roll back ~0.5 px. The user would need to press Undo ~200 times to fully revert. Most users will assume Undo is broken (the same false-positive symptom #89 already has).

Severity: medium. Doesn't break anything visible today (because #89 makes Undo a no-op anyway). Once Undo is fixed, this becomes a UX disaster: every drag gesture fills the history buffer with one-snapshot-per-pixel noise, and the user has to mash Ctrl+Z to revert a single move.

Fix consistent with existing patterns: Add a dragNodeId: string | null prop and onDragStart?: (nodeId: string) => void / onDragEnd?: (nodeId: string) => void callbacks to EditorCanvasProps (EditorCanvas.tsx:15-37). In EditorCanvas.handleMouseDown (line 169), call onDragStart(found.node.id) right after setIsDragging(true). In handleMouseUp (line 225-232), call onDragEnd(dragNodeId) before resetting state. Then in Editor.tsx:

const handleDragStart = useCallback((nodeId: string) => {
  // capture pre-drag snapshot ONCE
  dragStartSnapshotRef.current = JSON.parse(JSON.stringify(paths));
}, [paths]);

const handleDragEnd = useCallback(() => {
  if (dragStartSnapshotRef.current) {
    pushHistory(dragStartSnapshotRef.current);
    dragStartSnapshotRef.current = null;
  }
}, [pushHistory]);

const handleNodeDrag = useCallback((nodeId, dx, dy) => {
  // NO pushHistory here — only mutate paths
  setPaths((prev) => prev.map((path) => {
    const nodeIndex = path.nodes.findIndex((n) => n.id === nodeId);
    if (nodeIndex === -1) return path;
    const newNodes = path.nodes.map((n, i) =>
      i === nodeIndex ? { ...n, x: n.x + dx, y: n.y + dy } : n
    );
    return { ...path, nodes: newNodes };
  }));
}, []);  // no path deps — uses functional setState

Same shape applies to handleCanvasClick/handleFinishPath if those also drag (path-drawing mode emits a onCanvasClick per click — already pushes per click, which is correct for discrete clicks but not for any continuous gesture).

Cross-reference: Pair this with #89 (undo/redo setPaths fix). When both land, dragging and undoing should produce one history entry per gesture, with the full drag range as the unit of undo. Today the user sees neither; once #89 lands without this fix, they'll see "Undo moves the node 0.5 px per click."

**Where:** `src/pages/Editor.tsx:243-258` (`handleNodeDrag`), `src/components/editor/EditorCanvas.tsx:184-223` (`handleMouseMove`). **Bug:** `EditorCanvas.handleMouseMove` calls `onNodeDrag(dragNodeId, dx, dy)` whenever the mouse moves more than ~0.5 px from the last drag sample (EditorCanvas.tsx:199-205), passing the *incremental* delta and resetting `dragStart` to the current position. Each call lands in `Editor.handleNodeDrag`: ```ts // src/pages/Editor.tsx:243 const handleNodeDrag = useCallback((nodeId, deltaX, deltaY) => { const next = paths.map((path) => { ... }); const mutated = next.some((p, i) => p !== paths[i]); if (mutated) pushHistory(paths); // ← fires on EVERY ~0.5 px move setPaths(next); }, [paths, pushHistory]); ``` A 100 px drag fires ~200 `onNodeDrag` calls. Each one pushes the pre-drag-step `paths` to history. So a single gesture creates ~200 history entries. With `HISTORY_LIMIT = 50` (Editor.tsx:182), the first 150 entries get shifted off, leaving the last 50 to be intermediate-step snapshots. Undo (when #89 lands) would step through ~50 sub-pixel-offset states per gesture. There is no drag-start/drag-end signal between `EditorCanvas` and `Editor`. `EditorCanvas.handleMouseUp` (EditorCanvas.tsx:225-232) only resets local state — it never calls a `onDragEnd` callback. `Editor` therefore cannot distinguish "first pixel of a drag" from "200th pixel of the same drag." **Symptom (once #89 is fixed):** dragging a node 100 px right, then pressing Ctrl+Z once, would only roll back ~0.5 px. The user would need to press Undo ~200 times to fully revert. Most users will assume Undo is broken (the same false-positive symptom #89 already has). **Severity:** medium. Doesn't break anything visible today (because #89 makes Undo a no-op anyway). Once Undo is fixed, this becomes a UX disaster: every drag gesture fills the history buffer with one-snapshot-per-pixel noise, and the user has to mash Ctrl+Z to revert a single move. **Fix consistent with existing patterns:** Add a `dragNodeId: string | null` prop and `onDragStart?: (nodeId: string) => void` / `onDragEnd?: (nodeId: string) => void` callbacks to `EditorCanvasProps` (EditorCanvas.tsx:15-37). In `EditorCanvas.handleMouseDown` (line 169), call `onDragStart(found.node.id)` right after `setIsDragging(true)`. In `handleMouseUp` (line 225-232), call `onDragEnd(dragNodeId)` before resetting state. Then in `Editor.tsx`: ```ts const handleDragStart = useCallback((nodeId: string) => { // capture pre-drag snapshot ONCE dragStartSnapshotRef.current = JSON.parse(JSON.stringify(paths)); }, [paths]); const handleDragEnd = useCallback(() => { if (dragStartSnapshotRef.current) { pushHistory(dragStartSnapshotRef.current); dragStartSnapshotRef.current = null; } }, [pushHistory]); const handleNodeDrag = useCallback((nodeId, dx, dy) => { // NO pushHistory here — only mutate paths setPaths((prev) => prev.map((path) => { const nodeIndex = path.nodes.findIndex((n) => n.id === nodeId); if (nodeIndex === -1) return path; const newNodes = path.nodes.map((n, i) => i === nodeIndex ? { ...n, x: n.x + dx, y: n.y + dy } : n ); return { ...path, nodes: newNodes }; })); }, []); // no path deps — uses functional setState ``` Same shape applies to `handleCanvasClick`/`handleFinishPath` if those also drag (path-drawing mode emits a `onCanvasClick` per click — already pushes per click, which is correct for discrete clicks but not for any continuous gesture). **Cross-reference:** Pair this with #89 (undo/redo setPaths fix). When both land, dragging and undoing should produce one history entry per gesture, with the full drag range as the unit of undo. Today the user sees neither; once #89 lands without this fix, they'll see "Undo moves the node 0.5 px per click."