lhumina_code/hero_whiteboard

Fork 0

Whiteboard: z-order operations and frame slide reorder are not undoable #188

New issue

Open

opened 2026-05-14 11:23:06 +00:00 by AhmedHanafy725 · 2 comments

AhmedHanafy725 commented

2026-05-14 11:23:06 +00:00

Member

Problem

Every z-index reorder fires WhiteboardSync.onUpdate(...) to persist but never brackets the change with WhiteboardHistory.snapshotBefore + commitUpdate, so Ctrl+Z does nothing for any of these:

Bring to front / Send to back / Move forward / Move backward on the selected node(s) — static/web/js/whiteboard/tools.js:944-977 (toolbar / shortcut path) and static/web/js/whiteboard/contextmenu.js:197-217 (right-click menu).
Frame slide reorder — static/web/js/whiteboard/frames.js:66-80 (_swapOrder). The selection toolbars chevrons and the right-click context menu both call this; the user can move a slide up or down in the presentation deck and have no way to undo it.

Approach

Same pattern as the other history-coverage fixes:

Snapshot each affected node before the mutation.
Apply the z-index / frameOrder change.
Commit each snapshot after. For multi-node z-order ops (the toolbar/transformer path can have multiple nodes selected), wrap the commit loop in WhiteboardHistory.batch(...) so Ctrl+Z reverses the whole group in one step.
Keep the existing WhiteboardSync.onUpdate(node) broadcast.

Acceptance

Select a node → Bring to front → Ctrl+Z restores the previous z-index. Ctrl+Y reapplies.
Same for Send to back / Move forward / Move backward.
Multi-node selection + Bring to front → Ctrl+Z reverses all of them in a single step.
Right-click → Bring to front / Send to back on a single node → Ctrl+Z reverses.
Frame slide reorder (selection toolbar chevrons OR _swapOrder from any caller) → Ctrl+Z swaps the two frames frameOrder back. Slide badges update.
onUpdate still fires exactly once per affected node per op. No double-emit.
No regression to normal z-order behaviour.

## Problem Every z-index reorder fires `WhiteboardSync.onUpdate(...)` to persist but never brackets the change with `WhiteboardHistory.snapshotBefore` + `commitUpdate`, so Ctrl+Z does nothing for any of these: - **Bring to front / Send to back / Move forward / Move backward** on the selected node(s) — `static/web/js/whiteboard/tools.js:944-977` (toolbar / shortcut path) and `static/web/js/whiteboard/contextmenu.js:197-217` (right-click menu). - **Frame slide reorder** — `static/web/js/whiteboard/frames.js:66-80` (`_swapOrder`). The selection toolbars chevrons and the right-click context menu both call this; the user can move a slide up or down in the presentation deck and have no way to undo it. ## Approach Same pattern as the other history-coverage fixes: - Snapshot each affected node before the mutation. - Apply the z-index / `frameOrder` change. - Commit each snapshot after. For multi-node z-order ops (the toolbar/transformer path can have multiple nodes selected), wrap the commit loop in `WhiteboardHistory.batch(...)` so Ctrl+Z reverses the whole group in one step. - Keep the existing `WhiteboardSync.onUpdate(node)` broadcast. ## Acceptance - [ ] Select a node → Bring to front → Ctrl+Z restores the previous z-index. Ctrl+Y reapplies. - [ ] Same for Send to back / Move forward / Move backward. - [ ] Multi-node selection + Bring to front → Ctrl+Z reverses all of them in a single step. - [ ] Right-click → Bring to front / Send to back on a single node → Ctrl+Z reverses. - [ ] Frame slide reorder (selection toolbar chevrons OR `_swapOrder` from any caller) → Ctrl+Z swaps the two frames `frameOrder` back. Slide badges update. - [ ] `onUpdate` still fires exactly once per affected node per op. No double-emit. - [ ] No regression to normal z-order behaviour.

AhmedHanafy725 self-assigned this

2026-05-14 11:23:24 +00:00

AhmedHanafy725 commented

2026-05-14 11:23:40 +00:00

Author

Member

Implementation Spec for Issue #188

Files to Modify

static/web/js/whiteboard/tools.js
static/web/js/whiteboard/contextmenu.js
static/web/js/whiteboard/frames.js

Pattern

For each z-order op (single or multi-node):

function op() {
    var nodes = … // 1 or more
    if (nodes.length === 0) return;
    nodes.forEach(function (n) { WhiteboardHistory.snapshotBefore(n.id()); });
    nodes.forEach(applyOp);          // moveToTop / moveToBottom / etc.
    // existing post-mutation work (transformer.moveToTop, batchDraw, ...)
    WhiteboardHistory.batch(function () {
        nodes.forEach(function (n) { WhiteboardHistory.commitUpdate(n.id()); });
    });
    nodes.forEach(function (n) { WhiteboardSync.onUpdate(n); });
}

