Whiteboard: ungroup is not undoable #186

New issue

Open

opened 2026-05-14 07:06:57 +00:00 by AhmedHanafy725 · 2 comments

AhmedHanafy725 commented

2026-05-14 07:06:57 +00:00

Member

Problem

WhiteboardGroups.ungroup (static/web/js/whiteboard/groups.js:167-178) destroys the group container and calls WhiteboardSync.onDelete(id) directly, with no WhiteboardHistory.push({ type: "delete", id, state }). After ungrouping a 30-object group there is no Ctrl+Z to bring the group back.

Approach

Mirror the pattern used by WhiteboardObjects.deleteObject (objects.js:1384): snapshot the group via WhiteboardSync.serializeForServer(groupNode) BEFORE destroying, then push { type: "delete", id, state } once the local destroy + objects-registry cleanup are done, and let WhiteboardSync.onDelete(id) persist the server-side delete as today.

Undo path is already wired: history.js:171 calls recreateObject(action.state) for type === "delete", which goes through WhiteboardApp.renderSingleObject(state) → createGroupFromData(obj, data) (app.js:530). The serialized payload already includes _groupState.name and _groupState.children, so the group reappears with the same children references and the same name.

Acceptance

Group a few objects, then Ungroup → Ctrl+Z restores the group with the same name and child references. Children stay in place.
Ctrl+Y after that re-ungroups.
No regression to group create or to children drag-with-group behaviour.
Multi-user broadcast (object.deleted / object.created) still fires for both ungroup and the undo re-create.

Out of scope

