lhumina_code/hero_whiteboard

Fork 0

Create-then-move undo deletes the object instead of undoing the move (all object types); pending-snapshot leak on cancelled drag #194

New issue

Open

opened 2026-05-17 09:59:51 +00:00 by AhmedHanafy725 · 3 comments

AhmedHanafy725 commented

2026-05-17 09:59:51 +00:00

Member

Summary

On the whiteboard, creating an object and then moving it produces a broken undo: the first Ctrl+Z deletes the object instead of undoing the move. This affects all draggable object types (sticky, text, shape, frame, image, drawing, document, and the widget types).

Reproduction:

Create a shape (or any object) via the toolbar.
Drag it somewhere on the board.
Press Ctrl+Z.
Expected: the object returns to its original position. Actual: the object is deleted.

Root cause

Each object's create function assigns a temporary id (nextTempId()) and wires drag-history handlers that close over that creation-time id, e.g. in createShape (crates/hero_whiteboard_admin/static/web/js/whiteboard/objects.js):

var id = opts.id || nextTempId();
...
group.on('dragstart', function() { WhiteboardHistory.snapshotBefore(id); });
group.on('dragend',   function() { WhiteboardHistory.commitUpdate(id); ... });
...
WhiteboardHistory.push({ type: 'create', id: id });
WhiteboardSync.onCreate(group);

WhiteboardSync.onCreate persists the object; the server assigns a real id and objects.js remapId(tempId, serverId) rekeys the objects map, the Konva node id, and the stacked history actions.

After that remap the drag handlers still reference the stale temp id via closure:

dragstart -> snapshotBefore(tempId) -> snapshot(tempId) looks up WhiteboardObjects.getObject(tempId), which no longer exists (object is registered under the server id) -> stores undefined in _pendingBefore[tempId].
dragend -> commitUpdate(tempId) -> before is undefined -> early-returns -> no update history action is pushed.

So the move is never recorded. The undo stack contains only the create action (whose id was remapped correctly), so the first undo deletes the object.

Handlers that already read the live id at event time (group.id() / this.id()) work correctly; the per-type create handlers that close over id do not. The fix is to resolve the live id at event time in every drag-history handler across all object types.

Also in scope (same subsystem): pending-snapshot leak on cancelled drag

WhiteboardHistory.snapshotBefore(id) stores _pendingBefore[id]; only commitUpdate(id) clears it. If a drag starts but never cleanly ends (window blur, ESC, tab switch, aborted drag), commitUpdate never runs, so:

_pendingBefore[id] leaks (one dead entry per cancelled drag, unbounded).
A later mutation path that calls commitUpdate(id) without a fresh matching snapshotBefore diffs against the stale snapshot, producing an undo step that spans far more change than the user's action.

This affects the same dragstart/dragend pattern across kanban, mindmap, calendar, webframe, sticky, text, shape, document, and drawing. The pending snapshot should be discarded on drag-abort (e.g. window blur / visibilitychange / ESC / Konva drag abort).

Expected

Create then move any object -> Ctrl+Z undoes the move (object returns to original position); a second Ctrl+Z undoes the create.
Works identically for every draggable object type, including objects freshly created in the same session (before/after the temp->server id remap).
A cancelled/interrupted drag leaves no stale entry in the history pending map and does not corrupt the next undo step.

Affected

crates/hero_whiteboard_admin/static/web/js/whiteboard/objects.js — drag-history handlers in the per-type create functions.
crates/hero_whiteboard_admin/static/web/js/whiteboard/history.js — snapshotBefore / commitUpdate / _pendingBefore lifecycle (drag-abort cleanup).
Analogous dragstart/dragend handlers in the widget modules (kanban, mindmap, calendar, webframe) if they close over creation-time ids.