WhiteboardHistory.batch (history.js:43) folds the per-node commits into a single undo step. For single-node ops the batch is harmless (one push inside).

Implementation Plan

Step 1: `tools.js:944-977` — `bringToFront / sendToBack / moveForward / moveBackward`

Each function gets snapshotBefore over transformer.nodes() at the top and a batched commitUpdate loop before the existing onUpdate broadcast.

Step 2: `contextmenu.js:197-217` — right-click `bringToFront / sendToBack`

Single-node path. Add snapshotBefore(targetNode.id()) at the top after the early-exit, and commitUpdate(targetNode.id()) after batchDraw() but before WhiteboardSync.onUpdate(targetNode). No batch wrapper needed (single node).

Step 3: `frames.js:_swapOrder` (lines 66-80) — slide reorder

Snapshot both a and b before the setAttr calls; commit both after the redrawAllFrameBadges, wrapped in a single batch so one Ctrl+Z reverses the swap.

function _swapOrder(a, b) {
    var ka = _orderKey(a);
    var kb = _orderKey(b);
    WhiteboardHistory.snapshotBefore(a.id());
    WhiteboardHistory.snapshotBefore(b.id());
    a.setAttr('frameOrder', kb);
    b.setAttr('frameOrder', ka);
    if (typeof WhiteboardObjects !== 'undefined' && WhiteboardObjects.redrawAllFrameBadges) {
        WhiteboardObjects.redrawAllFrameBadges();
    }
    WhiteboardHistory.batch(function () {
        WhiteboardHistory.commitUpdate(a.id());
        WhiteboardHistory.commitUpdate(b.id());
    });
    if (typeof WhiteboardSync !== 'undefined' && WhiteboardSync.onUpdate) {
        WhiteboardSync.onUpdate(a);
        WhiteboardSync.onUpdate(b);
    }
}

Acceptance Criteria

Bring to front / Send to back / Move forward / Move backward → Ctrl+Z reverses.
Multi-node z-order → single Ctrl+Z reverses the whole group.
Right-click → Bring to front / Send to back → Ctrl+Z reverses.
Frame slide reorder → Ctrl+Z swaps both back (slide badges update).
onUpdate fires once per affected node.
No regression to existing behaviour.

Notes

snapshot reads z_index via serializeForServer (sync.js:382: z_index: node.zIndex() || 0). For frames it also captures frameOrder (sync.js:314-318). Both are restored by applySyncUpdate (sync.js:580-582 for z_index; the frame branch at sync.js:670-676 restores frameOrder). So undo restores the visual ordering correctly.
WhiteboardHistory.batch is no-op for single-node use cases — one push goes into the batch, applyUndo just unwraps it.

## Implementation Spec for Issue #188 ### Files to Modify - `static/web/js/whiteboard/tools.js` - `static/web/js/whiteboard/contextmenu.js` - `static/web/js/whiteboard/frames.js` ### Pattern For each z-order op (single or multi-node): ```js function op() { var nodes = … // 1 or more if (nodes.length === 0) return; nodes.forEach(function (n) { WhiteboardHistory.snapshotBefore(n.id()); }); nodes.forEach(applyOp); // moveToTop / moveToBottom / etc. // existing post-mutation work (transformer.moveToTop, batchDraw, ...) WhiteboardHistory.batch(function () { nodes.forEach(function (n) { WhiteboardHistory.commitUpdate(n.id()); }); }); nodes.forEach(function (n) { WhiteboardSync.onUpdate(n); }); } ``` `WhiteboardHistory.batch` (`history.js:43`) folds the per-node commits into a single undo step. For single-node ops the batch is harmless (one push inside). ### Implementation Plan #### Step 1: `tools.js:944-977` — `bringToFront / sendToBack / moveForward / moveBackward` Each function gets `snapshotBefore` over `transformer.nodes()` at the top and a batched `commitUpdate` loop before the existing `onUpdate` broadcast. #### Step 2: `contextmenu.js:197-217` — right-click `bringToFront / sendToBack` Single-node path. Add `snapshotBefore(targetNode.id())` at the top after the early-exit, and `commitUpdate(targetNode.id())` after `batchDraw()` but before `WhiteboardSync.onUpdate(targetNode)`. No batch wrapper needed (single node). #### Step 3: `frames.js:_swapOrder` (lines 66-80) — slide reorder Snapshot both `a` and `b` before the `setAttr` calls; commit both after the `redrawAllFrameBadges`, wrapped in a single `batch` so one Ctrl+Z reverses the swap. ```js function _swapOrder(a, b) { var ka = _orderKey(a); var kb = _orderKey(b); WhiteboardHistory.snapshotBefore(a.id()); WhiteboardHistory.snapshotBefore(b.id()); a.setAttr('frameOrder', kb); b.setAttr('frameOrder', ka); if (typeof WhiteboardObjects !== 'undefined' && WhiteboardObjects.redrawAllFrameBadges) { WhiteboardObjects.redrawAllFrameBadges(); } WhiteboardHistory.batch(function () { WhiteboardHistory.commitUpdate(a.id()); WhiteboardHistory.commitUpdate(b.id()); }); if (typeof WhiteboardSync !== 'undefined' && WhiteboardSync.onUpdate) { WhiteboardSync.onUpdate(a); WhiteboardSync.onUpdate(b); } } ``` ### Acceptance Criteria - [ ] Bring to front / Send to back / Move forward / Move backward → Ctrl+Z reverses. - [ ] Multi-node z-order → single Ctrl+Z reverses the whole group. - [ ] Right-click → Bring to front / Send to back → Ctrl+Z reverses. - [ ] Frame slide reorder → Ctrl+Z swaps both back (slide badges update). - [ ] `onUpdate` fires once per affected node. - [ ] No regression to existing behaviour. ### Notes - `snapshot` reads `z_index` via `serializeForServer` (`sync.js:382`: `z_index: node.zIndex() || 0`). For frames it also captures `frameOrder` (`sync.js:314-318`). Both are restored by `applySyncUpdate` (`sync.js:580-582` for `z_index`; the frame branch at `sync.js:670-676` restores `frameOrder`). So undo restores the visual ordering correctly. - `WhiteboardHistory.batch` is no-op for single-node use cases — one push goes into the batch, applyUndo just unwraps it.

