EditorCanvas.tsx — 3 imported drawing functions never called + no ResizeObserver so canvas doesn't reflow on window resize + onDragEnd prop missing for #90 fix #93

New issue

Closed

opened 2026-09-14 17:38:00 +00:00 by Mark · 0 comments

Mark commented

2026-09-14 17:38:00 +00:00

Owner

Where: src/components/editor/EditorCanvas.tsx.

Bug: Three issues found in the canvas component while walking the Editor.

Bug 1 — Three drawing functions are imported but never called. L4-13:

import {
  drawDotGrid,
  drawHoop,
  drawPath,
  drawUnderlayPath,    // ← imported, never called
  drawSequenceBadge,   // ← imported, never called
  drawAnimationDot,    // ← imported, never called
  drawRubberBand,
  drawNewPathNodes,
  drawSelectionBox,
} from './canvasRenderer';

grep -n 'drawUnderlayPath\|drawSequenceBadge\|drawAnimationDot' src/components/editor/EditorCanvas.tsx returns only the import lines — no call sites. The draw effect (L267-341) calls drawDotGrid, drawHoop, drawPath, drawNewPathNodes, drawRubberBand, drawSelectionBox — never the three unimplemented ones. Consequences:

drawAnimationDot: the user clicks Play in the Sequence panel, the rAF loop runs, sequenceState.progress advances — but no progress dot appears on the canvas because drawAnimationDot is never invoked. Sequence playback has no visual feedback. Same family as #91's dead Sequence branch — both render empty when the user expects animation.
drawSequenceBadge: sequence number badges (the small circle "1", "2", "3" near each path start) never render. The Sequence Order list in the right panel (also dead per #91) is the only place the order would be visible, but the canvas itself doesn't show badges.
drawUnderlayPath: underlay ghost-render never appears. The path.underlay.enabled flag is plumbed but drawUnderlayPath isn't invoked from the draw loop. With the Editor's 'underlay' mode being unreachable (#91), this is moot — but if a future change wires up underlay editing, the visualizer is missing.

These three functions are fully implemented in canvasRenderer.ts (drawUnderlayPath at L241-281, drawSequenceBadge at L283-324, drawAnimationDot at L326-368) but unused. ~130 lines of canvasRenderer code that doesn't reach the screen.

Bug 2 — No ResizeObserver on the canvas container; canvas doesn't reflow when the window resizes. The draw effect at L267-341 has [offset, scale, paths, mode, ...] as deps — none of these change on window resize. The resize() function inside the effect computes canvas dimensions from containerRef.current.clientWidth/Height (L275) but only runs when the deps change.

const resize = () => {
  const w = containerRef.current?.clientWidth ?? 0;
  const h = containerRef.current?.clientHeight ?? 0;
  canvas.width = w * dpr;
  canvas.height = h * dpr;
  // … setTransform(dpr,…) and redraw …
};

Symptom: open the Editor at 1280×800 — canvas correctly fills the container. Resize the browser to 1600×900 — canvas backing store stays at 1280×800 (with empty space at right and bottom). Drawn content stays at the old position. Resizing back to 1280×800 doesn't restore either, because no observer fires.

There is no ResizeObserver, no window.addEventListener('resize', …), no containerRef.current?.addEventListener('resize', …). Grep confirms:

$ grep -n 'ResizeObserver\|addEventListener.*resize\|onResize' src/components/editor/EditorCanvas.tsx
(no matches)

The Workspace canvas (CanvasArea.tsx) likely has the same bug — out of scope for this issue but worth filing separately.

Bug 3 — handleMouseUp (L230-240) calls onDragEnd?.(dragNodeId) but onDragEnd is not declared on EditorCanvasProps (L15-37) and Editor.tsx never passes it. The handler exists:

const handleMouseUp = useCallback(() => {
  if (selectionBox) {
    onSelectionBoxChange(null);
  }
  if (isDragging && dragNodeId) {
    onDragEnd?.(dragNodeId);   // ← always undefined?.() → no-op
  }
  // …
}, [selectionBox, onSelectionBoxChange, isDragging, dragNodeId, onDragEnd]);

onDragEnd?.() is optional-chained; if Editor doesn't pass it, nothing happens. This is the receive-side for the fix proposed in #90 (handleNodeDrag pushes history on every pixel — paired fix is to bracket a single history entry per gesture via onDragStart/onDragEnd callbacks). #90's fix can't land without first adding the onDragEnd (and matching onDragStart) props to EditorCanvasProps. Without them, the Editor can't capture the pre-drag snapshot once.

Fix consistent with existing patterns:

For Bug 1, inside the existing useEffect draw loop, after the path-draw call (L302) but before the sequence-animation handling:

if (sequenceState.isPlaying && paths.length > 0) {
  // Draw the current path with normal stroke, plus an animation dot
  // for the segment interpolated by sequenceState.progress
  const activePath = paths[sequenceState.currentPathIndex] ?? paths[0];
  drawPath(ctx, activePath, offset, scale, { isSelected: true });
  drawAnimationDot(ctx, activePath, offset, scale, sequenceState.progress);
}
// Draw sequence badges for all paths (number + direction arrow)
for (const path of paths) {
  drawSequenceBadge(ctx, path, offset, scale);
}
// Draw underlay ghost (when path.underlay.enabled and underlay has nodes)
for (const path of paths) {
  drawUnderlayPath(ctx, path, offset, scale);
}

For Bug 2, add a ResizeObserver in the same useEffect:

const ro = new ResizeObserver(() => resize());
ro.observe(containerRef.current!);
return () => { ro.disconnect(); cancelAnimationFrame(animId); };

For Bug 3, extend EditorCanvasProps (L15-37):

onDragStart?: (nodeId: string) => void;
onDragEnd?: (nodeId: string) => void;

Call onDragStart(found.node.id) in handleMouseDown at L169 (after setIsDragging(true)). The handleMouseUp side is already done — just needs the prop in Editor.tsx:519-541 to be passed once Editor.tsx adds a dragStartSnapshotRef.

Severity: Bug 1 = medium (visually missing sequence feedback), Bug 2 = medium (canvas out-of-sync after resize), Bug 3 = blocks #90's proper fix.

Cross-references: #89 (Editor undo/redo setPaths), #90 (drag-history pollution), #91 (dead Sequence/Underlay branches).

**Where:** `src/components/editor/EditorCanvas.tsx`. **Bug:** Three issues found in the canvas component while walking the Editor. **Bug 1 — Three drawing functions are imported but never called.** L4-13: ```ts import { drawDotGrid, drawHoop, drawPath, drawUnderlayPath, // ← imported, never called drawSequenceBadge, // ← imported, never called drawAnimationDot, // ← imported, never called drawRubberBand, drawNewPathNodes, drawSelectionBox, } from './canvasRenderer'; ``` `grep -n 'drawUnderlayPath\|drawSequenceBadge\|drawAnimationDot' src/components/editor/EditorCanvas.tsx` returns only the import lines — no call sites. The draw effect (L267-341) calls `drawDotGrid, drawHoop, drawPath, drawNewPathNodes, drawRubberBand, drawSelectionBox` — never the three unimplemented ones. Consequences: - `drawAnimationDot`: the user clicks Play in the Sequence panel, the rAF loop runs, `sequenceState.progress` advances — but no progress dot appears on the canvas because `drawAnimationDot` is never invoked. Sequence playback has no visual feedback. Same family as #91's dead Sequence branch — both render empty when the user expects animation. - `drawSequenceBadge`: sequence number badges (the small circle "1", "2", "3" near each path start) never render. The Sequence Order list in the right panel (also dead per #91) is the only place the order would be visible, but the canvas itself doesn't show badges. - `drawUnderlayPath`: underlay ghost-render never appears. The `path.underlay.enabled` flag is plumbed but `drawUnderlayPath` isn't invoked from the draw loop. With the Editor's 'underlay' mode being unreachable (#91), this is moot — but if a future change wires up underlay editing, the visualizer is missing. These three functions are fully implemented in `canvasRenderer.ts` (drawUnderlayPath at L241-281, drawSequenceBadge at L283-324, drawAnimationDot at L326-368) but unused. ~130 lines of canvasRenderer code that doesn't reach the screen. **Bug 2 — No ResizeObserver on the canvas container; canvas doesn't reflow when the window resizes.** The draw effect at L267-341 has `[offset, scale, paths, mode, ...]` as deps — none of these change on window resize. The `resize()` function inside the effect computes canvas dimensions from `containerRef.current.clientWidth/Height` (L275) but only runs when the deps change. ```ts const resize = () => { const w = containerRef.current?.clientWidth ?? 0; const h = containerRef.current?.clientHeight ?? 0; canvas.width = w * dpr; canvas.height = h * dpr; // … setTransform(dpr,…) and redraw … }; ``` Symptom: open the Editor at 1280×800 — canvas correctly fills the container. Resize the browser to 1600×900 — canvas backing store stays at 1280×800 (with empty space at right and bottom). Drawn content stays at the old position. Resizing back to 1280×800 doesn't restore either, because no observer fires. There is no `ResizeObserver`, no `window.addEventListener('resize', …)`, no `containerRef.current?.addEventListener('resize', …)`. Grep confirms: ``` $ grep -n 'ResizeObserver\|addEventListener.*resize\|onResize' src/components/editor/EditorCanvas.tsx (no matches) ``` The Workspace canvas (`CanvasArea.tsx`) likely has the same bug — out of scope for this issue but worth filing separately. **Bug 3 — `handleMouseUp` (L230-240) calls `onDragEnd?.(dragNodeId)` but `onDragEnd` is not declared on `EditorCanvasProps` (L15-37) and Editor.tsx never passes it.** The handler exists: ```ts const handleMouseUp = useCallback(() => { if (selectionBox) { onSelectionBoxChange(null); } if (isDragging && dragNodeId) { onDragEnd?.(dragNodeId); // ← always undefined?.() → no-op } // … }, [selectionBox, onSelectionBoxChange, isDragging, dragNodeId, onDragEnd]); ``` `onDragEnd?.()` is optional-chained; if Editor doesn't pass it, nothing happens. This is the receive-side for the fix proposed in #90 (handleNodeDrag pushes history on every pixel — paired fix is to bracket a single history entry per gesture via onDragStart/onDragEnd callbacks). **#90's fix can't land without first adding the `onDragEnd` (and matching `onDragStart`) props to `EditorCanvasProps`.** Without them, the Editor can't capture the pre-drag snapshot once. **Fix consistent with existing patterns:** For Bug 1, inside the existing useEffect draw loop, after the path-draw call (L302) but before the sequence-animation handling: ```ts if (sequenceState.isPlaying && paths.length > 0) { // Draw the current path with normal stroke, plus an animation dot // for the segment interpolated by sequenceState.progress const activePath = paths[sequenceState.currentPathIndex] ?? paths[0]; drawPath(ctx, activePath, offset, scale, { isSelected: true }); drawAnimationDot(ctx, activePath, offset, scale, sequenceState.progress); } // Draw sequence badges for all paths (number + direction arrow) for (const path of paths) { drawSequenceBadge(ctx, path, offset, scale); } // Draw underlay ghost (when path.underlay.enabled and underlay has nodes) for (const path of paths) { drawUnderlayPath(ctx, path, offset, scale); } ``` For Bug 2, add a ResizeObserver in the same useEffect: ```ts const ro = new ResizeObserver(() => resize()); ro.observe(containerRef.current!); return () => { ro.disconnect(); cancelAnimationFrame(animId); }; ``` For Bug 3, extend `EditorCanvasProps` (L15-37): ```ts onDragStart?: (nodeId: string) => void; onDragEnd?: (nodeId: string) => void; ``` Call `onDragStart(found.node.id)` in `handleMouseDown` at L169 (after `setIsDragging(true)`). The handleMouseUp side is already done — just needs the prop in Editor.tsx:519-541 to be passed once Editor.tsx adds a `dragStartSnapshotRef`. **Severity:** Bug 1 = medium (visually missing sequence feedback), Bug 2 = medium (canvas out-of-sync after resize), Bug 3 = blocks #90's proper fix. **Cross-references:** #89 (Editor undo/redo setPaths), #90 (drag-history pollution), #91 (dead Sequence/Underlay branches).