## Summary On the whiteboard, creating an object and then moving it produces a broken undo: the first Ctrl+Z **deletes the object** instead of undoing the move. This affects all draggable object types (sticky, text, shape, frame, image, drawing, document, and the widget types). Reproduction: 1. Create a shape (or any object) via the toolbar. 2. Drag it somewhere on the board. 3. Press Ctrl+Z. 4. Expected: the object returns to its original position. Actual: the object is deleted. ## Root cause Each object's create function assigns a temporary id (`nextTempId()`) and wires drag-history handlers that **close over that creation-time `id`**, e.g. in `createShape` (`crates/hero_whiteboard_admin/static/web/js/whiteboard/objects.js`): ```js var id = opts.id || nextTempId(); ... group.on('dragstart', function() { WhiteboardHistory.snapshotBefore(id); }); group.on('dragend', function() { WhiteboardHistory.commitUpdate(id); ... }); ... WhiteboardHistory.push({ type: 'create', id: id }); WhiteboardSync.onCreate(group); ``` `WhiteboardSync.onCreate` persists the object; the server assigns a real id and `objects.js remapId(tempId, serverId)` rekeys the objects map, the Konva node id, and the stacked history actions. After that remap the drag handlers still reference the stale temp `id` via closure: - `dragstart` -> `snapshotBefore(tempId)` -> `snapshot(tempId)` looks up `WhiteboardObjects.getObject(tempId)`, which no longer exists (object is registered under the server id) -> stores `undefined` in `_pendingBefore[tempId]`. - `dragend` -> `commitUpdate(tempId)` -> `before` is `undefined` -> early-returns -> **no `update` history action is pushed**. So the move is never recorded. The undo stack contains only the `create` action (whose id was remapped correctly), so the first undo deletes the object. Handlers that already read the live id at event time (`group.id()` / `this.id()`) work correctly; the per-type create handlers that close over `id` do not. The fix is to resolve the live id at event time in every drag-history handler across all object types. ## Also in scope (same subsystem): pending-snapshot leak on cancelled drag `WhiteboardHistory.snapshotBefore(id)` stores `_pendingBefore[id]`; only `commitUpdate(id)` clears it. If a drag starts but never cleanly ends (window blur, ESC, tab switch, aborted drag), `commitUpdate` never runs, so: 1. `_pendingBefore[id]` leaks (one dead entry per cancelled drag, unbounded). 2. A later mutation path that calls `commitUpdate(id)` without a fresh matching `snapshotBefore` diffs against the **stale** snapshot, producing an undo step that spans far more change than the user's action. This affects the same dragstart/dragend pattern across kanban, mindmap, calendar, webframe, sticky, text, shape, document, and drawing. The pending snapshot should be discarded on drag-abort (e.g. window blur / visibilitychange / ESC / Konva drag abort). ## Expected - Create then move any object -> Ctrl+Z undoes the move (object returns to original position); a second Ctrl+Z undoes the create. - Works identically for every draggable object type, including objects freshly created in the same session (before/after the temp->server id remap). - A cancelled/interrupted drag leaves no stale entry in the history pending map and does not corrupt the next undo step. ## Affected - `crates/hero_whiteboard_admin/static/web/js/whiteboard/objects.js` — drag-history handlers in the per-type create functions. - `crates/hero_whiteboard_admin/static/web/js/whiteboard/history.js` — `snapshotBefore` / `commitUpdate` / `_pendingBefore` lifecycle (drag-abort cleanup). - Analogous dragstart/dragend handlers in the widget modules (kanban, mindmap, calendar, webframe) if they close over creation-time ids.

AhmedHanafy725 commented

2026-05-17 10:04:56 +00:00

Author

Member

Implementation Spec for Issue #194

Objective

Fix two bugs in the Hero Whiteboard client-side undo subsystem (JS only, no backend changes):

Bug A: After create-then-move, the first Ctrl+Z deletes the object instead of undoing the move. Cause: per-create-function dragstart/dragend handlers close over the creation-time temp id; after remapId swaps temp to server id, snapshotBefore(tempId)/commitUpdate(tempId) operate on a nonexistent key, so no update action is pushed and only the create remains on the stack.
Bug B: WhiteboardHistory._pendingBefore[id] leaks when a drag starts but never cleanly ends (window blur, tab switch, ESC, Konva drag abort). The stale snapshot is later diffed against a fresh commitUpdate, producing a wrong/oversized undo step plus unbounded memory growth.

Requirements

Every object-drag history handler must resolve the live Konva id at event time (group.id() / this.id()), never a closed-over creation-time id.
Audit and fix all object types in objects.js and all four widget modules. connectors.js is already correct (reference pattern) and must not be changed.
Add a cancel/clear entry point to history.js and wire global drag-abort so a cancelled drag cannot poison the next edit. Clear-all-pending is preferred (simplest; only one object drags at a time).
Preserve existing behavior: history suppression during server load, batch grouping for frame multi-child moves, transform/editText handlers (already correct), recomputeParentFrame sync calls.

Files to Modify/Create

