Comments: resolving a comment shows live in the presentation view, but un-resolving does show until the viewer reloads #111

Open
opened 2026-04-29 16:28:26 +00:00 by eslamnawara · 3 comments
Member

Summary

The presentation view does not handle comment-state changes instantly:

  • Resolve shows immediately — when a comment is resolved in the editor, changes in the presentation view immediately.
  • Un-resolve does not show immediately — when the same comment is un-resolved in the editor, the presentation view continues to render it as resolved. The active state only returns after the presentation tab is manually reloaded.

Steps to reproduce

  1. Open a board in tab A (editor — owner URL).
  2. Open the same board in tab B in presentation mode (or via the shared link in presentation view).
  3. In tab A, place a comment on an object so its marker is visible in tab B.
  4. In tab A, resolve the comment. Observe tab B — the marker greys out immediately.
  5. In tab A, un-resolve the same comment. Observe tab B.

Expected

Tab B re-renders the comment marker as active immediately on un-resolve, just like it re-rendered on resolve.

Actual

Tab B keeps showing the marker as resolved (greyed out). The active state only appears after a manual reload of tab B.
image

## Summary The presentation view does not handle comment-state changes instantly: - **Resolve** shows immediately — when a comment is resolved in the editor, changes in the presentation view immediately. - **Un-resolve** does not show immediately — when the same comment is un-resolved in the editor, the presentation view continues to render it as resolved. The active state only returns after the presentation tab is manually reloaded. ## Steps to reproduce 1. Open a board in tab A (editor — owner URL). 2. Open the same board in tab B in presentation mode (or via the shared link in presentation view). 3. In tab A, place a comment on an object so its marker is visible in tab B. 4. In tab A, resolve the comment. Observe tab B — the marker greys out immediately. 5. In tab A, un-resolve the same comment. Observe tab B. ## Expected Tab B re-renders the comment marker as active immediately on un-resolve, just like it re-rendered on resolve. ## Actual Tab B keeps showing the marker as resolved (greyed out). The active state only appears after a manual reload of tab B. ![image](/attachments/0538a1cf-a3f1-4370-a412-31729fd65008)
Member

Implementation Spec for Issue #111

Objective

Comment un-resolve must apply live to the presentation/viewer tab the same way comment resolve does. Right now, un-resolving a comment in the editor leaves the marker stuck in the resolved (grey) state in any other tab until that tab is reloaded.

Root cause

crates/hero_whiteboard_ui/static/web/js/whiteboard/comments.js::setResolved broadcasts comment.resolved or comment.unresolved over the WebSocket. comments.js::handleSyncMessage already handles both types correctly. But the central WebSocket router in sync.js::handleWsMessage only forwards four comment types to the comments handler:

} else if (msg.type === 'comment.created' || msg.type === 'comment.updated'
        || msg.type === 'comment.resolved' || msg.type === 'comment.deleted') {
    WhiteboardComments.handleSyncMessage(msg);
}

comment.unresolved is missing from that list, so the message is silently dropped. That's why resolve syncs live but un-resolve doesn't.

Requirements

  • Un-resolving a comment in the editor immediately re-renders the marker as active in every other open tab (presentation, shared link, side-by-side editor) with no reload required.
  • No regression for resolve / create / update / delete sync.

Files to Modify

  • crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js — extend the comment-message router to include comment.unresolved.

Implementation Plan

Step 1: Route comment.unresolved to the comments handler

File: crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js

Change the dispatch arm in handleWsMessage:

} else if (msg.type === 'comment.created' || msg.type === 'comment.updated'
        || msg.type === 'comment.resolved' || msg.type === 'comment.unresolved'
        || msg.type === 'comment.deleted') {
    WhiteboardComments.handleSyncMessage(msg);
}

No changes needed in comments.jshandleSyncMessage already branches on comment.resolved || comment.unresolved and updates the marker state and appearance.

Dependencies: none.

Acceptance Criteria

  • Editor resolves a comment → all other tabs grey out the marker live (unchanged).
  • Editor un-resolves a comment → all other tabs re-show the marker as active live, no reload.
  • Affects shared editor view, viewer-only shared view, and tabs in presentation mode equally.
  • cargo fmt, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace --lib clean.

Notes

  • One-line surgical fix. The receiving handler was already wired for the unresolve case; only the router was filtering it out.
  • The localUserId echo-skip in handleWsMessage continues to ensure the originating editor tab doesn't double-apply its own change.