z-order chevrons (#14), comment marker rotate/scale (#25).

## Problem `WhiteboardGroups.ungroup` (`static/web/js/whiteboard/groups.js:167-178`) destroys the group container and calls `WhiteboardSync.onDelete(id)` directly, with no `WhiteboardHistory.push({ type: "delete", id, state })`. After ungrouping a 30-object group there is no Ctrl+Z to bring the group back. ## Approach Mirror the pattern used by `WhiteboardObjects.deleteObject` (`objects.js:1384`): snapshot the group via `WhiteboardSync.serializeForServer(groupNode)` BEFORE destroying, then push `{ type: "delete", id, state }` once the local destroy + objects-registry cleanup are done, and let `WhiteboardSync.onDelete(id)` persist the server-side delete as today. Undo path is already wired: `history.js:171` calls `recreateObject(action.state)` for `type === "delete"`, which goes through `WhiteboardApp.renderSingleObject(state)` → `createGroupFromData(obj, data)` (`app.js:530`). The serialized payload already includes `_groupState.name` and `_groupState.children`, so the group reappears with the same children references and the same name. ## Acceptance - [ ] Group a few objects, then Ungroup → Ctrl+Z restores the group with the same name and child references. Children stay in place. - [ ] Ctrl+Y after that re-ungroups. - [ ] No regression to group create or to children drag-with-group behaviour. - [ ] Multi-user broadcast (`object.deleted` / `object.created`) still fires for both ungroup and the undo re-create. ## Out of scope z-order chevrons (#14), comment marker rotate/scale (#25).

AhmedHanafy725 commented

2026-05-14 07:07:32 +00:00

Author

Member

Implementation Spec for Issue #186

Objective

Make ungroup undoable: Ctrl+Z restores the group container around the same children.

Approach

Mirror WhiteboardObjects.deleteObject (objects.js:1384): snapshot the group via WhiteboardSync.serializeForServer(groupNode) before destroying it, then push { type: 'delete', id, state }. Existing recreateObject path (history.js:119) → WhiteboardApp.renderSingleObject → createGroupFromData handles the undo recreate.

File to Modify

crates/hero_whiteboard_admin/static/web/js/whiteboard/groups.js

Implementation Plan

Step 1: Snapshot + history push in `ungroup`

Replace the function body (groups.js:167-178) with:

function ungroup(groupNode) {
    var id = groupNode.id();
    var state = groupNode._groupState;
    if (!state) return;

    // Snapshot before destroy so Ctrl+Z can recreate the group.
    var snapshot = (typeof WhiteboardSync !== 'undefined' && WhiteboardSync.serializeForServer)
        ? WhiteboardSync.serializeForServer(groupNode)
        : null;

    // Remove from objects
    groupNode.destroy();
    var objStore = WhiteboardObjects.getAllObjects();
    delete objStore[id];
    WhiteboardCanvas.getObjectLayer().batchDraw();

    if (snapshot) {
        WhiteboardHistory.push({ type: 'delete', id: id, state: snapshot });
    }
    WhiteboardSync.onDelete(id);
}

Acceptance Criteria

Group a few objects → Ungroup → Ctrl+Z restores the group; children stay in place.
Ctrl+Y re-ungroups.
No regression to group create / drag-with-children.
object.deleted / object.created broadcasts fire for the original ungroup and the undo recreate.

Notes

serializeForServer already captures _groupState.name + _groupState.children via the group type branch (sync.js:331-335).
createGroupFromData (app.js:530) handles the recreation: it rebuilds the visual chrome and rebinds the child ids list.
The snapshot guard (if (snapshot)) is defensive — serializeForServer should always return a payload for an existing group, but fail-soft on missing dependency.

## Implementation Spec for Issue #186 ### Objective Make `ungroup` undoable: Ctrl+Z restores the group container around the same children. ### Approach Mirror `WhiteboardObjects.deleteObject` (`objects.js:1384`): snapshot the group via `WhiteboardSync.serializeForServer(groupNode)` before destroying it, then push `{ type: 'delete', id, state }`. Existing `recreateObject` path (`history.js:119`) → `WhiteboardApp.renderSingleObject` → `createGroupFromData` handles the undo recreate. ### File to Modify - `crates/hero_whiteboard_admin/static/web/js/whiteboard/groups.js` ### Implementation Plan #### Step 1: Snapshot + history push in `ungroup` Replace the function body (`groups.js:167-178`) with: ```js function ungroup(groupNode) { var id = groupNode.id(); var state = groupNode._groupState; if (!state) return; // Snapshot before destroy so Ctrl+Z can recreate the group. var snapshot = (typeof WhiteboardSync !== 'undefined' && WhiteboardSync.serializeForServer) ? WhiteboardSync.serializeForServer(groupNode) : null; // Remove from objects groupNode.destroy(); var objStore = WhiteboardObjects.getAllObjects(); delete objStore[id]; WhiteboardCanvas.getObjectLayer().batchDraw(); if (snapshot) { WhiteboardHistory.push({ type: 'delete', id: id, state: snapshot }); } WhiteboardSync.onDelete(id); } ``` ### Acceptance Criteria - [ ] Group a few objects → Ungroup → Ctrl+Z restores the group; children stay in place. - [ ] Ctrl+Y re-ungroups. - [ ] No regression to group create / drag-with-children. - [ ] `object.deleted` / `object.created` broadcasts fire for the original ungroup and the undo recreate. ### Notes - `serializeForServer` already captures `_groupState.name` + `_groupState.children` via the `group` type branch (`sync.js:331-335`). - `createGroupFromData` (`app.js:530`) handles the recreation: it rebuilds the visual chrome and rebinds the child ids list. - The `snapshot` guard (`if (snapshot)`) is defensive — `serializeForServer` should always return a payload for an existing group, but fail-soft on missing dependency.

AhmedHanafy725 commented

2026-05-14 07:17:32 +00:00

Author

Member

Test Results + Final Summary

Changes

static/web/js/whiteboard/groups.js — ungroup now snapshots the group via WhiteboardSync.serializeForServer(groupNode) before destroy and pushes { type: 'delete', id, state: snapshot } on the history stack. WhiteboardSync.onDelete(id) continues to fire after, unchanged.

Behaviour after fix

Group + Ungroup + Ctrl+Z → group container reappears around the same children. Multi-user clients see one object.deleted (the ungroup) and one object.created (the undo).
Ctrl+Y re-ungroups.

Gates

node -c groups.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. Group a few objects (Ctrl+G), Ungroup via the selection-toolbar or shortcut, press Ctrl+Z — verify the group container is restored with its name and child references.

## Test Results + Final Summary ### Changes `static/web/js/whiteboard/groups.js` — `ungroup` now snapshots the group via `WhiteboardSync.serializeForServer(groupNode)` before destroy and pushes `{ type: 'delete', id, state: snapshot }` on the history stack. `WhiteboardSync.onDelete(id)` continues to fire after, unchanged. ### Behaviour after fix - Group + Ungroup + Ctrl+Z → group container reappears around the same children. Multi-user clients see one `object.deleted` (the ungroup) and one `object.created` (the undo). - Ctrl+Y re-ungroups. ### Gates - `node -c groups.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. Group a few objects (Ctrl+G), Ungroup via the selection-toolbar or shortcut, press Ctrl+Z — verify the group container is restored with its name and child references.