objects.js - fix closed-over id in 7 create-function drag handlers + the frame handler.
kanban.js - fix closed-over id in the top-level kanban group create handler.
mindmap.js - fix closed-over id in the create handler.
calendar.js - fix closed-over id in the create handler.
webframe.js - fix closed-over id in the history calls of the create handler (overlay-id handling unchanged).
history.js - add cancelPending(id) + cancelAllPending() and expose them.
tools.js - wire global drag-abort listeners (window blur, document visibilitychange, ESC keydown) to cancelAllPending().

Implementation Plan

Steps 1-5 (Bug A) are independent and may run in parallel. Steps 6-7 (Bug B) are sequential with each other, independent of 1-5.

Step 1: Fix closed-over temp id in objects.js create-function drag handlers

Files: crates/hero_whiteboard_admin/static/web/js/whiteboard/objects.js
In each dragstart/dragend pair, replace the closed-over id passed to snapshotBefore(id)/commitUpdate(id) with the live id group.id(). Do not change WhiteboardSync.onUpdate/recomputeParentFrame lines, and do not change WhiteboardHistory.push({type:'create', id:id}).
Functions/lines: createStickyNote (dragstart 228, dragend 231); createTextBox (338, 341); createShape (885, 888); createDocument (1777, 1780); createImage (1958, 1961); createEmoji (2078, 2081); createDrawing (2179, 2182).
Dependencies: none

Step 2: Fix the frame create handler (multi-child snapshot) in objects.js

Files: crates/hero_whiteboard_admin/static/web/js/whiteboard/objects.js
createFrame: line 1163 snapshotBefore(id) to snapshotBefore(group.id()); line 1199 (inside the WhiteboardHistory.batch callback) commitUpdate(id) to commitUpdate(group.id()). Lines 1177 snapshotBefore(child.id()) and 1201 commitUpdate(capturedChildren[i].node.id()) already use live ids; leave unchanged. Do not change the batch wrapper, dragmove, or sync calls.
Dependencies: none

Step 3: Fix kanban.js top-level create handler

Files: crates/hero_whiteboard_admin/static/web/js/whiteboard/kanban.js
createKanban dragstart line 81 snapshotBefore(id) to snapshotBefore(group.id()). dragend (83-88) calls persistKanbanMutation(group) which already does commitUpdate(group.id()) - leave it. Other kanban handlers already use group.id() live - leave unchanged.
Dependencies: none

Step 4: Fix mindmap.js and calendar.js create handlers

Files: crates/hero_whiteboard_admin/static/web/js/whiteboard/mindmap.js, crates/hero_whiteboard_admin/static/web/js/whiteboard/calendar.js
mindmap.js create handler: dragstart line 87 snapshotBefore(id) to snapshotBefore(group.id()); dragend line 90 commitUpdate(id) to commitUpdate(group.id()). Other mindmap snapshotBefore(group.id()) calls already correct.
calendar.js create handler: dragstart line 40 snapshotBefore(id) to snapshotBefore(group.id()); dragend line 43 commitUpdate(id) to commitUpdate(group.id()). Lines 54 and 392-424 already correct.
Dependencies: none

Step 5: Fix webframe.js create handler (history calls only)

Files: crates/hero_whiteboard_admin/static/web/js/whiteboard/webframe.js
createWebframe: line 77 snapshotBefore(id) to snapshotBefore(group.id()); line 82 commitUpdate(id) to commitUpdate(group.id()). Leave hideOverlay(id) (line 78) and showOverlay(id, group) (line 81) using the closed-over id - the overlay map is separately remapped via WhiteboardWebframe.remapOverlay and stage overlay listeners are re-bound; do NOT touch them. Only the two history calls are the bug.
Dependencies: none

Step 6: Add cancel/clear entry points to history.js

Files: crates/hero_whiteboard_admin/static/web/js/whiteboard/history.js
Add near commitUpdate: function cancelPending(id){ delete _pendingBefore[id]; } and function cancelAllPending(){ _pendingBefore = {}; }. _pendingBefore is var so reassignment is valid. Add both to the returned API object alongside snapshotBefore/commitUpdate. Do not change snapshotBefore/commitUpdate/remapId/snapshot/setEnabled logic.
Dependencies: none

Step 7: Wire global drag-abort to cancelAllPending in tools.js

Files: crates/hero_whiteboard_admin/static/web/js/whiteboard/tools.js
In init(), at the object-layer drag tracking block (~lines 315-327 where isDraggingObject is set), add abort wiring reusing isDraggingObject:

