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

Comment: scale not persisted across reload #41

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

Resizing a comment marker via the transformer (same surface as rotation above) visually scales the pin but the scale is discarded on reload.

Root cause: same pipeline as the rotation bug. applyTransform skips comment markers; the RPC has no scale field; the table has no scale column.

Fix: add scale_x and scale_y columns (or a single scale if we only allow uniform scaling — preferred), accept in comment.update, send on transformend, apply on load.

Resizing a comment marker via the transformer (same surface as rotation above) visually scales the pin but the scale is discarded on reload. Root cause: same pipeline as the rotation bug. `applyTransform` skips comment markers; the RPC has no scale field; the table has no scale column. Fix: add `scale_x` and `scale_y` columns (or a single `scale` if we only allow uniform scaling — preferred), accept in `comment.update`, send on transformend, apply on load.
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#41
No description provided.