Whiteboard: undo/redo does not track comment actions #179
Labels
No labels
prio_critical
prio_low
type_bug
type_contact
type_issue
type_lead
type_question
type_story
type_task
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
lhumina_code/hero_whiteboard#179
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
Undo (Ctrl+Z) and redo (Ctrl+Y / Ctrl+Shift+Z) do not affect comments. The undo stack only sees object-layer actions because comments.js never calls
WhiteboardHistory.push.Affected operations on the whiteboard:
comment.updatex/y)comment.resolve)Expected
Each of those operations becomes a single undo step:
createcallscomment.delete; redo recreates with the same payload.deleterecreates from a snapshot taken before deletion; redo deletes.movecallscomment.updatewith the originalx/y; redo restores the newx/y.resolvetoggles the resolved flag back; redo re-applies it.Multi-user broadcasts continue to fire naturally because the inverse operations go through the same
rpcCall+broadcastCommentpath.Acceptance
Out of scope
comment.createwithparent_id).Implementation Spec for Issue #179
Objective
Make undo/redo cover the four user-facing comment operations (create, delete, drag-move, resolve/unresolve) on the whiteboard.
Approach
Extend
history.jswith acustomaction type. The action carries its ownundo()andredo()callbacks.applyUndo/applyRedoinvoke them. This keeps the existing object-layer machinery untouched and givescomments.js(and any future non-Konva feature) a clean integration point.In
comments.js, after every successful comment-mutating RPC, push acustomaction that captures the inverse. To handle the id changing on re-create, each action stores its working state in a closure: when redo re-creates a comment, it patches the closure'sidso the next undo targets the new id. The action functions themselves are RPC-driven, so multi-user broadcasts continue to fire as normal.No history pushes for replies (
parent_id != null) — they don't have markers and are out of scope per the issue.Files to Modify
crates/hero_whiteboard_admin/static/web/js/whiteboard/history.jscrates/hero_whiteboard_admin/static/web/js/whiteboard/comments.jsImplementation Plan
Step 1: Add
customaction type to history.jsIn
applyUndo(around the existingif/else ifchain), add:In
applyRedo:Custom actions are skipped by
fixStack(it only rewrites object ids).Dependencies: none.
Step 2: Helper RPC wrappers in comments.js
Add private helpers near the top of the module that issue RPCs without pushing to history (so undo/redo doesn't recurse):
Dependencies: none.
Step 3: Push history for
createIn
createComment(currently callsrpcCall('comment.create', params)at line 60), after the success branch:Dependencies: Step 1 + 2.
Step 4: Push history for
deleteIn
deleteComment(line 127), take a snapshot of the entry's data BEFORE issuing the RPC, then push after success:Dependencies: Step 1 + 2.
Step 5: Push history for
move(dragend)Capture the position at
dragstartand push after thecomment.updatesucceeds:Dependencies: Step 1 + 2.
Step 6: Push history for
resolve/unresolveIn
setResolved(id, resolved)(line 100), after the existing success branch:Dependencies: Step 1 + 2.
Acceptance Criteria
parent_id != null) does not push history (out of scope).Notes
state.idso undo finds the latest server id after a redo creates a new comment.fixStack(object-id rewriter) ignorescustomactions because their bodies aren't{id, state}shaped.Test Results + Final Summary
Changes
crates/hero_whiteboard_admin/static/web/js/whiteboard/history.js— added acustomaction type to bothapplyUndoandapplyRedo. Custom actions carry caller-suppliedundo()/redo()callbacks; everything else (fixStack, button state, batch wrapping) is untouched.crates/hero_whiteboard_admin/static/web/js/whiteboard/comments.js:_createCommentFromData,_deleteCommentById,_updateCommentXY,_setResolvedByIdthat update local state + firebroadcastCommentbut never push history.createCommentpushes a custom action; the action's closure tracks the latest server id so subsequent undo/redo target the right comment.deleteCommentsnapshots the comment before issuing the RPC, then pushes a custom action whose undo recreates from the snapshot (and restores the resolved flag if applicable).setResolvedpushes a toggle action.dragstartcoords and, ondragendsuccess, pushes a coord-update action.Gates
node -c comments.js/node -c history.js— JS syntax OKcargo fmt --check— passcargo clippy --workspace --all-targets -- -D warnings— passcargo test --workspace --lib— 0 tests, 0 failuresAcceptance Criteria
parent_id != null) untouched (no marker, no history push).Manual verification still required
Rebuild + restart
hero_whiteboard_admin, hard-reload, open a board and walk through:Out of scope (per the issue)