useWorkspace reducer history is incomplete — UNDO/REDO restores only stitchObjects + threadStops; UPDATE_OBJECT, ADD/REMOVE/REORDER_THREAD_STOP don't push history at all #92

New issue

Closed

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

Mark commented

2026-09-14 17:37:26 +00:00

Owner

Where: src/components/workspace/useWorkspace.ts (pushHistory at L129-138, UNDO at L314-323, REDO at L325-334, UPDATE_OBJECT at L304-309, ADD_THREAD_STOP at L336-346, REMOVE_THREAD_STOP at L348-349, REORDER_THREAD_STOPS at L351-356).

Bug: Three related gaps in the Workspace reducer's history support.

Bug 1 — pushHistory snapshot is incomplete. L130-133:

const snapshot = {
  stitchObjects: JSON.parse(JSON.stringify(state.stitchObjects)),
  threadStops: JSON.parse(JSON.stringify(state.threadStops)),
};

Only stitchObjects and threadStops are snapshotted. UNDO/REDO (L314-334) restore only those fields. The fields NOT captured/restored:

colorRegions — restored to [] on Undo even though DELETE_SELECTED (L216-237) explicitly removes from both stitchObjects AND colorRegions (L234-235). So after Delete-then-Undo: regions come back but stitch objects also come back (from snapshot), but colorRegions are NOT in the snapshot, so they DON'T come back. The UI shows ghost regions on the canvas with no underlying objects, or vice versa.
designSettings — HOOP SIZE / FABRIC / STABILIZER changes via SET_DESIGN_SETTING (L148-152) don't even push history; they're not snapshot-eligible at all. So changing HOOP from "4x4" to "5x7", then deleting a stitch, then Undo — HOOP stays at "5x7" because (a) the setting change never entered history, (b) even if it had, snapshots don't include it.
viewport (zoom/pan) — SET_VIEWPORT (L186-187) doesn't push history. Same gap.
selectedIds — not snapshotted. After Undo, selection is stale.
appState — not snapshotted. After digitize → Undo, appState stays 'digitized' but stitchObjects are empty.
processingStep/processingMessage — usually cleared together with state changes, less critical.

Bug 2 — Several mutations don't push history at all:

UPDATE_OBJECT (L304-309): setStitchObjects.map(...), no pushHistory call. Changing an object's color/density/visibility/name from the UI is un-undoable.
ADD_THREAD_STOP (L336-346): no pushHistory. Adding a thread stop is un-undoable.
REMOVE_THREAD_STOP (L348-349): no pushHistory. Removing is un-undoable.
REORDER_THREAD_STOPS (L351-356): no pushHistory. Drag-reorder of stops is un-undoable.

Compare with the existing working flow: DELETE_SELECTED (L236), DUPLICATE_SELECTED (L257), GROUP_SELECTED (L286), UNGROUP_SELECTED (L301), LOAD_DESIGN (L375) all wrap their return in pushHistory(...). The thread-stop and UPDATE_OBJECT mutations were forgotten.

Bug 3 — commitDigitizedResult snapshots state that is already on the undo target. L502 calls PUSH_HISTORY after SET_STITCH_OBJECTS/SET_STATE 'digitized'. So history[n+1] = post-digitize state. User pressing Undo after digitize lands on history[n] = pre-digitize — correct. But history[n+1] is never visited except by Redo, and if Redo is pressed immediately after Undo (without any intermediate mutation), it correctly restores history[n+1]. So this specific flow is fine.

However: LOAD_DESIGN (L369-387) calls pushHistory AFTER replacing stitchObjects/threadStops/designSettings, so history captures the LOADED state. Undo from loaded state goes back to pre-load (the state before LOAD_DESIGN). Correct.

Symptom (real, reproducible):