## Implementation Spec for Issue #111 ### Objective Comment un-resolve must apply live to the presentation/viewer tab the same way comment resolve does. Right now, un-resolving a comment in the editor leaves the marker stuck in the resolved (grey) state in any other tab until that tab is reloaded. ### Root cause `crates/hero_whiteboard_ui/static/web/js/whiteboard/comments.js::setResolved` broadcasts `comment.resolved` or `comment.unresolved` over the WebSocket. `comments.js::handleSyncMessage` already handles both types correctly. But the central WebSocket router in `sync.js::handleWsMessage` only forwards four comment types to the comments handler: ```js } else if (msg.type === 'comment.created' || msg.type === 'comment.updated' || msg.type === 'comment.resolved' || msg.type === 'comment.deleted') { WhiteboardComments.handleSyncMessage(msg); } ``` `comment.unresolved` is missing from that list, so the message is silently dropped. That's why resolve syncs live but un-resolve doesn't. ### Requirements - Un-resolving a comment in the editor immediately re-renders the marker as active in every other open tab (presentation, shared link, side-by-side editor) with no reload required. - No regression for resolve / create / update / delete sync. ### Files to Modify - `crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js` — extend the comment-message router to include `comment.unresolved`. ### Implementation Plan #### Step 1: Route `comment.unresolved` to the comments handler File: `crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js` Change the dispatch arm in `handleWsMessage`: ```js } else if (msg.type === 'comment.created' || msg.type === 'comment.updated' || msg.type === 'comment.resolved' || msg.type === 'comment.unresolved' || msg.type === 'comment.deleted') { WhiteboardComments.handleSyncMessage(msg); } ``` No changes needed in `comments.js` — `handleSyncMessage` already branches on `comment.resolved || comment.unresolved` and updates the marker state and appearance. Dependencies: none. ### Acceptance Criteria - [ ] Editor resolves a comment → all other tabs grey out the marker live (unchanged). - [ ] Editor un-resolves a comment → all other tabs re-show the marker as active live, no reload. - [ ] Affects shared editor view, viewer-only shared view, and tabs in presentation mode equally. - [ ] `cargo fmt`, `cargo clippy --workspace --all-targets -- -D warnings`, `cargo test --workspace --lib` clean. ### Notes - One-line surgical fix. The receiving handler was already wired for the unresolve case; only the router was filtering it out. - The `localUserId` echo-skip in `handleWsMessage` continues to ensure the originating editor tab doesn't double-apply its own change.
Member

Test Results

  • cargo fmt --all -- --check — clean
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • cargo test --workspace --lib — 0 failed
  • node --check sync.js — clean
## Test Results - `cargo fmt --all -- --check` — clean - `cargo clippy --workspace --all-targets -- -D warnings` — clean - `cargo test --workspace --lib` — 0 failed - `node --check sync.js` — clean
Member

Implementation Summary

One-line surgical fix: route comment.unresolved from the WebSocket through to the comments handler.

Root cause

comments.js::setResolved broadcasts either comment.resolved or comment.unresolved over the WebSocket, and comments.js::handleSyncMessage already branches on both correctly. But the central router in sync.js::handleWsMessage only forwarded four comment types (created / updated / resolved / deleted) to the comments handler — comment.unresolved was silently dropped, so other tabs never re-rendered the marker as active.

crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js

Added comment.unresolved to the dispatch arm:

} else if (msg.type === 'comment.created' || msg.type === 'comment.updated'
        || msg.type === 'comment.resolved' || msg.type === 'comment.unresolved'
        || msg.type === 'comment.deleted') {
    WhiteboardComments.handleSyncMessage(msg);
}

No changes needed elsewhere. The receiving handler in comments.js:593-599 was already prepared:

} else if (msg.type === 'comment.resolved' || msg.type === 'comment.unresolved') {
    var entry = _comments[msg.data.id];
    if (entry) {
        entry.data.resolved = (msg.type === 'comment.resolved');
        updateMarkerAppearance(entry);
        WhiteboardCanvas.getObjectLayer().batchDraw();
    }
}

Files Changed

  • crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js+1/-1

Test Results

  • cargo fmt --all -- --check — clean
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • cargo test --workspace --lib — 0 failed
  • node --check sync.js — clean

Manual smoke

  1. Open a board in editor tab A and a presentation/shared tab B.
  2. Place a comment in A — marker appears in B.
  3. Resolve the comment in A — marker greys out in B (unchanged).
  4. Un-resolve the same comment in A — marker now turns active in B immediately, no reload needed.

Notes

  • The localUserId echo-skip earlier in handleWsMessage continues to ensure the originating editor tab doesn't double-apply its own change.
  • Affects all secondary views equally: shared editor view, viewer-only shared view, and presentation tabs.
## Implementation Summary One-line surgical fix: route `comment.unresolved` from the WebSocket through to the comments handler. ### Root cause `comments.js::setResolved` broadcasts either `comment.resolved` or `comment.unresolved` over the WebSocket, and `comments.js::handleSyncMessage` already branches on both correctly. But the central router in `sync.js::handleWsMessage` only forwarded four comment types (`created / updated / resolved / deleted`) to the comments handler — `comment.unresolved` was silently dropped, so other tabs never re-rendered the marker as active. ### `crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js` Added `comment.unresolved` to the dispatch arm: ```js } else if (msg.type === 'comment.created' || msg.type === 'comment.updated' || msg.type === 'comment.resolved' || msg.type === 'comment.unresolved' || msg.type === 'comment.deleted') { WhiteboardComments.handleSyncMessage(msg); } ``` No changes needed elsewhere. The receiving handler in `comments.js:593-599` was already prepared: ```js } else if (msg.type === 'comment.resolved' || msg.type === 'comment.unresolved') { var entry = _comments[msg.data.id]; if (entry) { entry.data.resolved = (msg.type === 'comment.resolved'); updateMarkerAppearance(entry); WhiteboardCanvas.getObjectLayer().batchDraw(); } } ``` ### Files Changed - `crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js` — `+1/-1` ### Test Results - `cargo fmt --all -- --check` — clean - `cargo clippy --workspace --all-targets -- -D warnings` — clean - `cargo test --workspace --lib` — 0 failed - `node --check sync.js` — clean ### Manual smoke 1. Open a board in editor tab A and a presentation/shared tab B. 2. Place a comment in A — marker appears in B. 3. Resolve the comment in A — marker greys out in B (unchanged). 4. Un-resolve the same comment in A — marker now turns active in B immediately, no reload needed. ### Notes - The `localUserId` echo-skip earlier in `handleWsMessage` continues to ensure the originating editor tab doesn't double-apply its own change. - Affects all secondary views equally: shared editor view, viewer-only shared view, and presentation tabs.
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
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#111
No description provided.