window blur: if isDraggingObject and WhiteboardHistory.cancelAllPending, call it, reset isDraggingObject=false.
document visibilitychange: if document.hidden and isDraggingObject, same cleanup.
keydown Escape: if isDraggingObject, call WhiteboardHistory.cancelAllPending() then stage.stopDrag() (cancel-then-stop so ESC = abort with no history), reset flag.
Guard all calls with typeof WhiteboardHistory !== 'undefined' && WhiteboardHistory.cancelAllPending. Never call cancelAllPending on the normal layer dragend path - only on abort signals, so legitimate moves still reach commitUpdate.
Dependencies: Step 6

Acceptance Criteria

Create a sticky via toolbar, drag it, Ctrl+Z -> object returns to original position (move undone), not deleted. Second Ctrl+Z -> object deleted (create undone). Redo restores both in order.
Same for: text, shape, frame (including a frame containing a pre-existing child - child also returns), image, drawing, emoji, document.
Same for widgets: kanban, mindmap, calendar, webframe.
Frame drag-with-children after create: single Ctrl+Z reverts frame AND all captured children together (batch intact).
Start dragging a freshly created object, blur/tab-switch mid-drag, return, make a small move, dragend -> undo step reflects only the small move; _pendingBefore empty after abort.
Start a drag, press ESC mid-drag -> drag aborts, no poisoned history.
Page refresh / server load still produces an unaffected undo stack - first Ctrl+Z after load does not delete a server-loaded object.
One create+move produces exactly one create and one update (or one batch for frame).
Webframe overlay still tracks the iframe after create-then-move and after remap.

Notes

Reading group.id() at event time is correct for both remap orderings because remapId calls obj.group.id(strServerId), so the Konva node always carries the current id when a drag fires. The create-time push uses the temp id intentionally and is fixed by WhiteboardHistory.remapId; do not change it.
The existing _pendingBefore remap move in history.js (temp to server key) only helps if a drag is in flight across the remap (rare); leave as-is, it complements the live-id fix.
Frame: only the frame's own id (1163, 1199) was stale; child ids already live. Keep the batch wrapper so one Ctrl+Z reverts the whole group.
Transform/editText/resize handlers already resolve group.id() live and are NOT part of this bug; do not modify.
cancelAllPending must never run on the normal dragend path, only on abort signals, else a legitimate move could be silently dropped.
During board load app.js disables sync+history; cancelPending/cancelAllPending are pure deletes safe regardless of _enabled. Path unchanged.
Minimal per-site edits chosen over a shared helper: each dragend does type-specific work (overlay, frame batch, persistKanbanMutation, recomputeParentFrame); a helper would be a larger, riskier change. ~16 single-token edits matching the established correct convention. A one-line comment at the first fixed site is recommended to discourage regression.

