lhumina_code/hero_whiteboard
24
0
Fork 0
Code Issues 57 Pull requests Projects Releases Packages Wiki Activity Actions

Comment: rotation not persisted across reload #40

New issue
Open
opened 2026-04-21 14:44:51 +00:00 by AhmedHanafy725 · 2 comments
AhmedHanafy725 commented 2026-04-21 14:44:51 +00:00
Member
Copy link

Rotating a comment marker (via the transformer, now that rubber-band selection includes comments) visually rotates the pin, but on reload it comes back at rotation 0.

Root cause:

  • The dragend handler in addCommentMarker only sends {x, y} in comment.update.
  • The transformer's transformend goes through WhiteboardObjects.applyTransform which returns early for comment markers (they are not in the objects store), so the new rotation is never sent either.
  • The server comments table has no rotation column; the comment.update RPC doesn't accept rotation.

Fix: add a rotation REAL NOT NULL DEFAULT 0 column via a new migration, accept the field in comment.update, include it in the rendered row, send it from the client on dragend and on transformend (detect comment markers in the transformer path), and apply it in addCommentMarker when loading.

Rotating a comment marker (via the transformer, now that rubber-band selection includes comments) visually rotates the pin, but on reload it comes back at rotation 0. Root cause: - The `dragend` handler in `addCommentMarker` only sends `{x, y}` in `comment.update`. - The transformer's `transformend` goes through `WhiteboardObjects.applyTransform` which returns early for comment markers (they are not in the `objects` store), so the new rotation is never sent either. - The server `comments` table has no `rotation` column; the `comment.update` RPC doesn't accept `rotation`. Fix: add a `rotation REAL NOT NULL DEFAULT 0` column via a new migration, accept the field in `comment.update`, include it in the rendered row, send it from the client on dragend and on transformend (detect comment markers in the transformer path), and apply it in `addCommentMarker` when loading.
AhmedHanafy725 commented 2026-04-21 14:47:28 +00:00
Author
Member
Copy link

Consolidated Spec for Issues #36-#41

Objective

Make comments feel like first-class board objects: erasable, deletable from the sidebar, toggleable between active/resolved, non-reactive to the right mouse button, and with their rotation and scale surviving a reload.

Scope at a glance