Upload image → digitize → 8 thread stops appear.
Delete a region from canvas (Ctrl+A or via context menu DELETE_SELECTED).
Press Ctrl+Z.
Observe: region comes back in stitchObjects but does NOT come back in colorRegions. Canvas may render the region preview without the underlying geometry, or vice versa, depending on which renderer the user is in.
Press Ctrl+Y (redo).
Region disappears again, but colorRegions is also gone (it was already gone after step2; re-undo doesn't restore).

Symptom 2:

Upload image → digitize.
Click on a stitch object, change its color via RightPanel.
Ctrl+Z.
Color stays changed (UPDATE_OBJECT doesn't push history).

Symptom 3:

Add a manual thread stop via the Thread Palette (the action exists at L336-346, mapped to workspace via thread-stops reducer).
Ctrl+Z.
Thread stop stays (ADD_THREAD_STOP doesn't push history).

Fix consistent with existing patterns: Mirror what useWorkspace.ts itself already does for DELETE_SELECTED / DUPLICATE_SELECTED etc., and what Editor.tsx:184-194 does for the Editor's history:

Expand the snapshot. Replace L130-133 with a broader capture that includes the fields you actually want to roll back. Recommended set: stitchObjects, threadStops, colorRegions, selectedIds, designSettings. appState and processing* are usually fine to leave alone (a digitize→undo cycle restoring appState is rarely the user's intent; they likely want to keep the design but roll back a delete inside the digitize). For viewport, either include it (so pan/zoom rolls back) or document that it's intentionally not undoable.
```
const snapshot = {
  stitchObjects: JSON.parse(JSON.stringify(state.stitchObjects)),
  threadStops: JSON.parse(JSON.stringify(state.threadStops)),
  colorRegions: JSON.parse(JSON.stringify(state.colorRegions)),
  selectedIds: [...state.selectedIds],
  designSettings: { ...state.designSettings },
};
```
Mirror the new fields in UNDO/REDO at L319-322 and L330-332.
Push history for the missed mutations. L308 (UPDATE_OBJECT), L345 (ADD_THREAD_STOP), L349 (REMOVE_THREAD_STOP), L355 (REORDER_THREAD_STOPS): wrap the return in pushHistory(...) like the four working actions do.
Add tests. useWorkspace.test.ts should cover:
- DELETE_SELECTED then UNDO → both stitchObjects AND colorRegions restored.
- UPDATE_OBJECT then UNDO → property rolled back.
- ADD_THREAD_STOP then UNDO → stop removed.

Severity: medium-high. Users can lose data via the 'Delete → undo' flow (color regions vanish from canvas preview despite objects coming back), and several common UI flows (color change, thread stop add) are un-undoable despite the UI suggesting Ctrl+Z should work. Same family as #89 (Editor undo/redo setPaths missing).

Cross-references: #89 (Editor undo/redo is non-functional — handleUndo/handleRedo never setPaths). #63 (Workspace had no Save Session button — saveCurrentDesign since added). #74 (Gallery has no Open in Workspace action — uses LOAD_DESIGN which DOES push history correctly).

**Where:** `src/components/workspace/useWorkspace.ts` (`pushHistory` at L129-138, `UNDO` at L314-323, `REDO` at L325-334, `UPDATE_OBJECT` at L304-309, `ADD_THREAD_STOP` at L336-346, `REMOVE_THREAD_STOP` at L348-349, `REORDER_THREAD_STOPS` at L351-356). **Bug:** Three related gaps in the Workspace reducer's history support. **Bug 1 — `pushHistory` snapshot is incomplete.** L130-133: ```ts const snapshot = { stitchObjects: JSON.parse(JSON.stringify(state.stitchObjects)), threadStops: JSON.parse(JSON.stringify(state.threadStops)), }; ``` Only `stitchObjects` and `threadStops` are snapshotted. `UNDO`/`REDO` (L314-334) restore only those fields. The fields NOT captured/restored: - `colorRegions` — restored to `[]` on Undo even though `DELETE_SELECTED` (L216-237) explicitly removes from both `stitchObjects` AND `colorRegions` (L234-235). So after Delete-then-Undo: regions come back but stitch objects also come back (from snapshot), but `colorRegions` are NOT in the snapshot, so they DON'T come back. The UI shows ghost regions on the canvas with no underlying objects, or vice versa. - `designSettings` — HOOP SIZE / FABRIC / STABILIZER changes via `SET_DESIGN_SETTING` (L148-152) don't even push history; they're not snapshot-eligible at all. So changing HOOP from "4x4" to "5x7", then deleting a stitch, then Undo — HOOP stays at "5x7" because (a) the setting change never entered history, (b) even if it had, snapshots don't include it. - `viewport` (zoom/pan) — `SET_VIEWPORT` (L186-187) doesn't push history. Same gap. - `selectedIds` — not snapshotted. After Undo, selection is stale. - `appState` — not snapshotted. After digitize → Undo, appState stays 'digitized' but stitchObjects are empty. - `processingStep`/`processingMessage` — usually cleared together with state changes, less critical. **Bug 2 — Several mutations don't push history at all:** - `UPDATE_OBJECT` (L304-309): `setStitchObjects.map(...)`, no `pushHistory` call. Changing an object's color/density/visibility/name from the UI is un-undoable. - `ADD_THREAD_STOP` (L336-346): no `pushHistory`. Adding a thread stop is un-undoable. - `REMOVE_THREAD_STOP` (L348-349): no `pushHistory`. Removing is un-undoable. - `REORDER_THREAD_STOPS` (L351-356): no `pushHistory`. Drag-reorder of stops is un-undoable. Compare with the existing working flow: `DELETE_SELECTED` (L236), `DUPLICATE_SELECTED` (L257), `GROUP_SELECTED` (L286), `UNGROUP_SELECTED` (L301), `LOAD_DESIGN` (L375) all wrap their return in `pushHistory(...)`. The thread-stop and UPDATE_OBJECT mutations were forgotten. **Bug 3 — `commitDigitizedResult` snapshots state that is already on the undo target.** L502 calls `PUSH_HISTORY` after `SET_STITCH_OBJECTS`/`SET_STATE 'digitized'`. So history[n+1] = post-digitize state. User pressing Undo after digitize lands on history[n] = pre-digitize — correct. But history[n+1] is **never visited except by Redo**, and if Redo is pressed immediately after Undo (without any intermediate mutation), it correctly restores history[n+1]. So this specific flow is fine. However: `LOAD_DESIGN` (L369-387) calls `pushHistory` AFTER replacing stitchObjects/threadStops/designSettings, so history captures the LOADED state. Undo from loaded state goes back to pre-load (the state before LOAD_DESIGN). Correct. **Symptom (real, reproducible):** 1. Upload image → digitize → 8 thread stops appear. 2. Delete a region from canvas (Ctrl+A or via context menu DELETE_SELECTED). 3. Press Ctrl+Z. 4. Observe: region comes back in `stitchObjects` but does NOT come back in `colorRegions`. Canvas may render the region preview without the underlying geometry, or vice versa, depending on which renderer the user is in. 5. Press Ctrl+Y (redo). 6. Region disappears again, but `colorRegions` is also gone (it was already gone after step2; re-undo doesn't restore). **Symptom 2:** 1. Upload image → digitize. 2. Click on a stitch object, change its color via RightPanel. 3. Ctrl+Z. 4. Color stays changed (UPDATE_OBJECT doesn't push history). **Symptom 3:** 1. Add a manual thread stop via the Thread Palette (the action exists at L336-346, mapped to workspace via thread-stops reducer). 2. Ctrl+Z. 3. Thread stop stays (ADD_THREAD_STOP doesn't push history). **Fix consistent with existing patterns:** Mirror what `useWorkspace.ts` itself already does for `DELETE_SELECTED` / `DUPLICATE_SELECTED` etc., and what `Editor.tsx:184-194` does for the Editor's history: 1. **Expand the snapshot.** Replace L130-133 with a broader capture that includes the fields you actually want to roll back. Recommended set: `stitchObjects, threadStops, colorRegions, selectedIds, designSettings`. `appState` and `processing*` are usually fine to leave alone (a digitize→undo cycle restoring appState is rarely the user's intent; they likely want to keep the design but roll back a delete inside the digitize). For `viewport`, either include it (so pan/zoom rolls back) or document that it's intentionally not undoable. ```ts const snapshot = { stitchObjects: JSON.parse(JSON.stringify(state.stitchObjects)), threadStops: JSON.parse(JSON.stringify(state.threadStops)), colorRegions: JSON.parse(JSON.stringify(state.colorRegions)), selectedIds: [...state.selectedIds], designSettings: { ...state.designSettings }, }; ``` Mirror the new fields in UNDO/REDO at L319-322 and L330-332. 2. **Push history for the missed mutations.** L308 (UPDATE_OBJECT), L345 (ADD_THREAD_STOP), L349 (REMOVE_THREAD_STOP), L355 (REORDER_THREAD_STOPS): wrap the return in `pushHistory(...)` like the four working actions do. 3. **Add tests.** `useWorkspace.test.ts` should cover: - `DELETE_SELECTED` then `UNDO` → both `stitchObjects` AND `colorRegions` restored. - `UPDATE_OBJECT` then `UNDO` → property rolled back. - `ADD_THREAD_STOP` then `UNDO` → stop removed. **Severity:** medium-high. Users can lose data via the 'Delete → undo' flow (color regions vanish from canvas preview despite objects coming back), and several common UI flows (color change, thread stop add) are un-undoable despite the UI suggesting Ctrl+Z should work. Same family as #89 (Editor undo/redo setPaths missing). **Cross-references:** #89 (Editor undo/redo is non-functional — handleUndo/handleRedo never setPaths). #63 (Workspace had no Save Session button — saveCurrentDesign since added). #74 (Gallery has no Open in Workspace action — uses LOAD_DESIGN which DOES push history correctly).