AhmedHanafy725 commented

2026-05-14 11:25:39 +00:00

Author

Member

Test Results + Final Summary

Changes

static/web/js/whiteboard/tools.js: added _applyZOrder / _commitZOrder helpers; wrapped bringToFront, sendToBack, moveForward, moveBackward. Multi-node selection commits are batched.
static/web/js/whiteboard/contextmenu.js: single-node bringToFront / sendToBack now snapshot before + commit after.
static/web/js/whiteboard/frames.js: _swapOrder snapshots both a and b before the frameOrder swap and commits both in a batch.

Behaviour after fix

Bring to front / Send to back / Move forward / Move backward → Ctrl+Z reverses each affected node. Multi-node selection rolls back in a single step (batch).
Right-click → Bring to front / Send to back → Ctrl+Z reverses.
Frame slide reorder (selection toolbar chevrons or any other caller of _swapOrder) → Ctrl+Z swaps both back, slide badges update.
onUpdate fires once per affected node, no double-emit.

Gates

node -c tools.js / contextmenu.js / frames.js — JS syntax OK
cargo fmt --check — pass
cargo clippy --workspace --all-targets -- -D warnings — pass

Manual verification still required

Rebuild + restart hero_whiteboard_admin, hard-reload. Walk through each op (single + multi-node) and confirm Ctrl+Z / Ctrl+Y behave correctly. Verify frame slide reorder via the selection toolbar chevrons in particular.

## Test Results + Final Summary ### Changes - `static/web/js/whiteboard/tools.js`: added `_applyZOrder` / `_commitZOrder` helpers; wrapped `bringToFront`, `sendToBack`, `moveForward`, `moveBackward`. Multi-node selection commits are batched. - `static/web/js/whiteboard/contextmenu.js`: single-node `bringToFront` / `sendToBack` now snapshot before + commit after. - `static/web/js/whiteboard/frames.js`: `_swapOrder` snapshots both `a` and `b` before the `frameOrder` swap and commits both in a batch. ### Behaviour after fix - Bring to front / Send to back / Move forward / Move backward → Ctrl+Z reverses each affected node. Multi-node selection rolls back in a single step (batch). - Right-click → Bring to front / Send to back → Ctrl+Z reverses. - Frame slide reorder (selection toolbar chevrons or any other caller of `_swapOrder`) → Ctrl+Z swaps both back, slide badges update. - `onUpdate` fires once per affected node, no double-emit. ### Gates - `node -c tools.js / contextmenu.js / frames.js` — JS syntax OK - `cargo fmt --check` — pass - `cargo clippy --workspace --all-targets -- -D warnings` — pass ### Manual verification still required Rebuild + restart `hero_whiteboard_admin`, hard-reload. Walk through each op (single + multi-node) and confirm Ctrl+Z / Ctrl+Y behave correctly. Verify frame slide reorder via the selection toolbar chevrons in particular.

AhmedHanafy725 referenced this issue from a commit

2026-05-14 11:33:02 +00:00

feat(whiteboard): undo/redo support for z-order ops and frame slide reorder