lhumina_code/hero_whiteboard

Fork 0

Mind map can be edited while locked #156

New issue

Open

opened 2026-05-06 09:57:33 +00:00 by AhmedHanafy725 · 3 comments

AhmedHanafy725 commented

2026-05-06 09:57:33 +00:00

Member

Expected

When WhiteboardObjects.isLocked(group.id()) is true:

Right-click on a node either does nothing or shows a read-only menu (greyed-out / no destructive items).
Add Comment / Add Child / Delete Node menu actions do nothing.
Comment popup save/remove buttons are inert.
Inline node-text and title editors do not open.
Direction-flip arrow does nothing.
Collapse +/− toggle does nothing.
Add-child + button does nothing.
Comment 💬 icon click does nothing (or opens a read-only popup).

Actual

Most click handlers already gate on WhiteboardObjects.isLocked(group.id()) (collapse toggle, add-child button, comment icon click, double-click to edit), but:

The onNodeContextMenu handler does not check lock — the per-node right-click menu opens and its actions run unconditionally.
The standard whiteboard context menu (used by other shapes when locked) shows a Locked-aware menu; the mind-map per-node menu does not.

Repro

Open a board, create a mind map
Select the mindmap, lock it via the toolbar / context menu
Right-click any node → menu appears, Delete Node / Add Child / Add Comment all run successfully

Notes

The lock-respecting pattern used elsewhere in mindmap.js:

if (typeof WhiteboardObjects !== "undefined" && WhiteboardObjects.isLocked && WhiteboardObjects.isLocked(group.id())) return;

Apply the same gate to onNodeContextMenu (and verify any other un-gated entry points for editing).

When a mind map object is locked, the user can still edit it via the right-click context menu (Add Comment / Add Child / Delete Node) and possibly other paths. Locking should prevent all edits — only inspection and selection should remain. ## Expected When `WhiteboardObjects.isLocked(group.id())` is true: - Right-click on a node either does nothing or shows a read-only menu (greyed-out / no destructive items). - Add Comment / Add Child / Delete Node menu actions do nothing. - Comment popup save/remove buttons are inert. - Inline node-text and title editors do not open. - Direction-flip arrow does nothing. - Collapse `+`/`−` toggle does nothing. - Add-child `+` button does nothing. - Comment 💬 icon click does nothing (or opens a read-only popup). ## Actual Most click handlers already gate on `WhiteboardObjects.isLocked(group.id())` (collapse toggle, add-child button, comment icon click, double-click to edit), but: - The `onNodeContextMenu` handler does not check lock — the per-node right-click menu opens and its actions run unconditionally. - The standard whiteboard context menu (used by other shapes when locked) shows a Locked-aware menu; the mind-map per-node menu does not. ## Repro 1. Open a board, create a mind map 2. Select the mindmap, lock it via the toolbar / context menu 3. Right-click any node → menu appears, Delete Node / Add Child / Add Comment all run successfully ## Notes The lock-respecting pattern used elsewhere in `mindmap.js`: ```js if (typeof WhiteboardObjects !== "undefined" && WhiteboardObjects.isLocked && WhiteboardObjects.isLocked(group.id())) return; ``` Apply the same gate to `onNodeContextMenu` (and verify any other un-gated entry points for editing).

AhmedHanafy725 commented

2026-05-06 10:00:33 +00:00

Author

Member

Implementation Spec

Root cause

renderMindmap and helpers wire per-node interaction handlers in drawNodes. Most begin with the standard guard:

if (typeof WhiteboardObjects !== 'undefined' && WhiteboardObjects.isLocked && WhiteboardObjects.isLocked(group.id())) return;

But the per-node contextmenu handler onNodeContextMenu (mindmap.js ~L427) does not. Because that handler is wired to every per-node sub-shape and calls e.evt.stopPropagation() + e.cancelBubble = true, right-clicking a node on a locked mind map suppresses the document-level standard menu and opens the per-node menu — Add/Edit Comment, Add Child, Delete Node all fully functional.

Two secondary gaps:

showCommentPopup Save/Remove buttons never re-check isLocked. The popup is detached DOM; can survive a setLocked(id, true) arriving over the wire.
addChildToNode / deleteNode / flipDirection / toggleCollapseRoot don't gate themselves. Today every caller is gated, but defense-in-depth costs a couple of lines each.

Audit (mindmap.js)

