Workspace: 5 of 6 LeftToolbar tools are cosmetic (select/auto-digitize/manual-stitch/text/shapes never read); bring-to-front/send-to-back skip pushHistory; BottomThreadBar selectedColorId is always null #95

New issue

Closed

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

Mark commented

2026-09-14 17:40:05 +00:00

Owner

Where: src/pages/Workspace.tsx, src/components/workspace/CanvasArea.tsx, src/components/workspace/BottomThreadBar.tsx, src/components/workspace/LeftToolbar.tsx.

Bug: Three independent issues in the Workspace surface, all same orphan-state family as #91 / #94 / #88.

Bug 1 — state.activeTool is only consumed for 'zoom'; the other 5 tools are decorative.

The Tool type (src/components/workspace/types.ts:9) enumerates six values: 'select' | 'auto-digitize' | 'manual-stitch' | 'text' | 'shapes' | 'zoom'. The LeftToolbar (src/components/workspace/LeftToolbar.tsx:28-33) renders a button for each. The reducer handles SET_TOOL and stores it (useWorkspace.ts:142-143). But grep for actual consumers:

$ grep -rn "activeTool" src/ | grep -v test
src/components/workspace/types.ts:154:  activeTool: Tool;
src/components/workspace/LeftToolbar.tsx:19,37,60:  // reads for isActive highlight
src/components/workspace/CanvasArea.tsx:165:    } else if (dragRef.current.spaceDown || state.activeTool === 'zoom') {
src/components/workspace/CanvasArea.tsx:175:  }, [viewport, dispatch, state.activeTool]);
src/components/workspace/useWorkspace.ts:51,85,142-143:  // initial state + reducer case
src/pages/Workspace.tsx:46,88,257-258:  // destructure + dispatch + pass to LeftToolbar

Only ONE production read: CanvasArea.tsx:165 checking state.activeTool === 'zoom' for wheel-pan-vs-zoom behaviour. The five tools 'select', 'auto-digitize', 'manual-stitch', 'text', 'shapes' are never consumed.

So:

Click Select in the toolbar → state.activeTool becomes 'select'. The canvas's hit-testing doesn't branch on this. The cursor stays the same. No selection model kicks in.
Click Auto-Digitize → state becomes 'auto-digitize'. The actual auto-digitize button in the right panel is what triggers the digitize; this tool is dead.
Click Manual Stitch → no behavior.
Click Text → no behavior. (No text tool implementation anywhere.)
Click Shapes → no behavior. (No shape tool.)
Click Zoom → wheel-pan when over canvas (only thing that actually works).

The toolbar buttons highlight (via isActive = activeTool === tool.id, LeftToolbar.tsx:60) so the user sees the click registered, but no UI consequence follows. Same orphan-UI family as #91 (EditorRightPanel dead branches), #94 (EditorThreadBar static data), #88 (region-classifier orphan).

Bug 2 — handleBringToFront and handleSendToBack (Workspace.tsx:224-235) dispatch SET_STITCH_OBJECTS directly without pushHistory.

const handleBringToFront = useCallback(() => {
  const selected = stitchObjects.filter((o) => selectedIds.includes(o.id));
  const rest = stitchObjects.filter((o) => !selectedIds.includes(o.id));
  dispatch({ type: 'SET_STITCH_OBJECTS', objects: [...rest, ...selected] });
}, [stitchObjects, selectedIds, dispatch]);

SET_STITCH_OBJECTS (useWorkspace.ts:163-164) just replaces the array — no history. Compare with DELETE_SELECTED (L236), DUPLICATE_SELECTED (L257), GROUP_SELECTED (L286), UNGROUP_SELECTED (L301) which all wrap the return in pushHistory(...). Z-order changes are un-undoable. Same family as #92 (UPDATE_OBJECT, ADD/REMOVE/REORDER_THREAD_STOP also skip pushHistory).

Bug 3 — BottomThreadBar.selectedColorId is always null.

Workspace.tsx:284: <BottomThreadBar ... selectedColorId={null} />. The bar accepts selectedColorId: string | null (BottomThreadBar.tsx:12) and uses it to compute isSelected = selectedColorId === stop.colorId (L62) for visual highlighting. Since null never equals a stop.colorId, no thread stop is ever visually selected in the bar. The handleSelectStop callback (Workspace.tsx:181-187) does fire — it dispatches SELECT for objects matching the clicked color — but the bar can't reflect "you picked blue" because selectedColorId is never updated.