## Implementation Spec for Issue #194 ### Objective Fix two bugs in the Hero Whiteboard client-side undo subsystem (JS only, no backend changes): - Bug A: After create-then-move, the first Ctrl+Z deletes the object instead of undoing the move. Cause: per-create-function dragstart/dragend handlers close over the creation-time temp id; after remapId swaps temp to server id, snapshotBefore(tempId)/commitUpdate(tempId) operate on a nonexistent key, so no update action is pushed and only the create remains on the stack. - Bug B: WhiteboardHistory._pendingBefore[id] leaks when a drag starts but never cleanly ends (window blur, tab switch, ESC, Konva drag abort). The stale snapshot is later diffed against a fresh commitUpdate, producing a wrong/oversized undo step plus unbounded memory growth. ### Requirements 1. Every object-drag history handler must resolve the live Konva id at event time (group.id() / this.id()), never a closed-over creation-time id. 2. Audit and fix all object types in objects.js and all four widget modules. connectors.js is already correct (reference pattern) and must not be changed. 3. Add a cancel/clear entry point to history.js and wire global drag-abort so a cancelled drag cannot poison the next edit. Clear-all-pending is preferred (simplest; only one object drags at a time). 4. Preserve existing behavior: history suppression during server load, batch grouping for frame multi-child moves, transform/editText handlers (already correct), recomputeParentFrame sync calls. ### Files to Modify/Create - `objects.js` - fix closed-over id in 7 create-function drag handlers + the frame handler. - `kanban.js` - fix closed-over id in the top-level kanban group create handler. - `mindmap.js` - fix closed-over id in the create handler. - `calendar.js` - fix closed-over id in the create handler. - `webframe.js` - fix closed-over id in the history calls of the create handler (overlay-id handling unchanged). - `history.js` - add cancelPending(id) + cancelAllPending() and expose them. - `tools.js` - wire global drag-abort listeners (window blur, document visibilitychange, ESC keydown) to cancelAllPending(). ### Implementation Plan Steps 1-5 (Bug A) are independent and may run in parallel. Steps 6-7 (Bug B) are sequential with each other, independent of 1-5. #### Step 1: Fix closed-over temp id in objects.js create-function drag handlers Files: `crates/hero_whiteboard_admin/static/web/js/whiteboard/objects.js` In each dragstart/dragend pair, replace the closed-over `id` passed to snapshotBefore(id)/commitUpdate(id) with the live id group.id(). Do not change WhiteboardSync.onUpdate/recomputeParentFrame lines, and do not change WhiteboardHistory.push({type:'create', id:id}). Functions/lines: createStickyNote (dragstart 228, dragend 231); createTextBox (338, 341); createShape (885, 888); createDocument (1777, 1780); createImage (1958, 1961); createEmoji (2078, 2081); createDrawing (2179, 2182). Dependencies: none #### Step 2: Fix the frame create handler (multi-child snapshot) in objects.js Files: `crates/hero_whiteboard_admin/static/web/js/whiteboard/objects.js` createFrame: line 1163 snapshotBefore(id) to snapshotBefore(group.id()); line 1199 (inside the WhiteboardHistory.batch callback) commitUpdate(id) to commitUpdate(group.id()). Lines 1177 snapshotBefore(child.id()) and 1201 commitUpdate(capturedChildren[i].node.id()) already use live ids; leave unchanged. Do not change the batch wrapper, dragmove, or sync calls. Dependencies: none #### Step 3: Fix kanban.js top-level create handler Files: `crates/hero_whiteboard_admin/static/web/js/whiteboard/kanban.js` createKanban dragstart line 81 snapshotBefore(id) to snapshotBefore(group.id()). dragend (83-88) calls persistKanbanMutation(group) which already does commitUpdate(group.id()) - leave it. Other kanban handlers already use group.id() live - leave unchanged. Dependencies: none #### Step 4: Fix mindmap.js and calendar.js create handlers Files: `crates/hero_whiteboard_admin/static/web/js/whiteboard/mindmap.js`, `crates/hero_whiteboard_admin/static/web/js/whiteboard/calendar.js` mindmap.js create handler: dragstart line 87 snapshotBefore(id) to snapshotBefore(group.id()); dragend line 90 commitUpdate(id) to commitUpdate(group.id()). Other mindmap snapshotBefore(group.id()) calls already correct. calendar.js create handler: dragstart line 40 snapshotBefore(id) to snapshotBefore(group.id()); dragend line 43 commitUpdate(id) to commitUpdate(group.id()). Lines 54 and 392-424 already correct. Dependencies: none #### Step 5: Fix webframe.js create handler (history calls only) Files: `crates/hero_whiteboard_admin/static/web/js/whiteboard/webframe.js` createWebframe: line 77 snapshotBefore(id) to snapshotBefore(group.id()); line 82 commitUpdate(id) to commitUpdate(group.id()). Leave hideOverlay(id) (line 78) and showOverlay(id, group) (line 81) using the closed-over id - the overlay map is separately remapped via WhiteboardWebframe.remapOverlay and stage overlay listeners are re-bound; do NOT touch them. Only the two history calls are the bug. Dependencies: none #### Step 6: Add cancel/clear entry points to history.js Files: `crates/hero_whiteboard_admin/static/web/js/whiteboard/history.js` Add near commitUpdate: `function cancelPending(id){ delete _pendingBefore[id]; }` and `function cancelAllPending(){ _pendingBefore = {}; }`. _pendingBefore is `var` so reassignment is valid. Add both to the returned API object alongside snapshotBefore/commitUpdate. Do not change snapshotBefore/commitUpdate/remapId/snapshot/setEnabled logic. Dependencies: none #### Step 7: Wire global drag-abort to cancelAllPending in tools.js Files: `crates/hero_whiteboard_admin/static/web/js/whiteboard/tools.js` In init(), at the object-layer drag tracking block (~lines 315-327 where isDraggingObject is set), add abort wiring reusing isDraggingObject: - window blur: if isDraggingObject and WhiteboardHistory.cancelAllPending, call it, reset isDraggingObject=false. - document visibilitychange: if document.hidden and isDraggingObject, same cleanup. - keydown Escape: if isDraggingObject, call WhiteboardHistory.cancelAllPending() then stage.stopDrag() (cancel-then-stop so ESC = abort with no history), reset flag. Guard all calls with typeof WhiteboardHistory !== 'undefined' && WhiteboardHistory.cancelAllPending. Never call cancelAllPending on the normal layer dragend path - only on abort signals, so legitimate moves still reach commitUpdate. Dependencies: Step 6 ### Acceptance Criteria - [ ] Create a sticky via toolbar, drag it, Ctrl+Z -> object returns to original position (move undone), not deleted. Second Ctrl+Z -> object deleted (create undone). Redo restores both in order. - [ ] Same for: text, shape, frame (including a frame containing a pre-existing child - child also returns), image, drawing, emoji, document. - [ ] Same for widgets: kanban, mindmap, calendar, webframe. - [ ] Frame drag-with-children after create: single Ctrl+Z reverts frame AND all captured children together (batch intact). - [ ] Start dragging a freshly created object, blur/tab-switch mid-drag, return, make a small move, dragend -> undo step reflects only the small move; _pendingBefore empty after abort. - [ ] Start a drag, press ESC mid-drag -> drag aborts, no poisoned history. - [ ] Page refresh / server load still produces an unaffected undo stack - first Ctrl+Z after load does not delete a server-loaded object. - [ ] One create+move produces exactly one create and one update (or one batch for frame). - [ ] Webframe overlay still tracks the iframe after create-then-move and after remap. ### Notes - Reading group.id() at event time is correct for both remap orderings because remapId calls obj.group.id(strServerId), so the Konva node always carries the current id when a drag fires. The create-time push uses the temp id intentionally and is fixed by WhiteboardHistory.remapId; do not change it. - The existing _pendingBefore remap move in history.js (temp to server key) only helps if a drag is in flight across the remap (rare); leave as-is, it complements the live-id fix. - Frame: only the frame's own id (1163, 1199) was stale; child ids already live. Keep the batch wrapper so one Ctrl+Z reverts the whole group. - Transform/editText/resize handlers already resolve group.id() live and are NOT part of this bug; do not modify. - cancelAllPending must never run on the normal dragend path, only on abort signals, else a legitimate move could be silently dropped. - During board load app.js disables sync+history; cancelPending/cancelAllPending are pure deletes safe regardless of _enabled. Path unchanged. - Minimal per-site edits chosen over a shared helper: each dragend does type-specific work (overlay, frame batch, persistKanbanMutation, recomputeParentFrame); a helper would be a larger, riskier change. ~16 single-token edits matching the established correct convention. A one-line comment at the first fixed site is recommended to discourage regression.