Entry point	Gated today?
`titleText` dblclick → `editMindmapTitle`	YES
`dirBtn` click → `flipDirection`	YES
`indicator` click (collapse)	YES
`addBtn` click → `addChildToNode`	YES
`commentIcon` click → `showCommentPopup`	YES
`nodeRect` / `label` dblclick → `editMindmapNode`	YES
`onNodeContextMenu` (per-node menu)	NO — bug
`editMindmapNode` / `editMindmapTitle` blur save	NO (entered via gated dblclick)
`showCommentPopup` Save / Remove handlers	NO (entered via gated entry)
`addChildToNode` / `deleteNode` / `flipDirection` / `toggleCollapseRoot` (function-level)	NO
`dragstart` / `dragend`	OK — handled by `group.draggable(false)` from `setLocked`

contextmenu.js shows the canvas menu for locked objects with destructive items hidden and the Lock item rewritten as Unlock. The mind-map per-node menu has no Unlock equivalent and every item is destructive (mutates the tree). Cleanest match: defer to the standard canvas menu when locked. In onNodeContextMenu, if locked, just return — without stopPropagation/preventDefault — and the document-level handler in contextmenu.js will fire the standard menu.

Approach: per-handler gates

Matches existing style. Every other gated site uses the literal if (typeof WhiteboardObjects !== 'undefined' && ...isLocked(group.id())) return; pattern (7 occurrences). Adding the same line at the un-gated sites keeps the codebase greppable and avoids churning the recently-landed e3c4cb8 (#154) and 92f9859 (#155).

Files to Modify

crates/hero_whiteboard_ui/static/web/js/whiteboard/mindmap.js — only file.

Implementation Plan

Step 1 — Primary fix: gate `onNodeContextMenu`

function onNodeContextMenu(e) {
    if (typeof WhiteboardObjects !== 'undefined' && WhiteboardObjects.isLocked && WhiteboardObjects.isLocked(group.id())) {
        return; // let the standard canvas context menu handle it
    }
    e.evt.preventDefault();
    e.evt.stopPropagation();
    e.cancelBubble = true;
    showNodeContextMenu(group, node, nodeRect, e.evt);
}

Closing this issue as filed.

Step 2 — Defense-in-depth at function level

Add the standard guard as the first line of:

addChildToNode(group, node)
deleteNode(group, nodeData)
flipDirection(group)
toggleCollapseRoot(group)

(addChildToRoot delegates to addChildToNode — covered transitively.)

Step 3 — Edge case: discard pending edits if locked between open and commit

In the blur / Save / Remove handlers of editMindmapNode, editMindmapTitle, and showCommentPopup: skip the mutation and WhiteboardSync.onUpdate(group) when isLocked returns true at commit time, but still tear down the DOM cleanly.

Dependencies: none.

Acceptance Criteria

Locked mindmap: right-click on any node sub-shape opens the standard canvas/object menu (with Unlock), not the per-node menu.
Locked mindmap: double-click on title / rect / label does not open an inline editor.
Locked mindmap: single-click on +/−, +, 💬, ↕/↔ does nothing.
Locked mindmap: cannot be dragged.
Unlocking restores all interactions byte-identically to before.
Unlocked mindmap: every interaction behaves identically to today.
addChildToNode / deleteNode / flipDirection / toggleCollapseRoot are no-ops when called against a locked group, even outside wired handlers.
If setLocked(id, true) arrives while an inline editor / comment popup is open, the next blur/Save discards the edit (no WhiteboardSync.onUpdate); the popup/editor still closes cleanly.

What NOT to break

Commit e3c4cb8 (#154 wiring): all five contextmenu registrations on per-node shapes remain.
Commit 92f9859 (#155 unification): per-node menu items and WhiteboardContextMenu.show integration unchanged — they simply aren't reached when locked.
Non-locked behavior at every entry point byte-identical to today.
Existing 7 gated sites untouched.

## Implementation Spec ### Root cause `renderMindmap` and helpers wire per-node interaction handlers in `drawNodes`. Most begin with the standard guard: ```js if (typeof WhiteboardObjects !== 'undefined' && WhiteboardObjects.isLocked && WhiteboardObjects.isLocked(group.id())) return; ``` But the per-node `contextmenu` handler `onNodeContextMenu` (mindmap.js ~L427) does **not**. Because that handler is wired to every per-node sub-shape and calls `e.evt.stopPropagation()` + `e.cancelBubble = true`, right-clicking a node on a locked mind map suppresses the document-level standard menu and opens the per-node menu — Add/Edit Comment, Add Child, Delete Node all fully functional. Two secondary gaps: - `showCommentPopup` Save/Remove buttons never re-check `isLocked`. The popup is detached DOM; can survive a `setLocked(id, true)` arriving over the wire. - `addChildToNode` / `deleteNode` / `flipDirection` / `toggleCollapseRoot` don't gate themselves. Today every caller is gated, but defense-in-depth costs a couple of lines each. ### Audit (mindmap.js) | Entry point | Gated today? | |---|---| | `titleText` dblclick → `editMindmapTitle` | YES | | `dirBtn` click → `flipDirection` | YES | | `indicator` click (collapse) | YES | | `addBtn` click → `addChildToNode` | YES | | `commentIcon` click → `showCommentPopup` | YES | | `nodeRect` / `label` dblclick → `editMindmapNode` | YES | | `onNodeContextMenu` (per-node menu) | **NO — bug** | | `editMindmapNode` / `editMindmapTitle` blur save | NO (entered via gated dblclick) | | `showCommentPopup` Save / Remove handlers | NO (entered via gated entry) | | `addChildToNode` / `deleteNode` / `flipDirection` / `toggleCollapseRoot` (function-level) | NO | | `dragstart` / `dragend` | OK — handled by `group.draggable(false)` from `setLocked` | ### Standard menu's locked behavior `contextmenu.js` shows the canvas menu for locked objects with destructive items hidden and the Lock item rewritten as Unlock. The mind-map per-node menu has no Unlock equivalent and every item is destructive (mutates the tree). Cleanest match: **defer to the standard canvas menu** when locked. In `onNodeContextMenu`, if locked, just `return` — without `stopPropagation`/`preventDefault` — and the document-level handler in `contextmenu.js` will fire the standard menu. ### Approach: per-handler gates Matches existing style. Every other gated site uses the literal `if (typeof WhiteboardObjects !== 'undefined' && ...isLocked(group.id())) return;` pattern (7 occurrences). Adding the same line at the un-gated sites keeps the codebase greppable and avoids churning the recently-landed `e3c4cb8` (#154) and `92f9859` (#155). ### Files to Modify - `crates/hero_whiteboard_ui/static/web/js/whiteboard/mindmap.js` — only file. ### Implementation Plan #### Step 1 — Primary fix: gate `onNodeContextMenu` ```js function onNodeContextMenu(e) { if (typeof WhiteboardObjects !== 'undefined' && WhiteboardObjects.isLocked && WhiteboardObjects.isLocked(group.id())) { return; // let the standard canvas context menu handle it } e.evt.preventDefault(); e.evt.stopPropagation(); e.cancelBubble = true; showNodeContextMenu(group, node, nodeRect, e.evt); } ``` Closing this issue as filed. #### Step 2 — Defense-in-depth at function level Add the standard guard as the first line of: - `addChildToNode(group, node)` - `deleteNode(group, nodeData)` - `flipDirection(group)` - `toggleCollapseRoot(group)` (`addChildToRoot` delegates to `addChildToNode` — covered transitively.) #### Step 3 — Edge case: discard pending edits if locked between open and commit In the blur / Save / Remove handlers of `editMindmapNode`, `editMindmapTitle`, and `showCommentPopup`: skip the mutation and `WhiteboardSync.onUpdate(group)` when `isLocked` returns true at commit time, but still tear down the DOM cleanly. Dependencies: none. ### Acceptance Criteria - [ ] Locked mindmap: right-click on any node sub-shape opens the standard canvas/object menu (with Unlock), not the per-node menu. - [ ] Locked mindmap: double-click on title / rect / label does not open an inline editor. - [ ] Locked mindmap: single-click on +/−, +, 💬, ↕/↔ does nothing. - [ ] Locked mindmap: cannot be dragged. - [ ] Unlocking restores all interactions byte-identically to before. - [ ] Unlocked mindmap: every interaction behaves identically to today. - [ ] `addChildToNode` / `deleteNode` / `flipDirection` / `toggleCollapseRoot` are no-ops when called against a locked group, even outside wired handlers. - [ ] If `setLocked(id, true)` arrives while an inline editor / comment popup is open, the next blur/Save discards the edit (no `WhiteboardSync.onUpdate`); the popup/editor still closes cleanly. ### What NOT to break - Commit `e3c4cb8` (#154 wiring): all five `contextmenu` registrations on per-node shapes remain. - Commit `92f9859` (#155 unification): per-node menu items and `WhiteboardContextMenu.show` integration unchanged — they simply aren't reached when locked. - Non-locked behavior at every entry point byte-identical to today. - Existing 7 gated sites untouched.

AhmedHanafy725 commented

2026-05-06 10:03:57 +00:00

Author

Member

Validation

Check	Result
Files changed	1 file (mindmap.js, +26/-0)
`cargo check --workspace`	pass
`cargo test --workspace --lib`	pass

## Validation | Check | Result | |---|---| | Files changed | 1 file (mindmap.js, +26/-0) | | `cargo check --workspace` | pass | | `cargo test --workspace --lib` | pass |

AhmedHanafy725 commented

2026-05-06 10:04:22 +00:00

Author

Member

Implementation summary

Changes

crates/hero_whiteboard_ui/static/web/js/whiteboard/mindmap.js — added the standard WhiteboardObjects.isLocked(group.id()) guard at 9 new sites (going from 7 → 16 gated sites). All edits use the exact existing guard pattern so the codebase remains greppable.
- Primary fix in onNodeContextMenu: early-return BEFORE preventDefault/stopPropagation, so right-clicks on a locked mindmap fall through to the document-level handler in contextmenu.js and show the standard canvas/object menu (with Unlock).
- Function-level defense-in-depth at the head of addChildToNode, deleteNode, flipDirection, toggleCollapseRoot — any future caller (programmatic, scripted) is also gated.
- Commit-time edge case in editMindmapNode blur, editMindmapTitle blur, showCommentPopup Save, and showCommentPopup Remove: if setLocked arrives over the wire while an editor or popup is open, the next blur/click discards the edit (no WhiteboardSync.onUpdate) but still tears the DOM down cleanly.

The 7 pre-existing guards and the wiring from e3c4cb8 (#154) and 92f9859 (#155) are untouched.

Validation

cargo check --workspace: pass
cargo test --workspace --lib: pass
Diff scope: 1 file, +26/-0

Notes

UI-only change. Verify:

Lock a mindmap → right-click any node sub-shape → standard canvas menu (with Unlock) appears, not the per-node menu.
Lock → double-click rect/label/title → no inline editor.
Lock → click +/-, +, 💬, ↕/↔ → no effect.
Unlock → all interactions byte-identical to before.
(Edge) Open the comment popup, then have another tab lock the board → click Save → popup closes, no comment change synced.

## Implementation summary ### Changes - `crates/hero_whiteboard_ui/static/web/js/whiteboard/mindmap.js` — added the standard `WhiteboardObjects.isLocked(group.id())` guard at 9 new sites (going from 7 → 16 gated sites). All edits use the exact existing guard pattern so the codebase remains greppable. - **Primary fix** in `onNodeContextMenu`: early-return BEFORE `preventDefault`/`stopPropagation`, so right-clicks on a locked mindmap fall through to the document-level handler in `contextmenu.js` and show the standard canvas/object menu (with Unlock). - **Function-level defense-in-depth** at the head of `addChildToNode`, `deleteNode`, `flipDirection`, `toggleCollapseRoot` — any future caller (programmatic, scripted) is also gated. - **Commit-time edge case** in `editMindmapNode` blur, `editMindmapTitle` blur, `showCommentPopup` Save, and `showCommentPopup` Remove: if `setLocked` arrives over the wire while an editor or popup is open, the next blur/click discards the edit (no `WhiteboardSync.onUpdate`) but still tears the DOM down cleanly. The 7 pre-existing guards and the wiring from `e3c4cb8` (#154) and `92f9859` (#155) are untouched. ### Validation - `cargo check --workspace`: pass - `cargo test --workspace --lib`: pass - Diff scope: 1 file, +26/-0 ### Notes UI-only change. Verify: 1. Lock a mindmap → right-click any node sub-shape → standard canvas menu (with Unlock) appears, not the per-node menu. 2. Lock → double-click rect/label/title → no inline editor. 3. Lock → click +/-, +, 💬, ↕/↔ → no effect. 4. Unlock → all interactions byte-identical to before. 5. (Edge) Open the comment popup, then have another tab lock the board → click Save → popup closes, no comment change synced.