Compare with EditorThreadBar (#94) which actually tracks selectedThreadIndex. The Workspace's BottomThreadBar should either:

accept the dispatched SELECT and reflect "currently selected objects' color", or
manage its own selectedColorId state via useState in the bar and update it on click.

Fix consistent with existing patterns:

For Bug 1, two paths:

Honest (cheaper): in LeftToolbar.tsx:28-33, drop the four tools with no consumers ('auto-digitize', 'manual-stitch', 'text', 'shapes'). Keep 'select' (the default — and as a placeholder for any future selection UI) and 'zoom' (the only one with real behavior). Trim the Tool type to 'select' | 'zoom'. The keyboard shortcuts in Workspace.tsx:75-82 also need pruning (drop a, s, t, r).
Wire (longer): add the actual tool behaviors — make 'manual-stitch' a path-draw mode, 'auto-digitize' a shortcut for the FAB, 'text' and 'shapes' real tools. That's a feature, not a bug fix.

Recommended: Honest path. ~20 LOC removed, no UX regression (the dead buttons are misleading anyway).

For Bug 2, wrap each return in pushHistory(...) like the four working actions do. Two-line change.

For Bug 3, change selectedColorId={null} (Workspace.tsx:284) to selectedColorId={selectedIds.length > 0 ? (stitchObjects.find((o) => selectedIds.includes(o.id))?.colorId ?? null) : null}. The bar's click handler can also update local selectedColorId for selection-mode UX, but at minimum the prop should reflect current selection.

Severity: Bug 1 = medium (misleading toolbar; 4 buttons do nothing), Bug 2 = low (a single un-undoable action), Bug 3 = low (visual highlight never works).

Cross-references: #91 (EditorRightPanel dead Sequence/Underlay branches), #92 (useWorkspace reducer history gaps — same UPDATE_OBJECT-style gap), #94 (EditorThreadBar static data).

**Where:** `src/pages/Workspace.tsx`, `src/components/workspace/CanvasArea.tsx`, `src/components/workspace/BottomThreadBar.tsx`, `src/components/workspace/LeftToolbar.tsx`. **Bug:** Three independent issues in the Workspace surface, all same orphan-state family as #91 / #94 / #88. **Bug 1 — `state.activeTool` is only consumed for 'zoom'; the other 5 tools are decorative.** The `Tool` type (`src/components/workspace/types.ts:9`) enumerates six values: `'select' | 'auto-digitize' | 'manual-stitch' | 'text' | 'shapes' | 'zoom'`. The `LeftToolbar` (`src/components/workspace/LeftToolbar.tsx:28-33`) renders a button for each. The reducer handles `SET_TOOL` and stores it (`useWorkspace.ts:142-143`). But grep for actual consumers: ``` $ grep -rn "activeTool" src/ | grep -v test src/components/workspace/types.ts:154: activeTool: Tool; src/components/workspace/LeftToolbar.tsx:19,37,60: // reads for isActive highlight src/components/workspace/CanvasArea.tsx:165: } else if (dragRef.current.spaceDown || state.activeTool === 'zoom') { src/components/workspace/CanvasArea.tsx:175: }, [viewport, dispatch, state.activeTool]); src/components/workspace/useWorkspace.ts:51,85,142-143: // initial state + reducer case src/pages/Workspace.tsx:46,88,257-258: // destructure + dispatch + pass to LeftToolbar ``` Only ONE production read: `CanvasArea.tsx:165` checking `state.activeTool === 'zoom'` for wheel-pan-vs-zoom behaviour. The five tools `'select'`, `'auto-digitize'`, `'manual-stitch'`, `'text'`, `'shapes'` are never consumed. So: - Click **Select** in the toolbar → state.activeTool becomes 'select'. The canvas's hit-testing doesn't branch on this. The cursor stays the same. No selection model kicks in. - Click **Auto-Digitize** → state becomes 'auto-digitize'. The actual auto-digitize button in the right panel is what triggers the digitize; this tool is dead. - Click **Manual Stitch** → no behavior. - Click **Text** → no behavior. (No text tool implementation anywhere.) - Click **Shapes** → no behavior. (No shape tool.) - Click **Zoom** → wheel-pan when over canvas (only thing that actually works). The toolbar buttons highlight (via `isActive = activeTool === tool.id`, LeftToolbar.tsx:60) so the user sees the click registered, but no UI consequence follows. Same orphan-UI family as #91 (EditorRightPanel dead branches), #94 (EditorThreadBar static data), #88 (region-classifier orphan). **Bug 2 — `handleBringToFront` and `handleSendToBack` (Workspace.tsx:224-235) dispatch `SET_STITCH_OBJECTS` directly without `pushHistory`.** ```ts const handleBringToFront = useCallback(() => { const selected = stitchObjects.filter((o) => selectedIds.includes(o.id)); const rest = stitchObjects.filter((o) => !selectedIds.includes(o.id)); dispatch({ type: 'SET_STITCH_OBJECTS', objects: [...rest, ...selected] }); }, [stitchObjects, selectedIds, dispatch]); ``` `SET_STITCH_OBJECTS` (useWorkspace.ts:163-164) just replaces the array — no history. Compare with `DELETE_SELECTED` (L236), `DUPLICATE_SELECTED` (L257), `GROUP_SELECTED` (L286), `UNGROUP_SELECTED` (L301) which all wrap the return in `pushHistory(...)`. Z-order changes are un-undoable. Same family as #92 (UPDATE_OBJECT, ADD/REMOVE/REORDER_THREAD_STOP also skip pushHistory). **Bug 3 — `BottomThreadBar.selectedColorId` is always `null`.** `Workspace.tsx:284`: `<BottomThreadBar ... selectedColorId={null} />`. The bar accepts `selectedColorId: string | null` (`BottomThreadBar.tsx:12`) and uses it to compute `isSelected = selectedColorId === stop.colorId` (L62) for visual highlighting. Since `null` never equals a `stop.colorId`, **no thread stop is ever visually selected in the bar**. The `handleSelectStop` callback (Workspace.tsx:181-187) does fire — it dispatches `SELECT` for objects matching the clicked color — but the bar can't reflect "you picked blue" because `selectedColorId` is never updated. Compare with `EditorThreadBar` (#94) which actually tracks `selectedThreadIndex`. The Workspace's `BottomThreadBar` should either: - accept the dispatched `SELECT` and reflect "currently selected objects' color", or - manage its own `selectedColorId` state via `useState` in the bar and update it on click. **Fix consistent with existing patterns:** For Bug 1, two paths: - **Honest (cheaper):** in `LeftToolbar.tsx:28-33`, drop the four tools with no consumers ('auto-digitize', 'manual-stitch', 'text', 'shapes'). Keep 'select' (the default — and as a placeholder for any future selection UI) and 'zoom' (the only one with real behavior). Trim the `Tool` type to `'select' | 'zoom'`. The keyboard shortcuts in Workspace.tsx:75-82 also need pruning (drop `a, s, t, r`). - **Wire (longer):** add the actual tool behaviors — make 'manual-stitch' a path-draw mode, 'auto-digitize' a shortcut for the FAB, 'text' and 'shapes' real tools. That's a feature, not a bug fix. **Recommended:** Honest path. ~20 LOC removed, no UX regression (the dead buttons are misleading anyway). For Bug 2, wrap each return in `pushHistory(...)` like the four working actions do. Two-line change. For Bug 3, change `selectedColorId={null}` (Workspace.tsx:284) to `selectedColorId={selectedIds.length > 0 ? (stitchObjects.find((o) => selectedIds.includes(o.id))?.colorId ?? null) : null}`. The bar's click handler can also update local selectedColorId for selection-mode UX, but at minimum the prop should reflect current selection. **Severity:** Bug 1 = medium (misleading toolbar; 4 buttons do nothing), Bug 2 = low (a single un-undoable action), Bug 3 = low (visual highlight never works). **Cross-references:** #91 (EditorRightPanel dead Sequence/Underlay branches), #92 (useWorkspace reducer history gaps — same UPDATE_OBJECT-style gap), #94 (EditorThreadBar static data).