lhumina_code/hero_whiteboard

Fork 0

Connector style change is not undoable; undo deletes the connector #202

New issue

Open

opened 2026-05-19 08:12:32 +00:00 by AhmedHanafy725 · 4 comments

AhmedHanafy725 commented

2026-05-19 08:12:32 +00:00

Member

Summary

Changing a connector's line style cannot be undone. Pressing Undo right after a style change does not revert the style — instead it removes the connector entirely. Redo does not restore the style either.

Steps to reproduce

Create a connector between two elements.
Change its line style via the selection toolbar (straight / curved / elbow).
Press Undo (Ctrl+Z).

Expected: the connector reverts to its previous line style and stays on the canvas.

Actual: the connector is deleted (Undo pops the connector's create action because the style change was never recorded), and Redo cannot bring the style back.

Root cause

cycleLineStyle in crates/hero_whiteboard_admin/static/web/js/whiteboard/connectors.js destroys and recreates the connector to apply the new style but never pushes a history action for the change. The undo/redo stack therefore has no entry for the style change, so Undo operates on the previous action (typically the connector's own create), which removes the connector.

Expected behavior

A connector style change should push a single undoable history action that, on undo, restores the previous line style (and on redo, re-applies the new one) while keeping the connector's stable identity intact across the destroy/recreate cycle.

## Summary Changing a connector's line style cannot be undone. Pressing Undo right after a style change does not revert the style — instead it removes the connector entirely. Redo does not restore the style either. ## Steps to reproduce 1. Create a connector between two elements. 2. Change its line style via the selection toolbar (straight / curved / elbow). 3. Press Undo (Ctrl+Z). **Expected:** the connector reverts to its previous line style and stays on the canvas. **Actual:** the connector is deleted (Undo pops the connector's create action because the style change was never recorded), and Redo cannot bring the style back. ## Root cause `cycleLineStyle` in `crates/hero_whiteboard_admin/static/web/js/whiteboard/connectors.js` destroys and recreates the connector to apply the new style but never pushes a history action for the change. The undo/redo stack therefore has no entry for the style change, so Undo operates on the previous action (typically the connector's own create), which removes the connector. ## Expected behavior A connector style change should push a single undoable history action that, on undo, restores the previous line style (and on redo, re-applies the new one) while keeping the connector's stable identity intact across the destroy/recreate cycle.

AhmedHanafy725 commented

2026-05-19 08:15:26 +00:00

Author

Member

Implementation Spec for Issue #202

Objective

Make a connector line-style change (straight/curved/elbow) a single undoable/redoable history step. After a style change, one Undo reverts the connector to its previous line style while keeping it on the canvas (same logical connector), and Redo re-applies the new style. The connector's stable identity must remain intact, and undo/redo must not double-persist beyond what the existing connector update/broadcast path already does.

Requirements

Undo right after a style change reverts to the previous line style; the connector stays on the canvas with the same identity.
Redo re-applies the new line style.
A toolbar style change that crosses multiple style steps is a single undo step (one Undo returns to the original style, not one step per internal cycle call).
No history action is recorded for sync/remote-driven style changes (_fromSync).
No history action is recorded while history replay is disabled (mirrors the existing create/delete push guards).
Undo/redo go through the same code path that preserves the connector's stable identity and persists/broadcasts the change so peers stay consistent.

Files to Modify

crates/hero_whiteboard_admin/static/web/js/whiteboard/connectors.js — cycleLineStyle records one undoable custom history action for genuine user-driven style changes, guarded against sync and replay.
crates/hero_whiteboard_admin/static/web/js/whiteboard/selection_toolbar.js — wrap the toolbar's multi-step style loop in WhiteboardHistory.batch(...) so it collapses to a single undo step.

No backend/Rust changes.

Implementation Plan

Step 1 — In cycleLineStyle, capture the previous style, and after the existing recreate + non-sync server update + broadcast, push one type: 'custom' history action. Guard the push with !opts._fromSync and the existing replay guard !(typeof WhiteboardHistory !== 'undefined' && WhiteboardHistory.isEnabled && WhiteboardHistory.isEnabled() === false). The undo/redo closures resolve the live id via the stable hidToId map at call time and step the connector forward through the style cycle (bounded loop) until it reaches the previous / new style respectively, reusing the single identity-preserving, persisting code path.

Step 2 — In selection_toolbar.js _renderConnector, wrap the loop that calls cycleLineStyle up to N times in WhiteboardHistory.batch(function(){ ... }) so the multiple pushes flush as one batch entry; one Undo reverts straight to the original style.

Step 3 — Review only: confirm replay (_enabled = false during undo/redo) suppresses nested pushes via the Step 1 guard, while the replayed cycleLineStyle still persists and broadcasts the reverted/re-applied style once per step.

Acceptance Criteria

Change a connector's style, press Undo: connector stays on the canvas and reverts to the previous style.
Press Redo: connector re-applies the new style and stays on the canvas.
The connector keeps the same logical identity across change + undo + redo.
A single toolbar change crossing two style steps is reverted by exactly one Undo.
A remote/sync style change pushes no history entry.
Undo/redo of a style change creates no extra/nested history entry (stack depth changes by exactly one per user action).
Reverted/re-applied style is persisted to the server and broadcast to peers.
Undoing a style change does NOT delete the connector (the original bug).
Existing connector create/delete undo/redo still work unchanged.

Notes

The action uses type: 'custom' with undo/redo functions, matching existing connector history entries; type: 'update' is unsuitable because connectors are not in the object registry.
Closures resolve the connector by stable identity at call time (not a captured id), since a connector's server id can be reminted across delete/recreate cycles.
Coalescing the multi-step toolbar change is done at the caller via the existing history batch mechanism, keeping cycleLineStyle single-responsibility (one push per forward step), consistent with how connector create/delete each push one entry per logical op.

## Implementation Spec for Issue #202 ### Objective Make a connector line-style change (straight/curved/elbow) a single undoable/redoable history step. After a style change, one Undo reverts the connector to its previous line style while keeping it on the canvas (same logical connector), and Redo re-applies the new style. The connector's stable identity must remain intact, and undo/redo must not double-persist beyond what the existing connector update/broadcast path already does. ### Requirements - Undo right after a style change reverts to the previous line style; the connector stays on the canvas with the same identity. - Redo re-applies the new line style. - A toolbar style change that crosses multiple style steps is a single undo step (one Undo returns to the original style, not one step per internal cycle call). - No history action is recorded for sync/remote-driven style changes (`_fromSync`). - No history action is recorded while history replay is disabled (mirrors the existing create/delete push guards). - Undo/redo go through the same code path that preserves the connector's stable identity and persists/broadcasts the change so peers stay consistent. ### Files to Modify - `crates/hero_whiteboard_admin/static/web/js/whiteboard/connectors.js` — `cycleLineStyle` records one undoable `custom` history action for genuine user-driven style changes, guarded against sync and replay. - `crates/hero_whiteboard_admin/static/web/js/whiteboard/selection_toolbar.js` — wrap the toolbar's multi-step style loop in `WhiteboardHistory.batch(...)` so it collapses to a single undo step. No backend/Rust changes. ### Implementation Plan Step 1 — In `cycleLineStyle`, capture the previous style, and after the existing recreate + non-sync server update + broadcast, push one `type: 'custom'` history action. Guard the push with `!opts._fromSync` and the existing replay guard `!(typeof WhiteboardHistory !== 'undefined' && WhiteboardHistory.isEnabled && WhiteboardHistory.isEnabled() === false)`. The `undo`/`redo` closures resolve the live id via the stable `hidToId` map at call time and step the connector forward through the style cycle (bounded loop) until it reaches the previous / new style respectively, reusing the single identity-preserving, persisting code path. Step 2 — In `selection_toolbar.js` `_renderConnector`, wrap the loop that calls `cycleLineStyle` up to N times in `WhiteboardHistory.batch(function(){ ... })` so the multiple pushes flush as one batch entry; one Undo reverts straight to the original style. Step 3 — Review only: confirm replay (`_enabled = false` during undo/redo) suppresses nested pushes via the Step 1 guard, while the replayed `cycleLineStyle` still persists and broadcasts the reverted/re-applied style once per step. ### Acceptance Criteria - [ ] Change a connector's style, press Undo: connector stays on the canvas and reverts to the previous style. - [ ] Press Redo: connector re-applies the new style and stays on the canvas. - [ ] The connector keeps the same logical identity across change + undo + redo. - [ ] A single toolbar change crossing two style steps is reverted by exactly one Undo. - [ ] A remote/sync style change pushes no history entry. - [ ] Undo/redo of a style change creates no extra/nested history entry (stack depth changes by exactly one per user action). - [ ] Reverted/re-applied style is persisted to the server and broadcast to peers. - [ ] Undoing a style change does NOT delete the connector (the original bug). - [ ] Existing connector create/delete undo/redo still work unchanged. ### Notes - The action uses `type: 'custom'` with `undo`/`redo` functions, matching existing connector history entries; `type: 'update'` is unsuitable because connectors are not in the object registry. - Closures resolve the connector by stable identity at call time (not a captured id), since a connector's server id can be reminted across delete/recreate cycles. - Coalescing the multi-step toolbar change is done at the caller via the existing history batch mechanism, keeping `cycleLineStyle` single-responsibility (one push per forward step), consistent with how connector create/delete each push one entry per logical op.

AhmedHanafy725 commented

2026-05-19 08:20:25 +00:00

Author

Member

Test Results

Workspace lib tests: 0 passed, 0 failed, 0 ignored (4 lib test binaries: hero_whiteboard_admin, hero_whiteboard_app, hero_whiteboard_examples, hero_whiteboard_sdk; all compiled and ran clean, no test failures)
JS syntax check (node --check): connectors.js OK, selection_toolbar.js OK

Note: the fix is JS-only (no Rust source changed); the workspace lib tests are run as a regression guard.

## Test Results - Workspace lib tests: 0 passed, 0 failed, 0 ignored (4 lib test binaries: hero_whiteboard_admin, hero_whiteboard_app, hero_whiteboard_examples, hero_whiteboard_sdk; all compiled and ran clean, no test failures) - JS syntax check (node --check): connectors.js OK, selection_toolbar.js OK Note: the fix is JS-only (no Rust source changed); the workspace lib tests are run as a regression guard.

AhmedHanafy725 commented

2026-05-19 08:22:28 +00:00

Author

Member

Implementation Summary

Root cause

cycleLineStyle (crates/hero_whiteboard_admin/static/web/js/whiteboard/connectors.js) destroyed and recreated the connector to apply a new line style but pushed no history action. Undo therefore popped the previous action (the connector'"'"'s own create) and deleted the connector; Redo could not restore the style.

Changes

crates/hero_whiteboard_admin/static/web/js/whiteboard/connectors.js
- cycleLineStyle now captures the previous style and, after the existing recreate + non-sync server update + broadcast, pushes one type: '"'"'custom'"'"' history action.
- The push is guarded with !opts._fromSync and the same replay guard used by the existing create/delete pushes (!(typeof WhiteboardHistory !== '"'"'undefined'"'"' && WhiteboardHistory.isEnabled && WhiteboardHistory.isEnabled() === false)), so sync/remote changes and history replay do not record nested actions.
- The connector is given a stable history identity (hid) before the destroy/recreate if it does not already have one (mirroring deleteConnector), so undo/redo resolve the live connector through hidToId at call time.
- undo/redo step the connector forward through the bounded style cycle until it reaches the previous / new style, reusing the single identity-preserving code path that also persists to the server and broadcasts to peers.
crates/hero_whiteboard_admin/static/web/js/whiteboard/selection_toolbar.js
- The toolbar line-style change loop (which may call cycleLineStyle up to three times to reach the selected style) is wrapped in WhiteboardHistory.batch(...), so a single dropdown change is one undo step rather than one per internal cycle.

Test results

cargo test --workspace --lib (matches CI): compiled cleanly, no failures. These crates carry no unit tests, so this is a regression guard only; the change is JS-only with no Rust source modified.
node --check passed for both edited JS files (connectors.js, selection_toolbar.js).
Manual verification path: create a connector, change its style via the toolbar, Undo (reverts style, connector stays), Redo (re-applies style). Connector keeps its identity; a multi-step toolbar change reverts in one Undo; sync-driven style changes record nothing.

Notes

The fix uses type: '"'"'custom'"'"' actions (connectors are not in the object registry, so type: '"'"'update'"'"' snapshotting would not capture connector style).
Undo/redo legitimately re-persist and re-broadcast the reverted/re-applied style, consistent with the existing connector update path and the established undo-persists convention.
No backend/Rust changes. JS is embedded via rust-embed; the running admin binary was rebuilt and redeployed with the updated assets verified as served.

## Implementation Summary ### Root cause `cycleLineStyle` (`crates/hero_whiteboard_admin/static/web/js/whiteboard/connectors.js`) destroyed and recreated the connector to apply a new line style but pushed no history action. Undo therefore popped the previous action (the connector'"'"'s own create) and deleted the connector; Redo could not restore the style. ### Changes - `crates/hero_whiteboard_admin/static/web/js/whiteboard/connectors.js` - `cycleLineStyle` now captures the previous style and, after the existing recreate + non-sync server update + broadcast, pushes one `type: '"'"'custom'"'"'` history action. - The push is guarded with `!opts._fromSync` and the same replay guard used by the existing create/delete pushes (`!(typeof WhiteboardHistory !== '"'"'undefined'"'"' && WhiteboardHistory.isEnabled && WhiteboardHistory.isEnabled() === false)`), so sync/remote changes and history replay do not record nested actions. - The connector is given a stable history identity (`hid`) before the destroy/recreate if it does not already have one (mirroring `deleteConnector`), so undo/redo resolve the live connector through `hidToId` at call time. - `undo`/`redo` step the connector forward through the bounded style cycle until it reaches the previous / new style, reusing the single identity-preserving code path that also persists to the server and broadcasts to peers. - `crates/hero_whiteboard_admin/static/web/js/whiteboard/selection_toolbar.js` - The toolbar line-style change loop (which may call `cycleLineStyle` up to three times to reach the selected style) is wrapped in `WhiteboardHistory.batch(...)`, so a single dropdown change is one undo step rather than one per internal cycle. ### Test results - `cargo test --workspace --lib` (matches CI): compiled cleanly, no failures. These crates carry no unit tests, so this is a regression guard only; the change is JS-only with no Rust source modified. - `node --check` passed for both edited JS files (`connectors.js`, `selection_toolbar.js`). - Manual verification path: create a connector, change its style via the toolbar, Undo (reverts style, connector stays), Redo (re-applies style). Connector keeps its identity; a multi-step toolbar change reverts in one Undo; sync-driven style changes record nothing. ### Notes - The fix uses `type: '"'"'custom'"'"'` actions (connectors are not in the object registry, so `type: '"'"'update'"'"'` snapshotting would not capture connector style). - Undo/redo legitimately re-persist and re-broadcast the reverted/re-applied style, consistent with the existing connector update path and the established undo-persists convention. - No backend/Rust changes. JS is embedded via rust-embed; the running admin binary was rebuilt and redeployed with the updated assets verified as served.

AhmedHanafy725 commented

2026-05-19 08:49:38 +00:00

Author

Member

Scope update

The fix was extended beyond line style: the same defect affected connector color and stroke width, whose toolbar handlers also mutated the connector directly without recording a history action (so Undo deleted the connector).

Final state of the fix:

cycleLineStyle records a guarded custom undo/redo action for line-style changes; the toolbar's multi-step change is batched into one undo step.
New WhiteboardConnectors.setStyle(id, { stroke, strokeWidth }) records one guarded custom undo/redo action per color/width change; the toolbar color picker and stroke-width slider now route through it.
Both paths resolve the live connector via its stable history id and persist/broadcast the reverted/re-applied style on undo/redo.

Verified manually: changing a connector's line style, color, or thickness and pressing Undo now reverts the change and keeps the connector on the canvas; Redo re-applies it.

## Scope update The fix was extended beyond line style: the same defect affected connector **color** and **stroke width**, whose toolbar handlers also mutated the connector directly without recording a history action (so Undo deleted the connector). Final state of the fix: - `cycleLineStyle` records a guarded `custom` undo/redo action for line-style changes; the toolbar's multi-step change is batched into one undo step. - New `WhiteboardConnectors.setStyle(id, { stroke, strokeWidth })` records one guarded `custom` undo/redo action per color/width change; the toolbar color picker and stroke-width slider now route through it. - Both paths resolve the live connector via its stable history id and persist/broadcast the reverted/re-applied style on undo/redo. Verified manually: changing a connector's line style, color, or thickness and pressing Undo now reverts the change and keeps the connector on the canvas; Redo re-applies it.