# Fix Surface
36 Eraser also deletes comments tools.js
37 Sidebar shows comment pane with Delete / Resolve-Unresolve properties.js
38 Server supports un-resolve; popover renders Unresolve button server/*, comments.js
39 Right-click no longer opens the comment popover comments.js
40 Comment rotation persists through server schema + RPC + comments.js
41 Comment scale persists through server schema + RPC + comments.js

Files to Modify / Create

  • crates/hero_whiteboard_server/src/migrations/005_comment_transform.sql (NEW) — adds rotation REAL NOT NULL DEFAULT 0 and scale REAL NOT NULL DEFAULT 1 columns to comments.
  • crates/hero_whiteboard_server/src/db/models.rs — extend the Comment struct with rotation and scale.
  • crates/hero_whiteboard_server/src/db/queries.rs — include both columns in SELECT / INSERT / UPDATE for comments.
  • crates/hero_whiteboard_server/src/handlers/comment.rs — create accepts optional rotation / scale; update accepts rotation / scale; new set_resolved accepts a resolved bool (reuses comment.resolve method name with the new field to stay backwards-compatible).
  • crates/hero_whiteboard_server/src/db/queries.rs — rename resolve_comment to set_comment_resolved(conn, id, resolved, now) setting resolved = ?.
  • crates/hero_whiteboard_server/openrpc.json — document the new fields on comment.create / comment.update / comment.resolve.
  • crates/hero_whiteboard_ui/static/web/js/whiteboard/tools.js — eraser walks .comment-marker; transformer transformend routes comment markers to WhiteboardComments.onTransformEnd.
  • crates/hero_whiteboard_ui/static/web/js/whiteboard/comments.js — right-click guard, Unresolve support, load/save rotation + scale, onTransformEnd helper, unresolveComment export.
  • crates/hero_whiteboard_ui/static/web/js/whiteboard/properties.js — detect comment-marker in show() and render a small pane (Resolved badge, Resolve/Unresolve button, Delete button).

Implementation Plan

Step 1: Schema + server (rotation, scale, unresolve)

Files: migration, models, queries, handler, openrpc.json

  • Add migration 005_comment_transform.sql adding rotation REAL NOT NULL DEFAULT 0 and scale REAL NOT NULL DEFAULT 1 to the comments table.
  • Extend Comment struct; SELECT/INSERT/UPDATE in queries.rs include both columns.
  • comment.update handler reads optional rotation and scale from params and passes them through. Clamp scale server-side to [0.2, 5.0] as a safety net.
  • Rename resolve_comment → set_comment_resolved(conn, id, resolved, now); comment.resolve handler reads params["resolved"].as_bool().unwrap_or(true) and calls the new helper.
  • Update openrpc.json to document the new fields and the resolved param on comment.resolve.

Dependencies: none.

Step 2: Client-side rotation + scale persistence

Files: comments.js, tools.js

  • In addCommentMarker, apply comment.rotation || 0 and comment.scale || 1 to the group before adding to the layer.
  • Extend the dragend handler to include rotation and scale in the comment.update payload (they may not have changed, but sending them is idempotent).
  • Add a new helper WhiteboardComments.onTransformEnd(group) that reads group.rotation() and averages scaleX/scaleY, clamps to [0.2, 5], bakes the uniform scale back onto the group (so the next transform is clean), updates _comments[id].data, sends the RPC, and broadcasts.
  • In handleSyncMessage, add a comment.updated branch that also reads rotation and scale and applies them to the marker.
  • In tools.js transformend, iterate nodes; for each node.hasName('comment-marker') call WhiteboardComments.onTransformEnd(node) instead of WhiteboardObjects.applyTransform(node).

Dependencies: Step 1 (needs the new RPC fields).

Step 3: Right-click guard

Files: comments.js

  • In addCommentMarker, the click tap handler short-circuits when e.evt && e.evt.button !== 0. The native contextmenu handler in contextmenu.js already shows the canvas menu (comment markers are not .object ancestors), so no further change is needed there.

Dependencies: none.

Step 4: Unresolve UI

Files: comments.js

  • Export unresolveComment(id) which calls rpcCall('comment.resolve', { id: id, resolved: false }), flips entry.data.resolved = false, updates marker appearance, broadcasts comment.unresolved, and re-renders the popover.
  • In showCommentPopover, render an Unresolve button (class comment-btn unresolve) when comment.resolved, wired to WhiteboardComments.unresolveComment.
  • Handle comment.unresolved in handleSyncMessage (mirror of the resolved branch).

Dependencies: Step 1 (needs server to accept resolved: false).

Step 5: Eraser deletes comments

Files: tools.js

  • In eraseAtPosition, after the object + connector loops, walk layer.find('.comment-marker'); for each that intersects the eraser box, extract numericId = parseInt(node.id().replace('comment-', ''), 10) and call WhiteboardComments.deleteComment(numericId).

Dependencies: none.

Step 6: Properties panel for comments

Files: properties.js

  • In show(node), before the regular type dispatch, detect node.hasName && node.hasName('comment-marker') and render a minimal pane:
    • Header "Comment"
    • Position (X/Y read-only)
    • Resolved badge (Active/Resolved)
    • Buttons: Resolve / Unresolve (depending on current state) + Delete (red), wired to WhiteboardComments.resolveComment / unresolveComment / deleteComment.
  • Extract numeric id the same way the eraser does.

Dependencies: Step 4 (unresolveComment export).

Acceptance Criteria

  • Eraser drag over a comment marker deletes it from the server and removes it from the canvas.
  • Selecting a single comment marker shows a Comment pane in the sidebar with Resolve/Unresolve and Delete buttons; both buttons perform the expected action and refresh UI.
  • Resolved comments show an Unresolve button in the popover; clicking flips the state on the server and in all connected clients.
  • Right-clicking a comment marker opens the native canvas context menu only; the comment popover does not open.
  • Rotating a comment via the transformer survives a full reload.
  • Resizing a comment via the transformer survives a full reload; the uniform scale is clamped to [0.2, 5].
  • No regression in: creating a new comment, replying, deleting from the popover, resolve, comment-marker drag, multi-select + group drag.
  • All migrations apply cleanly to an existing DB; comments created before the migration read back with rotation=0, scale=1.

Notes

  • Scale is kept uniform (single scale column) because the pin is a circle; non-uniform deformation has no useful semantics.
  • The choice to extend comment.resolve with a resolved param (instead of adding comment.unresolve) keeps the method count flat and matches how most systems model boolean flips.
  • The popover double-position fix, rubber-band inclusion, and Esc-to-close fixes from the earlier PR (#35) stay untouched; this spec builds on top of them.
## Consolidated Spec for Issues #36-#41 ### Objective Make comments feel like first-class board objects: erasable, deletable from the sidebar, toggleable between active/resolved, non-reactive to the right mouse button, and with their rotation and scale surviving a reload. ### Scope at a glance | # | Fix | Surface | |---|---|---| | 36 | Eraser also deletes comments | `tools.js` | | 37 | Sidebar shows comment pane with Delete / Resolve-Unresolve | `properties.js` | | 38 | Server supports un-resolve; popover renders Unresolve button | `server/*`, `comments.js` | | 39 | Right-click no longer opens the comment popover | `comments.js` | | 40 | Comment rotation persists through server | schema + RPC + `comments.js` | | 41 | Comment scale persists through server | schema + RPC + `comments.js` | ### Files to Modify / Create - `crates/hero_whiteboard_server/src/migrations/005_comment_transform.sql` (NEW) — adds `rotation REAL NOT NULL DEFAULT 0` and `scale REAL NOT NULL DEFAULT 1` columns to `comments`. - `crates/hero_whiteboard_server/src/db/models.rs` — extend the `Comment` struct with `rotation` and `scale`. - `crates/hero_whiteboard_server/src/db/queries.rs` — include both columns in SELECT / INSERT / UPDATE for comments. - `crates/hero_whiteboard_server/src/handlers/comment.rs` — `create` accepts optional `rotation` / `scale`; `update` accepts `rotation` / `scale`; new `set_resolved` accepts a `resolved` bool (reuses `comment.resolve` method name with the new field to stay backwards-compatible). - `crates/hero_whiteboard_server/src/db/queries.rs` — rename `resolve_comment` to `set_comment_resolved(conn, id, resolved, now)` setting `resolved = ?`. - `crates/hero_whiteboard_server/openrpc.json` — document the new fields on `comment.create` / `comment.update` / `comment.resolve`. - `crates/hero_whiteboard_ui/static/web/js/whiteboard/tools.js` — eraser walks `.comment-marker`; transformer `transformend` routes comment markers to `WhiteboardComments.onTransformEnd`. - `crates/hero_whiteboard_ui/static/web/js/whiteboard/comments.js` — right-click guard, Unresolve support, load/save rotation + scale, `onTransformEnd` helper, `unresolveComment` export. - `crates/hero_whiteboard_ui/static/web/js/whiteboard/properties.js` — detect `comment-marker` in `show()` and render a small pane (Resolved badge, Resolve/Unresolve button, Delete button). ### Implementation Plan #### Step 1: Schema + server (rotation, scale, unresolve) Files: migration, models, queries, handler, openrpc.json - Add migration `005_comment_transform.sql` adding `rotation REAL NOT NULL DEFAULT 0` and `scale REAL NOT NULL DEFAULT 1` to the `comments` table. - Extend `Comment` struct; SELECT/INSERT/UPDATE in `queries.rs` include both columns. - `comment.update` handler reads optional `rotation` and `scale` from params and passes them through. Clamp scale server-side to `[0.2, 5.0]` as a safety net. - Rename `resolve_comment` → `set_comment_resolved(conn, id, resolved, now)`; `comment.resolve` handler reads `params["resolved"].as_bool().unwrap_or(true)` and calls the new helper. - Update openrpc.json to document the new fields and the `resolved` param on `comment.resolve`. Dependencies: none. #### Step 2: Client-side rotation + scale persistence Files: `comments.js`, `tools.js` - In `addCommentMarker`, apply `comment.rotation || 0` and `comment.scale || 1` to the group before adding to the layer. - Extend the `dragend` handler to include `rotation` and `scale` in the `comment.update` payload (they may not have changed, but sending them is idempotent). - Add a new helper `WhiteboardComments.onTransformEnd(group)` that reads `group.rotation()` and averages `scaleX/scaleY`, clamps to `[0.2, 5]`, bakes the uniform scale back onto the group (so the next transform is clean), updates `_comments[id].data`, sends the RPC, and broadcasts. - In `handleSyncMessage`, add a `comment.updated` branch that also reads `rotation` and `scale` and applies them to the marker. - In `tools.js` `transformend`, iterate nodes; for each `node.hasName('comment-marker')` call `WhiteboardComments.onTransformEnd(node)` instead of `WhiteboardObjects.applyTransform(node)`. Dependencies: Step 1 (needs the new RPC fields). #### Step 3: Right-click guard Files: `comments.js` - In `addCommentMarker`, the `click tap` handler short-circuits when `e.evt && e.evt.button !== 0`. The native `contextmenu` handler in `contextmenu.js` already shows the canvas menu (comment markers are not `.object` ancestors), so no further change is needed there. Dependencies: none. #### Step 4: Unresolve UI Files: `comments.js` - Export `unresolveComment(id)` which calls `rpcCall('comment.resolve', { id: id, resolved: false })`, flips `entry.data.resolved = false`, updates marker appearance, broadcasts `comment.unresolved`, and re-renders the popover. - In `showCommentPopover`, render an Unresolve button (class `comment-btn unresolve`) when `comment.resolved`, wired to `WhiteboardComments.unresolveComment`. - Handle `comment.unresolved` in `handleSyncMessage` (mirror of the resolved branch). Dependencies: Step 1 (needs server to accept `resolved: false`). #### Step 5: Eraser deletes comments Files: `tools.js` - In `eraseAtPosition`, after the object + connector loops, walk `layer.find('.comment-marker')`; for each that intersects the eraser box, extract `numericId = parseInt(node.id().replace('comment-', ''), 10)` and call `WhiteboardComments.deleteComment(numericId)`. Dependencies: none. #### Step 6: Properties panel for comments Files: `properties.js` - In `show(node)`, before the regular type dispatch, detect `node.hasName && node.hasName('comment-marker')` and render a minimal pane: - Header "Comment" - Position (X/Y read-only) - Resolved badge (Active/Resolved) - Buttons: Resolve / Unresolve (depending on current state) + Delete (red), wired to `WhiteboardComments.resolveComment` / `unresolveComment` / `deleteComment`. - Extract numeric id the same way the eraser does. Dependencies: Step 4 (`unresolveComment` export). ### Acceptance Criteria - [ ] Eraser drag over a comment marker deletes it from the server and removes it from the canvas. - [ ] Selecting a single comment marker shows a Comment pane in the sidebar with Resolve/Unresolve and Delete buttons; both buttons perform the expected action and refresh UI. - [ ] Resolved comments show an Unresolve button in the popover; clicking flips the state on the server and in all connected clients. - [ ] Right-clicking a comment marker opens the native canvas context menu only; the comment popover does not open. - [ ] Rotating a comment via the transformer survives a full reload. - [ ] Resizing a comment via the transformer survives a full reload; the uniform scale is clamped to `[0.2, 5]`. - [ ] No regression in: creating a new comment, replying, deleting from the popover, resolve, comment-marker drag, multi-select + group drag. - [ ] All migrations apply cleanly to an existing DB; comments created before the migration read back with rotation=0, scale=1. ### Notes - Scale is kept uniform (single `scale` column) because the pin is a circle; non-uniform deformation has no useful semantics. - The choice to extend `comment.resolve` with a `resolved` param (instead of adding `comment.unresolve`) keeps the method count flat and matches how most systems model boolean flips. - The popover double-position fix, rubber-band inclusion, and Esc-to-close fixes from the earlier PR (#35) stay untouched; this spec builds on top of them.
AhmedHanafy725 commented 2026-04-21 15:03:55 +00:00
Author
Member
Copy link

Pull request opened: #42

This PR implements the consolidated spec (posted above) covering all six comment fixes.

CI

  • cargo check --workspace: pass
  • cargo clippy --workspace -- -D warnings: pass
  • cargo fmt --check: pass
Pull request opened: https://forge.ourworld.tf/lhumina_code/hero_whiteboard/pulls/42 This PR implements the consolidated spec (posted above) covering all six comment fixes. ### CI - `cargo check --workspace`: pass - `cargo clippy --workspace -- -D warnings`: pass - `cargo fmt --check`: pass
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
development
development_16
development_5
development_mik2
main
No results found.
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
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#40
No description provided.