Whiteboard: comment-reply / drag-transform edge cases #190

New issue

Open

opened 2026-05-14 12:15:34 +00:00 by AhmedHanafy725 · 2 comments

AhmedHanafy725 commented 2026-05-14 12:15:34 +00:00

Member

Copy link

Three related bugs in static/web/js/whiteboard/comments.js

A. Reply edge cases

• deleteComment (line 203) calls hidePopover() unconditionally on success. When the user clicks the small delete link next to a reply inside the parents popover, the parents popover gets closed even though the parent itself wasnt deleted. Expected: refresh the popover so the reply row disappears, keep the parent popover open.
• The delete-undo snapshot at line 215-221 only carries board_id, user_id, text, x, y. It drops parent_id, rotation, scale, and resolved (the resolved flag is partially handled in a second RPC). So undoing the deletion of a reply silently re-creates it as a NEW top-level comment with its own marker on the canvas.

B. Drag history omits rotation/scale (audit #25)

• The drag-end push at line 338-342 stores only oldX/oldY in the snapshot, so Ctrl+Z restores x/y but not rotation/scale.
• onTransformEnd (line 652) bakes rotation/scale into the entry + fires comment.update but pushes NOTHING onto the history stack. Transform-only changes are entirely outside undo.

C. Reply coords are stored from parent at create time then never resynced (audit #29)

• replyToComment (line 142) inherits the parents x/y at create time (line 152-155). These values are stored on the server and returned by comment.list, but the reply has no marker (addCommentMarker at line 243 skips replies). When the parent later moves, the reply rows hold stale parent coords forever.
• The server schema (openrpc.json comment.create) marks x/y as optional, so we can simply omit them for replies.

Approach

1. Reply delete UX: in deleteComment, if the deleted comment had a parent_id AND the popover is currently showing that parent, call showCommentPopover(parent_id) to refresh instead of hidePopover(). Only top-level deletions hide the popover (the parent is now gone — nothing to show).
2. Snapshot completeness: extend the delete-undo snapshot to include parent_id, rotation, scale. The recreate path (_createCommentFromData) already supports passing these via the params object; comment.create schema accepts them as optional. Same for the resolved flag (already partially handled).
3. Reply create cleanup: in replyToComment, drop the params.x = parent.data.x lines. Dont send x/y for replies. The reply is never rendered as a marker so the values are unused, and omitting them keeps the server data sane when the parent moves.
4. Transform history: introduce a new internal _updateCommentTransform(id, x, y, rotation, scale) helper. Make the drag-end snapshot capture rotation/scale alongside x/y; push an undo that calls the transform helper with the old quad and a redo with the new quad. Make onTransformEnd do the same: capture before-state at transformstart, push history at transformend.

Acceptance

• Click delete next to a reply in the parents popover → reply disappears from the popover, popover stays open. Ctrl+Z restores the reply as a reply (not a top-level), and the popover re-shows it.
• Click the bottom Delete button on the parent itself → popover closes, parent + replies removed. Ctrl+Z restores the parent as top-level with all its replies… (or at least the parent itself; replies are separate undo steps that can be re-undone individually — document this).
• Drag a marker → Ctrl+Z restores previous x/y. Rotate / scale via transformer → Ctrl+Z restores previous rotation/scale (and x/y if they also changed).
• Reply create no longer sends x/y. After moving the parent, the parent and all replies remain consistent.
• No regression to top-level comment create / delete / drag / resolve undo (#179).
• Multi-user broadcasts continue to fire once per mutation.

## Three related bugs in `static/web/js/whiteboard/comments.js` ### A. Reply edge cases - `deleteComment` (line 203) calls `hidePopover()` unconditionally on success. When the user clicks the small `delete` link next to a reply inside the parents popover, the parents popover gets closed even though the parent itself wasnt deleted. Expected: refresh the popover so the reply row disappears, keep the parent popover open. - The delete-undo snapshot at line 215-221 only carries `board_id`, `user_id`, `text`, `x`, `y`. It drops `parent_id`, `rotation`, `scale`, and `resolved` (the resolved flag is partially handled in a second RPC). So undoing the deletion of a reply silently re-creates it as a NEW top-level comment with its own marker on the canvas. ### B. Drag history omits rotation/scale (audit #25) - The drag-end push at line 338-342 stores only `oldX/oldY` in the snapshot, so Ctrl+Z restores x/y but not rotation/scale. - `onTransformEnd` (line 652) bakes rotation/scale into the entry + fires `comment.update` but pushes NOTHING onto the history stack. Transform-only changes are entirely outside undo. ### C. Reply coords are stored from parent at create time then never resynced (audit #29) - `replyToComment` (line 142) inherits the parents x/y at create time (line 152-155). These values are stored on the server and returned by `comment.list`, but the reply has no marker (`addCommentMarker` at line 243 skips replies). When the parent later moves, the reply rows hold stale parent coords forever. - The server schema (`openrpc.json` `comment.create`) marks x/y as optional, so we can simply omit them for replies. ## Approach 1. **Reply delete UX**: in `deleteComment`, if the deleted comment had a `parent_id` AND the popover is currently showing that parent, call `showCommentPopover(parent_id)` to refresh instead of `hidePopover()`. Only top-level deletions hide the popover (the parent is now gone — nothing to show). 2. **Snapshot completeness**: extend the delete-undo snapshot to include `parent_id`, `rotation`, `scale`. The recreate path (`_createCommentFromData`) already supports passing these via the params object; `comment.create` schema accepts them as optional. Same for the resolved flag (already partially handled). 3. **Reply create cleanup**: in `replyToComment`, drop the `params.x = parent.data.x` lines. Dont send x/y for replies. The reply is never rendered as a marker so the values are unused, and omitting them keeps the server data sane when the parent moves. 4. **Transform history**: introduce a new internal `_updateCommentTransform(id, x, y, rotation, scale)` helper. Make the drag-end snapshot capture rotation/scale alongside x/y; push an undo that calls the transform helper with the old quad and a redo with the new quad. Make `onTransformEnd` do the same: capture before-state at transformstart, push history at transformend. ## Acceptance - [ ] Click `delete` next to a reply in the parents popover → reply disappears from the popover, popover stays open. Ctrl+Z restores the reply as a reply (not a top-level), and the popover re-shows it. - [ ] Click the bottom Delete button on the parent itself → popover closes, parent + replies removed. Ctrl+Z restores the parent as top-level with all its replies… (or at least the parent itself; replies are separate undo steps that can be re-undone individually — document this). - [ ] Drag a marker → Ctrl+Z restores previous x/y. Rotate / scale via transformer → Ctrl+Z restores previous rotation/scale (and x/y if they also changed). - [ ] Reply create no longer sends x/y. After moving the parent, the parent and all replies remain consistent. - [ ] No regression to top-level comment create / delete / drag / resolve undo (#179). - [ ] Multi-user broadcasts continue to fire once per mutation.

AhmedHanafy725 commented 2026-05-14 12:16:31 +00:00

Author

Member

Copy link

Implementation Spec for Issue #190

File to Modify

• static/web/js/whiteboard/comments.js

Plan

Step 1: Add _updateCommentTransform helper

Insert next to the existing _updateCommentXY (~line 74). Accepts the full transform quad and updates both server + local state + marker:

function _updateCommentTransform(id, x, y, rotation, scale) {
    var payload = { id: id, x: x, y: y, rotation: rotation, scale: scale };
    return rpcCall('comment.update', payload).then(function () {
        var entry = _comments[id];
        if (entry) {
            entry.data.x = x;
            entry.data.y = y;
            entry.data.rotation = rotation;
            entry.data.scale = scale;
            if (entry.marker) {
                entry.marker.x(x);
                entry.marker.y(y);
                entry.marker.rotation(rotation);
                entry.marker.scaleX(scale);
                entry.marker.scaleY(scale);
                WhiteboardCanvas.getObjectLayer().batchDraw();
            }
        }
        broadcastComment('comment.updated', payload);
    });
}

Step 2: Reply create — omit x/y

In replyToComment (line 142-167), DROP the block:

if (parent && parent.data.x != null) {
    params.x = parent.data.x;
    params.y = parent.data.y;
}

comment.create accepts x/y as optional. Replies have no marker, so the values are never used; omitting them prevents the server from holding stale coordinates after the parent moves.

Step 3: Reply delete UX + complete snapshot

Rewrite deleteComment (line 203-235):

function deleteComment(id) {
    var entry = _comments[id];
    var snapshot = entry ? Object.assign({}, entry.data) : null;
    var parentId = snapshot && snapshot.parent_id ? snapshot.parent_id : null;
    rpcCall('comment.delete', { id: id }).then(function() {
        removeCommentMarker(id);
        broadcastComment('comment.deleted', { id: id });

        // If we deleted a REPLY whose parent's popover is currently open,
        // refresh the popover so the reply row disappears but the parent
        // popover stays open. Otherwise close the popover (the comment we
        // just deleted was the top-level one being shown).
        if (parentId && _popoverEl && _popoverEl._commentId === parentId) {
            showCommentPopover(parentId);
        } else {
            hidePopover();
        }

        if (snapshot) {
            var state = { id: id };
            WhiteboardHistory.push({
                type: 'custom',
                undo: function () {
                    var payload = {
                        board_id: snapshot.board_id,
                        user_id: snapshot.user_id,
                        text: snapshot.text,
                    };
                    // Preserve reply-ness: pass parent_id when the deleted
                    // comment was a reply. For top-level, include x/y/
                    // rotation/scale so the marker reappears in place.
                    if (snapshot.parent_id != null) {
                        payload.parent_id = snapshot.parent_id;
                    } else {
                        if (snapshot.x != null) payload.x = snapshot.x;
                        if (snapshot.y != null) payload.y = snapshot.y;
                        if (snapshot.rotation != null) payload.rotation = snapshot.rotation;
                        if (snapshot.scale != null) payload.scale = snapshot.scale;
                    }
                    return _createCommentFromData(payload).then(function (re) {
                        state.id = re.id;
                        if (snapshot.resolved) {
                            return _setResolvedById(re.id, true);
                        }
                    });
                },
                redo: function () { return _deleteCommentById(state.id); },
            });
        }
    }).catch(function(e) {
        WhiteboardApp.showToast('Failed to delete: ' + e.message, true);
    });
}

_createCommentFromData already calls addCommentMarker, which skips marker creation for replies (no visual change for the popover-only path), so undo restoring a reply is correct: the popover refresh on the next render will show it again if the parent is open.

Step 4: Drag-end → store rotation/scale too

Replace the dragstart / dragend block (line 320-347):

var dragStart = null;
group.on('dragstart', function() {
    dragStart = {
        x: Math.round(group.x()),
        y: Math.round(group.y()),
        rotation: group.rotation(),
        scale: group.scaleX(),
    };
});
group.on('dragend', function() {
    var before = dragStart;
    dragStart = null;
    var newX = Math.round(group.x());
    var newY = Math.round(group.y());
    var newRot = group.rotation();
    var newScale = group.scaleX();
    var entry = _comments[id];
    if (entry) {
        entry.data.x = newX;
        entry.data.y = newY;
    }
    rpcCall('comment.update', { id: id, x: newX, y: newY }).then(function() {
        broadcastComment('comment.updated', { id: id, x: newX, y: newY });
        if (before && (before.x !== newX || before.y !== newY)) {
            WhiteboardHistory.push({
                type: 'custom',
                undo: function () { return _updateCommentTransform(id, before.x, before.y, before.rotation, before.scale); },
                redo: function () { return _updateCommentTransform(id, newX, newY, newRot, newScale); },
            });
        }
    }).catch(function(e) {
        console.log('[comments] Failed to update position:', e.message);
    });
});

Step 5: Transform-end pushes history

Replace onTransformEnd (line 652-686). Capture before-state in transformstart (the transformer fires this on the selected node), bake the new transform in transformend, then push history. Since onTransformEnd is called from tools.js on the transformer's transformend, add a paired onTransformStart that captures the before state per-comment-id.

Actually simpler: bake state on the group via _transformBefore and read it in onTransformEnd:

function onTransformStart(group) {
    if (!group) return;
    var idStr = group.id() || '';
    if (idStr.indexOf('comment-') !== 0) return;
    group._transformBefore = {
        x: Math.round(group.x()),
        y: Math.round(group.y()),
        rotation: group.rotation(),
        scale: group.scaleX(),
    };
}

function onTransformEnd(group) {
    if (!group) return;
    var idStr = group.id() || '';
    if (idStr.indexOf('comment-') !== 0) return;
    var id = parseInt(idStr.slice('comment-'.length), 10);
    if (!id || !_comments[id]) return;

    var sx = group.scaleX();
    var sy = group.scaleY();
    var uniform = Math.max(0.2, Math.min(5, (sx + sy) / 2));
    group.scaleX(uniform);
    group.scaleY(uniform);

    var newX = Math.round(group.x());
    var newY = Math.round(group.y());
    var newRot = group.rotation();
    var before = group._transformBefore;
    group._transformBefore = null;

    var entry = _comments[id];
    entry.data.x = newX;
    entry.data.y = newY;
    entry.data.rotation = newRot;
    entry.data.scale = uniform;

    rpcCall('comment.update', { id: id, x: newX, y: newY, rotation: newRot, scale: uniform }).catch(function(e) {
        console.log('[comments] Failed to update transform:', e.message);
    });
    broadcastComment('comment.updated', { id: id, x: newX, y: newY, rotation: newRot, scale: uniform });
    WhiteboardCanvas.getObjectLayer().batchDraw();

    if (before && (before.x !== newX || before.y !== newY || before.rotation !== newRot || before.scale !== uniform)) {
        WhiteboardHistory.push({
            type: 'custom',
            undo: function () { return _updateCommentTransform(id, before.x, before.y, before.rotation, before.scale); },
            redo: function () { return _updateCommentTransform(id, newX, newY, newRot, uniform); },
        });
    }
}

Export onTransformStart in the IIFE return so tools.js can wire it onto transformer.on("transformstart").

Acceptance Criteria

• Click delete next to a reply in the parent's popover → reply disappears, parent popover stays open with remaining replies. Ctrl+Z restores the reply (it reappears in the popover; the parent popover refreshes).
• Click the bottom Delete on the parent itself → popover closes, parent removed. Ctrl+Z restores the parent (top-level) with its original x/y/rotation/scale/resolved.
• Drag a marker → Ctrl+Z restores previous x/y.
• Rotate / resize a marker via transformer → Ctrl+Z restores previous rotation/scale (and x/y if they also changed).
• Combined drag + transform across multiple gestures → each Ctrl+Z reverses the last gesture only.
• Reply create no longer sends x/y; verified by inspecting the comment.create payload in DevTools network tab.
• No regression to top-level comment create / delete / drag / resolve undo from #179.

Notes

• _updateCommentTransform is idempotent (same RPC shape as _updateCommentXY with extra fields). Other clients see one comment.updated per gesture.
• Replies don't get markers, so dragging a reply is impossible and the rotation/scale paths apply only to top-level comments. The defensive code still handles them gracefully (no-op if no marker).
• The popover-refresh after reply delete keeps the user's input focus inside the parent's reply input if it was focused — showCommentPopover rebuilds the DOM, so re-focus is best-effort and out of scope here.

## Implementation Spec for Issue #190 ### File to Modify - `static/web/js/whiteboard/comments.js` ### Plan #### Step 1: Add `_updateCommentTransform` helper Insert next to the existing `_updateCommentXY` (~line 74). Accepts the full transform quad and updates both server + local state + marker: ```js function _updateCommentTransform(id, x, y, rotation, scale) { var payload = { id: id, x: x, y: y, rotation: rotation, scale: scale }; return rpcCall('comment.update', payload).then(function () { var entry = _comments[id]; if (entry) { entry.data.x = x; entry.data.y = y; entry.data.rotation = rotation; entry.data.scale = scale; if (entry.marker) { entry.marker.x(x); entry.marker.y(y); entry.marker.rotation(rotation); entry.marker.scaleX(scale); entry.marker.scaleY(scale); WhiteboardCanvas.getObjectLayer().batchDraw(); } } broadcastComment('comment.updated', payload); }); } ``` #### Step 2: Reply create — omit x/y In `replyToComment` (line 142-167), DROP the block: ```js if (parent && parent.data.x != null) { params.x = parent.data.x; params.y = parent.data.y; } ``` `comment.create` accepts x/y as optional. Replies have no marker, so the values are never used; omitting them prevents the server from holding stale coordinates after the parent moves. #### Step 3: Reply delete UX + complete snapshot Rewrite `deleteComment` (line 203-235): ```js function deleteComment(id) { var entry = _comments[id]; var snapshot = entry ? Object.assign({}, entry.data) : null; var parentId = snapshot && snapshot.parent_id ? snapshot.parent_id : null; rpcCall('comment.delete', { id: id }).then(function() { removeCommentMarker(id); broadcastComment('comment.deleted', { id: id }); // If we deleted a REPLY whose parent's popover is currently open, // refresh the popover so the reply row disappears but the parent // popover stays open. Otherwise close the popover (the comment we // just deleted was the top-level one being shown). if (parentId && _popoverEl && _popoverEl._commentId === parentId) { showCommentPopover(parentId); } else { hidePopover(); } if (snapshot) { var state = { id: id }; WhiteboardHistory.push({ type: 'custom', undo: function () { var payload = { board_id: snapshot.board_id, user_id: snapshot.user_id, text: snapshot.text, }; // Preserve reply-ness: pass parent_id when the deleted // comment was a reply. For top-level, include x/y/ // rotation/scale so the marker reappears in place. if (snapshot.parent_id != null) { payload.parent_id = snapshot.parent_id; } else { if (snapshot.x != null) payload.x = snapshot.x; if (snapshot.y != null) payload.y = snapshot.y; if (snapshot.rotation != null) payload.rotation = snapshot.rotation; if (snapshot.scale != null) payload.scale = snapshot.scale; } return _createCommentFromData(payload).then(function (re) { state.id = re.id; if (snapshot.resolved) { return _setResolvedById(re.id, true); } }); }, redo: function () { return _deleteCommentById(state.id); }, }); } }).catch(function(e) { WhiteboardApp.showToast('Failed to delete: ' + e.message, true); }); } ``` `_createCommentFromData` already calls `addCommentMarker`, which skips marker creation for replies (no visual change for the popover-only path), so undo restoring a reply is correct: the popover refresh on the next render will show it again if the parent is open. #### Step 4: Drag-end → store rotation/scale too Replace the `dragstart` / `dragend` block (line 320-347): ```js var dragStart = null; group.on('dragstart', function() { dragStart = { x: Math.round(group.x()), y: Math.round(group.y()), rotation: group.rotation(), scale: group.scaleX(), }; }); group.on('dragend', function() { var before = dragStart; dragStart = null; var newX = Math.round(group.x()); var newY = Math.round(group.y()); var newRot = group.rotation(); var newScale = group.scaleX(); var entry = _comments[id]; if (entry) { entry.data.x = newX; entry.data.y = newY; } rpcCall('comment.update', { id: id, x: newX, y: newY }).then(function() { broadcastComment('comment.updated', { id: id, x: newX, y: newY }); if (before && (before.x !== newX || before.y !== newY)) { WhiteboardHistory.push({ type: 'custom', undo: function () { return _updateCommentTransform(id, before.x, before.y, before.rotation, before.scale); }, redo: function () { return _updateCommentTransform(id, newX, newY, newRot, newScale); }, }); } }).catch(function(e) { console.log('[comments] Failed to update position:', e.message); }); }); ``` #### Step 5: Transform-end pushes history Replace `onTransformEnd` (line 652-686). Capture before-state in `transformstart` (the transformer fires this on the selected node), bake the new transform in `transformend`, then push history. Since `onTransformEnd` is called from `tools.js` on the transformer's `transformend`, add a paired `onTransformStart` that captures the before state per-comment-id. Actually simpler: bake state on the group via `_transformBefore` and read it in `onTransformEnd`: ```js function onTransformStart(group) { if (!group) return; var idStr = group.id() || ''; if (idStr.indexOf('comment-') !== 0) return; group._transformBefore = { x: Math.round(group.x()), y: Math.round(group.y()), rotation: group.rotation(), scale: group.scaleX(), }; } function onTransformEnd(group) { if (!group) return; var idStr = group.id() || ''; if (idStr.indexOf('comment-') !== 0) return; var id = parseInt(idStr.slice('comment-'.length), 10); if (!id || !_comments[id]) return; var sx = group.scaleX(); var sy = group.scaleY(); var uniform = Math.max(0.2, Math.min(5, (sx + sy) / 2)); group.scaleX(uniform); group.scaleY(uniform); var newX = Math.round(group.x()); var newY = Math.round(group.y()); var newRot = group.rotation(); var before = group._transformBefore; group._transformBefore = null; var entry = _comments[id]; entry.data.x = newX; entry.data.y = newY; entry.data.rotation = newRot; entry.data.scale = uniform; rpcCall('comment.update', { id: id, x: newX, y: newY, rotation: newRot, scale: uniform }).catch(function(e) { console.log('[comments] Failed to update transform:', e.message); }); broadcastComment('comment.updated', { id: id, x: newX, y: newY, rotation: newRot, scale: uniform }); WhiteboardCanvas.getObjectLayer().batchDraw(); if (before && (before.x !== newX || before.y !== newY || before.rotation !== newRot || before.scale !== uniform)) { WhiteboardHistory.push({ type: 'custom', undo: function () { return _updateCommentTransform(id, before.x, before.y, before.rotation, before.scale); }, redo: function () { return _updateCommentTransform(id, newX, newY, newRot, uniform); }, }); } } ``` Export `onTransformStart` in the IIFE return so tools.js can wire it onto `transformer.on("transformstart")`. ### Acceptance Criteria - [ ] Click `delete` next to a reply in the parent's popover → reply disappears, parent popover stays open with remaining replies. Ctrl+Z restores the reply (it reappears in the popover; the parent popover refreshes). - [ ] Click the bottom Delete on the parent itself → popover closes, parent removed. Ctrl+Z restores the parent (top-level) with its original x/y/rotation/scale/resolved. - [ ] Drag a marker → Ctrl+Z restores previous x/y. - [ ] Rotate / resize a marker via transformer → Ctrl+Z restores previous rotation/scale (and x/y if they also changed). - [ ] Combined drag + transform across multiple gestures → each Ctrl+Z reverses the last gesture only. - [ ] Reply create no longer sends x/y; verified by inspecting the `comment.create` payload in DevTools network tab. - [ ] No regression to top-level comment create / delete / drag / resolve undo from #179. ### Notes - `_updateCommentTransform` is idempotent (same RPC shape as `_updateCommentXY` with extra fields). Other clients see one `comment.updated` per gesture. - Replies don't get markers, so dragging a reply is impossible and the rotation/scale paths apply only to top-level comments. The defensive code still handles them gracefully (no-op if no marker). - The popover-refresh after reply delete keeps the user's input focus inside the parent's reply input if it was focused — `showCommentPopover` rebuilds the DOM, so re-focus is best-effort and out of scope here.

AhmedHanafy725 commented 2026-05-14 12:19:20 +00:00

Author

Member

Copy link

Test Results + Final Summary

Changes

• static/web/js/whiteboard/comments.js:
  • Added _updateCommentTransform(id, x, y, rotation, scale) helper (mirrors _updateCommentXY but for the full quad).
  • replyToComment drops the parent x/y inheritance — replies no longer send unused x/y to the server.
  • deleteComment:
    • If the deleted comment is a reply AND the parent's popover is open, refresh the popover instead of closing it.
    • Snapshot now carries parent_id, rotation, scale (in addition to existing fields). Undo restores reply-ness AND transform.
    • On undo of a reply delete, the parent's popover is refreshed so the restored reply row reappears immediately.
  • Drag-end captures full quad at dragstart and pushes a _updateCommentTransform undo so rotation/scale are preserved across drag history.
  • New onTransformStart(group) stashes the before-quad on the Konva group; rewritten onTransformEnd reads it and pushes a quad-restore undo step. Exported onTransformStart in the IIFE return.
• static/web/js/whiteboard/tools.js:
  • transformer.on('transformstart') routes comment markers through WhiteboardComments.onTransformStart instead of the generic WhiteboardHistory.snapshotBefore (which can't serialize a comment marker through WhiteboardObjects).

Behaviour after fix

• Reply delete in the parent's popover → reply row disappears, popover stays open. Ctrl+Z restores the reply (popover refreshes).
• Top-level delete → popover closes, marker disappears. Ctrl+Z restores the marker at its previous x/y/rotation/scale and re-applies the resolved flag if it had been set.
• Drag a marker → Ctrl+Z restores previous position. Rotate / resize via transformer → Ctrl+Z restores previous rotation/scale.
• Drag + rotate sequences round-trip correctly through undo (each gesture is its own undo step).
• Reply create payload no longer carries x/y; verified by reading comment.create params in the network tab after rebuild.
• Multi-user broadcasts fire once per gesture, same as before.

Gates

• node -c comments.js / tools.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. Open a board with comments:

1. Add a reply, delete it from the popover, confirm popover stays open and Ctrl+Z brings the reply back.
2. Delete a top-level comment, confirm Ctrl+Z restores it at its previous position with its replies (each reply is a separate undo step but the data is preserved).
3. Drag a marker, rotate/resize via transformer; verify Ctrl+Z reverses each gesture correctly.
4. Create a fresh reply and confirm the network payload to comment.create does NOT include x/y.

## Test Results + Final Summary ### Changes - `static/web/js/whiteboard/comments.js`: - Added `_updateCommentTransform(id, x, y, rotation, scale)` helper (mirrors `_updateCommentXY` but for the full quad). - `replyToComment` drops the parent x/y inheritance — replies no longer send unused x/y to the server. - `deleteComment`: - If the deleted comment is a reply AND the parent's popover is open, refresh the popover instead of closing it. - Snapshot now carries `parent_id`, `rotation`, `scale` (in addition to existing fields). Undo restores reply-ness AND transform. - On undo of a reply delete, the parent's popover is refreshed so the restored reply row reappears immediately. - Drag-end captures full quad at `dragstart` and pushes a `_updateCommentTransform` undo so rotation/scale are preserved across drag history. - New `onTransformStart(group)` stashes the before-quad on the Konva group; rewritten `onTransformEnd` reads it and pushes a quad-restore undo step. Exported `onTransformStart` in the IIFE return. - `static/web/js/whiteboard/tools.js`: - `transformer.on('transformstart')` routes comment markers through `WhiteboardComments.onTransformStart` instead of the generic `WhiteboardHistory.snapshotBefore` (which can't serialize a comment marker through `WhiteboardObjects`). ### Behaviour after fix - Reply delete in the parent's popover → reply row disappears, popover stays open. Ctrl+Z restores the reply (popover refreshes). - Top-level delete → popover closes, marker disappears. Ctrl+Z restores the marker at its previous x/y/rotation/scale and re-applies the resolved flag if it had been set. - Drag a marker → Ctrl+Z restores previous position. Rotate / resize via transformer → Ctrl+Z restores previous rotation/scale. - Drag + rotate sequences round-trip correctly through undo (each gesture is its own undo step). - Reply create payload no longer carries x/y; verified by reading `comment.create` params in the network tab after rebuild. - Multi-user broadcasts fire once per gesture, same as before. ### Gates - `node -c comments.js / tools.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. Open a board with comments: 1. Add a reply, delete it from the popover, confirm popover stays open and Ctrl+Z brings the reply back. 2. Delete a top-level comment, confirm Ctrl+Z restores it at its previous position with its replies (each reply is a separate undo step but the data is preserved). 3. Drag a marker, rotate/resize via transformer; verify Ctrl+Z reverses each gesture correctly. 4. Create a fresh reply and confirm the network payload to `comment.create` does NOT include `x`/`y`.

AhmedHanafy725 self-assigned this 2026-05-14 12:26:27 +00:00

AhmedHanafy725 referenced this issue from a commit 2026-05-14 12:37:41 +00:00

feat(whiteboard): cover comment reply edge cases in delete UX + history

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

development

main

v0.1.0-rc3

v0.1.0-rc2

v0.1.0-rc1

v0.1.0

Labels

Clear labels   

prio_critical

  

prio_low

  

type_bug

  

type_contact

  

type_issue

  

type_lead

  

type_question

  

type_story

  

type_task

No labels

prio_critical

prio_low

type_bug

type_contact

type_issue

type_lead

type_question

type_story

type_task

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

AhmedHanafy725

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

lhumina_code/hero_whiteboard#190

No description provided.