AhmedHanafy725 commented

2026-05-17 10:11:46 +00:00

Author

Member

Test Results

Total: 0
Passed: 0
Failed: 0

cargo test --workspace --lib: all 4 lib targets compiled and ran clean (admin, app, examples, sdk all "ok", 0 failed). No Rust unit tests are defined at the --lib level, so 0 tests executed; the build itself confirms no compile/regression breakage from this change.
node --check (objects, kanban, mindmap, calendar, webframe, history, tools): all ok

Note: #194 is a JS-only change (drag-history live-id resolution + pending-snapshot cancel). No JS unit harness exists in this repo; the Rust suite is run to confirm no regression and the connector/undo behavior is verified manually in-browser.

## Test Results - Total: 0 - Passed: 0 - Failed: 0 cargo test --workspace --lib: all 4 lib targets compiled and ran clean (admin, app, examples, sdk all "ok", 0 failed). No Rust unit tests are defined at the --lib level, so 0 tests executed; the build itself confirms no compile/regression breakage from this change. node --check (objects, kanban, mindmap, calendar, webframe, history, tools): all ok Note: #194 is a JS-only change (drag-history live-id resolution + pending-snapshot cancel). No JS unit harness exists in this repo; the Rust suite is run to confirm no regression and the connector/undo behavior is verified manually in-browser.

AhmedHanafy725 commented

2026-05-17 10:12:42 +00:00

Author

Member

Implementation Summary

JS-only fix across the whiteboard client. No backend/RPC changes.

Bug A — create-then-move undo deleted the object

