Whiteboard: laser dot/trail fix, object snapping, letter hotkeys, per-user close#200
Whiteboard: laser dot/trail fix, object snapping, letter hotkeys, per-user close#200pj4real wants to merge 3 commits into
Conversation
…dd object snapping - Laser pointer now tracks the cursor as a plain dot while hovering and only paints a fading trail while the button is held; releasing lets the trail decay on its own instead of vanishing instantly. The trail is also rendered as a handful of smoothed, independently-faded bands instead of one straight segment per point pair. - Add Figma-style alignment guides: dragging or resizing a shape now snaps to nearby elements' edges/centers and draws a guide line while the gesture is active. Closes ACM-VIT#190, ACM-VIT#191
Each tool button now shows its keyboard shortcut as a small persistent letter badge (Excalidraw-style) instead of only a hover tooltip.
…or labels - Tool hotkeys are now letter mnemonics (V/H/P/E/R/O/L/A/T/S/K) with 1-9 kept as a secondary alias in the original order. - Add a corner close control on the whiteboard: everyone can hide it just for themselves (with a way to bring it back while it's still running), and admins additionally get a "close for everyone" option. This required a new local-only isHiddenForMe/hideForMe/showForMe state on AppsProvider, a "Show whiteboard" control (not admin-gated) in the overflow menu, and fixing the Apps panel so it lets anyone rejoin an app they hid instead of silently closing it for everyone. - Remote cursor labels now show first name only, to stay compact next to the pointer.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| this.snapGuides = adjustment.guides; | ||
| const next = translateElement(element, dx + adjustment.dx, dy + adjustment.dy); |
There was a problem hiding this comment.
After an element snaps, the next candidate starts from that already-snapped position but uses only the latest raw pointer delta. Moving away by less than the threshold per frame therefore snaps the element back on every move, so it remains stuck until one pointer event crosses the full threshold. The candidate position needs to come from an unsnapped drag origin or account for the previous snap offset.
Artifacts
Repro: executable TypeScript harness using the real whiteboard tool engine and Yjs document helpers
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
- Keeps the command output available without making the summary code-heavy.
| if (tool === "select") { | ||
| snapGuidesRef.current = engineRef.current?.getSnapGuides() ?? []; | ||
| paintSnapGuides(); | ||
| } |
There was a problem hiding this comment.
A pointercancel during a snapped move or resize never reaches the new pointer-up cleanup. Touch interruption or lost pointer capture can therefore leave the guide overlay visible and the engine or resize session active, causing the next gesture to continue stale state instead of starting cleanly.
theg1239
left a comment
theg1239
left a comment
There was a problem hiding this comment.
Review: laser dot/trail, object snapping, letter hotkeys, per-user close
Nice PR. The scope is well chosen, the code is clean and commented, and several details are done carefully: the laser wire format stays compatible in both directions (old clients parsing only pts still render trails from new senders, and new clients handle old senders with no dot), the new hotkeys inherit the existing editable-target and modifier guards plus event.code handling for non-QWERTY layouts, and the close-for-me state is local-only with a sensible reset when the active app changes. The AppsPanel fix (non-admins were fully locked out, and an admin rejoining would have closed the app for everyone) is a real catch.
I verified everything below against the PR head (ada6488). One issue should block merge; the rest are quick.
Must fix
1. Snapped elements get stuck: slow drags can never detach
packages/apps-sdk/src/apps/whiteboard/core/tools/engine.ts:277-299
Each move computes prospective = currentElementPosition + rawPerFrameDelta. Once the element has snapped, currentElementPosition is the snapped position, so the candidate is anchored to the snap target rather than to the cursor. If every per-frame delta stays under the threshold (8 screen px / scale), the element re-snaps on every frame and never escapes, no matter how far the cursor has actually traveled.
Because the move path is rAF-coalesced, per-frame deltas are per-16.7ms deltas: at 60 Hz you need a sustained drag faster than roughly 480 px/s to break out. Fast flicks escape; slow, precise drags (exactly the case where a user is trying to place something a few pixels away from another element) stay pinned forever, and the cursor-to-element offset grows the whole time, so when it finally escapes it jumps.
Suggested fix: at drag start, store the grabbed element's start bounds next to the existing dragStart pointer origin. On each move, compute the cumulative cursor delta from dragStart, derive the candidate from startBounds + cumulativeDelta, snap that, and write the result. That makes detachment follow the true cursor position and eliminates the drift by construction, which also makes the "keep lastPoint raw" workaround comment unnecessary.
Should fix
2. No onPointerCancel handler, so a cancelled gesture strands live state
packages/apps-sdk/src/apps/whiteboard/web/components/WhiteboardCanvas.tsx:1640-1643
The canvas wires onPointerDown/Move/Up/Leave and uses pointer capture, so pointercancel (touch interrupted by a system gesture, palm rejection, capture loss) is the one terminal event with no handler. handlePointerMove checks resizeSessionRef.current unconditionally, so after a cancel the next hover continues resizing with no button held, and the new snap-guide overlay stays painted. The stale-session half predates this PR, but the PR touches exactly this code and adds the visible symptom; onPointerCancel={handlePointerUp} covers every path (pan, laser, rotate, resize, engine) since handlePointerLeave already delegates the same way.
3. Digit hotkeys 8/9 silently changed meaning, and sticky lost its binding
WhiteboardWebApp.tsx keymaps: previously 8 = text and 9 = sticky; now 8 = arrow, 9 = text, and sticky has no digit (or numpad) binding at all. The PR description and the code comment both say numbers 1-9 are kept "in the original order", which reads as unchanged, but two of nine digits were remapped and one tool dropped out. Either keep 8/9 as they were and let arrow stay letter-only, or update the comment and PR text to say the aliases follow toolbar order and note that sticky is now S only.
Recommended
- Exclude freehand strokes from snap candidates, or cache bounds. Every move frame maps all other elements through
getBoundsForElement, which is O(points) per stroke with four array allocations and spread intoMath.min/max(which can throw RangeError on very long strokes; pre-existing, but this puts it on the hottest path). On a busy board this is O(total stroke points) per frame during drag and resize. Snapping to a scribble's invisible bounding box also feels random compared to snapping to shapes, stickies, and text. Filtering candidates toshape/sticky/textfixes both at once; a per-element bounds cache is a further option. - Clear the guide overlay when the tool changes.
engine.setToolclears its guide array, but nothing repaints the overlay: switch tools via keyboard mid-drag and the last guide lines stay on screen until some later select-move or resize repaints. A small effect ontoolthat emptiessnapGuidesRefand callspaintSnapGuides()covers it. - rAF-coalesce the resize path. Element moves go through
queuePointerMove, but the resize branch inhandlePointerMoveruns per raw pointer event (up to 240 Hz on some devices), each time doinggetPageElementsplus a full bounds map plus an overlay repaint. Reusing the same coalescing as moves keeps the cost bounded. - Consider a hold-to-disable-snapping modifier. Figma and Excalidraw use Cmd/Ctrl while dragging to bypass snapping; since the threshold is fixed, precise placement near another element otherwise fights the magnet. Pairs naturally with the fix for issue 1.
Minor
- The laser dot vanishes abruptly when its 500 ms TTL expires (
paintLaserFramedrops it and draws at full alpha until then). A short fade over the last ~150 ms would match the trail's behavior. Purely polish. - When min-size clamping engages during a resize, the snapped edge can miss the target while the guide still draws as if aligned, since the snap offset is applied to the pointer and then re-clamped. Edge case, fine to leave.
- Snapping and guides use axis-aligned bounds, so rotated elements snap on their unrotated box and guides may not visually touch them. Worth a code comment as a known limitation.
- Two people with the same first name now render identical cursor labels. Deliberate tradeoff for compactness, just noting it.
Tests
snapping.ts is a pure, well-documented module and an ideal first unit-test target for apps-sdk (the package has no test setup today, but meeting-core already has a vitest precedent in this repo). A table test over edge/center matches, threshold boundaries, and the per-axis independence would have caught nothing here, but the issue-1 fix (cumulative-delta candidate) is exactly the kind of logic a small engine test protects going forward.
Summary
Fix issue 1 before merge; 2 and 3 are small and worth doing in the same pass. Everything else can follow. The feature set itself is in good shape, and the manual two-session test plan in the description is appreciated.
🤖 Generated with Claude Code
2 participants
Summary
Vselect,Hpan,Ppen,Eeraser,Rrect,Oellipse,Lline,Aarrow,Ttext,Ssticky,Klaser), with1-9kept as a secondary alias in the original order. Every toolbar button now shows its shortcut as a small persistent badge instead of only a hover tooltip.closeApp()(closing for everyone) instead of un-hiding it locally.Test plan
pnpm typecheckpasses forapps-sdkandapps/webspent a lot of water and tokens to review your slop
Greptile Summary
This PR expands the collaborative whiteboard experience. The main changes are:
Confidence Score: 4/5
Incremental object dragging can remain stuck on a snap target and should be fixed before merging.
Snapping starts each move from the element's already-snapped position. Small pointer movements can repeatedly snap back to the same target. Pointer cancellation can leave guides and gesture state active. No security issue was identified in the changed close controls.
packages/apps-sdk/src/apps/whiteboard/core/tools/engine.ts and packages/apps-sdk/src/apps/whiteboard/web/components/WhiteboardCanvas.tsx
What T-Rex did
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR Pointer[Pointer move] --> Canvas[Whiteboard canvas] Canvas --> Engine[Tool engine] Engine --> Bounds[Prospective bounds] Bounds --> Snap[Snap adjustment] Snap --> Document[Yjs element update] Snap --> Guides[Guide overlay] LocalClose[Close for me] --> Hidden[Local hidden state] Hidden --> Layout[Meeting app visibility] SharedClose[Close for everyone] --> Server[Shared app close]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart LR Pointer[Pointer move] --> Canvas[Whiteboard canvas] Canvas --> Engine[Tool engine] Engine --> Bounds[Prospective bounds] Bounds --> Snap[Snap adjustment] Snap --> Document[Yjs element update] Snap --> Guides[Guide overlay] LocalClose[Close for me] --> Hidden[Local hidden state] Hidden --> Layout[Meeting app visibility] SharedClose[Close for everyone] --> Server[Shared app close]Reviews (1): Last reviewed commit: "feat(whiteboard): remap tool hotkeys, pe..." | Re-trigger Greptile