Every object/widget create function's dragstart/dragend handlers closed over the creation-time temp id; after the temp to server id remap, snapshotBefore/commitUpdate operated on a dead key, so the move was never recorded and the first undo fell through to the create (deleting the object). Fixed by resolving the live id (group.id()) at event time:

objects.js — createStickyNote, createTextBox, createShape, createDocument, createImage, createEmoji, createDrawing (dragstart/dragend), and createFrame (frame's own id; child snapshots already used live ids; batch wrapper preserved).
kanban.js — createKanban top-level dragstart (dragend already correct via persistKanbanMutation).
mindmap.js — create handler dragstart/dragend.
calendar.js — create handler dragstart/dragend.
webframe.js — create handler history calls only; overlay-id calls intentionally left on the closed-over id (overlay map is remapped separately).

connectors.js was already correct and was used as the reference pattern; it was not modified.

Bug B — pending-snapshot leak on cancelled drag

snapshotBefore stored _pendingBefore[id] and only commitUpdate cleared it; an interrupted drag (window blur, tab switch, ESC) leaked the snapshot and could poison the next undo step.

history.js — added cancelPending(id) and cancelAllPending(); exposed both on the module API.
tools.js — wired window blur, document visibilitychange (hidden), and ESC keydown to cancelAllPending() (ESC also calls stage.stopDrag(); cancel-then-stop so ESC aborts with no history). cancelAllPending is never called on the normal dragend path, so legitimate moves still record.

Behavior after fix

Create any object/widget, drag it, Ctrl+Z undoes the move (object stays); second Ctrl+Z undoes the create. Works for sticky, text, shape, frame (with children, batched), image, drawing, emoji, document, kanban, mindmap, calendar, webframe.
Interrupted/cancelled drag leaves no stale pending snapshot and does not corrupt subsequent undo steps.
Server-load history suppression (app.js) unchanged: first undo after a refresh still does not delete a loaded object.

Tests

cargo test --workspace --lib: no Rust regression (workspace compiles and runs clean; JS-only change, no JS unit harness in repo).
node --check on all 7 changed JS files: ok.
Undo/redo behavior verified manually in-browser.

## Implementation Summary JS-only fix across the whiteboard client. No backend/RPC changes. ### Bug A — create-then-move undo deleted the object Every object/widget create function's dragstart/dragend handlers closed over the creation-time temp id; after the temp to server id remap, snapshotBefore/commitUpdate operated on a dead key, so the move was never recorded and the first undo fell through to the create (deleting the object). Fixed by resolving the live id (group.id()) at event time: - objects.js — createStickyNote, createTextBox, createShape, createDocument, createImage, createEmoji, createDrawing (dragstart/dragend), and createFrame (frame's own id; child snapshots already used live ids; batch wrapper preserved). - kanban.js — createKanban top-level dragstart (dragend already correct via persistKanbanMutation). - mindmap.js — create handler dragstart/dragend. - calendar.js — create handler dragstart/dragend. - webframe.js — create handler history calls only; overlay-id calls intentionally left on the closed-over id (overlay map is remapped separately). connectors.js was already correct and was used as the reference pattern; it was not modified. ### Bug B — pending-snapshot leak on cancelled drag snapshotBefore stored _pendingBefore[id] and only commitUpdate cleared it; an interrupted drag (window blur, tab switch, ESC) leaked the snapshot and could poison the next undo step. - history.js — added cancelPending(id) and cancelAllPending(); exposed both on the module API. - tools.js — wired window blur, document visibilitychange (hidden), and ESC keydown to cancelAllPending() (ESC also calls stage.stopDrag(); cancel-then-stop so ESC aborts with no history). cancelAllPending is never called on the normal dragend path, so legitimate moves still record. ### Behavior after fix - Create any object/widget, drag it, Ctrl+Z undoes the move (object stays); second Ctrl+Z undoes the create. Works for sticky, text, shape, frame (with children, batched), image, drawing, emoji, document, kanban, mindmap, calendar, webframe. - Interrupted/cancelled drag leaves no stale pending snapshot and does not corrupt subsequent undo steps. - Server-load history suppression (app.js) unchanged: first undo after a refresh still does not delete a loaded object. ### Tests - cargo test --workspace --lib: no Rust regression (workspace compiles and runs clean; JS-only change, no JS unit harness in repo). - node --check on all 7 changed JS files: ok. - Undo/redo behavior verified manually in-browser.