feat(comments): show selection action after highlight, calmer styling

[puemos]

Problem

Anyone highlighting text in a desktop task's comments gets a Comment action that chases the mouse cursor mid-drag, appears nowhere near where the selection actually ends, gives no visible hover feedback, and looks louder than the text it sits on.

Five specifics:

• The action tracked the live selection during a drag, so it moved while the user was still highlighting.
• Each surface anchored the action differently, and none matched the selection's end: the code editor anchored to the start of the selection's last line (the button appeared far from the release point), text artifacts and canvases anchored to the whole-range bounding box, and the HTML-artifact bridge centered above the range.
• Hover produced no visible feedback: the desktop Quill button keeps cursor: default, and its hover background differed from rest by only a few percent. The in-iframe light theme's hover was #f5f5f4 on white — invisible.
• The action rendered in the heavy primary style: filled accent overlay button, saturated yellow in-iframe button.
• The labeled Comment button carried a hover tooltip saying the same thing the button already says.
• Published canvases kept both bugs after the first round of fixes. They run a second selection runtime, baked into the artifact by the cloud builder, that no desktop change reaches.

Changes

• One shared gate, installSelectionSettleGate (new selectionCommentAction.ts), shows the action only once the selection settles, following the pattern the established editor toolbars use (Floating UI's text-selection example, TipTap's BubbleMenu, the common selectstart/pointerup idiom): selectstart/pointerdown/selection keydown hide it, selectionchange is ignored mid-gesture, and pointerup/selection keyup report two animation frames later — browsers commit the selection after the pointerup handler runs, so reading it synchronously returns the mid-gesture range. pointercancel (which trackpads fire mid-drag) and window blur cancel. Secondary mouse buttons and plain typing don't start a gesture.
• All comment surfaces wire the same gate: artifact text annotations call it directly, and the artifact HTML bridge plus the canvas sandbox runtime inject it via .toString(), so the behavior has one implementation.
• One shared anchor, commentActionAnchorRect: a Range spanning block elements also reports its wrapper boxes (paragraph, blockquote, list), so neither the bounding box nor the last box marks where the selection ended. Keeping only leaf boxes and taking the lowest, right-most one gives the end of the last selected line.
• One shared placement, computeCommentActionPlacement: the action sits just right of the selection's end caret, vertically centered on the end line (Google Docs style). It flips below the end line when the right edge has no room, keeping its right edge at the caret, and flips above near the viewport bottom. Every anchor producer (CodeMirrorEditor, artifact text annotations, the artifact HTML bridge, the canvas runtime) now reports the selection's end-line rect.
• The action is its own component, SelectionCommentActionButton, rather than a Quill button. It renders the same stylesheet the sandboxed iframes inject, so the code editor, text artifacts, HTML artifacts and canvases cannot drift apart, and its palette is explicit per theme instead of inherited from ambient tokens.
• Hover is visible everywhere: pointer cursor and a real background shift. The shadow stays put — a shadow that jumped on hover read as a glitch.
• The canvas runtime stops hand-rolling its own button CSS and uses the shared theme/css/placement helpers like the artifact bridge.
• The labeled Comment button has no redundant tooltip; the icon-only "+" action keeps its tooltip, since it has no visible label.
• Theme follows the app: the artifact document bakes the mount-time theme, and live flips ride a theme bridge message, so a theme change never reloads the running preview.
• The cloud builder's baked runtime (products/canvas/packages/canvas_builder/build.mjs) installs the same gate and the same end-line anchor, so a published canvas behaves like every other surface.
• That code is a port, not an import: the builder runs inside its own build image and cannot reach the desktop workspace. Extracting one shared source crosses a product and an image boundary, so it is left for its own change. Both copies carry a comment naming the other.
• Both canvas runtimes drop a pending selection report when a new gesture starts. A drag begun inside the 80 ms debounce window would otherwise publish the previous selection mid-drag.

Note

Canvases built before this ships keep the old runtime until they are rebuilt. The runtime is baked into each artifact at build time, so republishing a canvas is what picks up the fix.

• Screenshots are not in this description: this runner's token cannot publish to the pr-assets repo (verified-signature ruleset). Before/after captures are attached to the originating task as artifacts.

How did you test this code?

• selectionCommentAction.test.ts: the gate ignores mid-drag selections, reports on pointer release and on keyboard-only selection, ignores presses inside the action UI, cancels on window blur, and settles on pointercancel; computeCommentActionPlacement covers right-of-end, flip-below, flip-above, and viewport-margin clamping. Guards the "button chases the cursor" and "button lands nowhere near the selection" regressions.
• artifactHtmlCommentBridge.test.ts: executes the generated bridge script in a real JSDOM document; the Comment button stays hidden through a simulated drag, appears on release at the expected end-line coordinates, and a theme message re-themes a running document. Guards template-string fumbles in the injected script itself.
• Tooltip coverage in SelectionCommentOverlay.test.tsx: the labeled Comment action opens no tooltip, while the icon-only action still does.
• test_cloud_builder.py drives the runtime the builder actually emits through a simulated drag in Node: nothing is reported while the pointer is down, one report lands on release, and its rect is the last selected line's box rather than the whole-range box. Run against the pre-fix runtime, both assertions fail. Guards the published-canvas path, which no desktop test can reach.
• The builder test drops two string pins that had frozen the old behavior: selectionchange present and mouseup absent. The drag test replaces them.
• Drove a real mouse drag across a multi-line selection in headless Chromium: the action is absent at six sampled points during the drag, and on release lands 8px right of the selection end with its vertical center within a pixel of the end line's.
• Pixel-diffed rest vs real hover in both themes: ~840 pixels change inside the button, zero elsewhere.

Automatic notifications

• Publish to changelog?

Docs update

None: desktop-app UI polish behind the posthog-code-comments feature flag.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

The gate, placement, and theme helpers are centralized in selectionCommentAction.ts so sandboxed iframe scripts can inject them via .toString(). The published-canvas gap surfaced in review of the first round: BuiltCanvas embeds a signed artifact whose runtime comes from the cloud builder, so the desktop fixes never applied there. Porting plus a behavioral test was chosen over a full extraction, which would have to cross into the canvas build image. Hover invisibility was verified against the actual CSS (Quill's cursor: default + near-identical --fill-hover delta; iframe hover #f5f5f4 on white) and against pixel diffs of rest-vs-hover renders.

[github-actions]

React Doctor found 8 issues in 2 files · 8 warnings.

8 warnings

packages/ui/src/features/code-editor/components/CodeMirrorEditor.tsx

• ⚠️ packages/ui/src/features/code-editor/components/CodeMirrorEditor.tsx:2 Heavy library loaded eagerly prefer-dynamic-import
• ⚠️ packages/ui/src/features/code-editor/components/CodeMirrorEditor.tsx:3 Heavy library loaded eagerly prefer-dynamic-import

packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx

• ⚠️ packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx:38 Non-component export in component file only-export-components
• ⚠️ packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx:51 Non-component export in component file only-export-components
• ⚠️ packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx:64 Non-component export in component file only-export-components
• ⚠️ packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx:219 State adjusted after a prop changes no-adjust-state-on-prop-change
• ⚠️ packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx:235 Data passed to parent via effect no-pass-data-to-parent
• ⚠️ packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx:372 Array index used as a key no-array-index-as-key

_{Reviewed by React Doctor for commit 70016bf.}

[trunk-io]

   

View Full Report ↗︎ ⋅ Docs

[trunk-io]

😎 Merged successfully - details.

[puemos]

Merge Confidence: 3.5/5 - Confident

"Looks good - minor concerns only"

Assessment

• ✓ Genuine consolidation: four ad-hoc selectionchange implementations collapse into one shared selectionCommentAction.ts, with the comment action's look shared between the app and sandboxed iframes via a single class + palette
• ✓ Strong test investment for a UI change — 15 unit tests over the gate/anchor/placement, a jsdom test that runs the real injected bridge script, and a Node harness driving the actual built canvas runtime through a simulated drag
• ✓ Security posture preserved: the new theme message sits inside the existing source/marker/channel guard and allowlists to light|dark; injected constants go through JSON.stringify; CSP and nonce handling untouched
• ⚠ Cmd+A leaves installSelectionSettleGate stuck in selecting when the modifier is released before the letter — onKeyUp re-evaluates isSelectionKey against the keyup event, so the gate swallows all selectionchange until the next pointerdown/pointerup. The comment action silently never appears for a select-all (selectionCommentAction.ts:258)
• ⚠ The test that would have caught it stops one event short: treats a plain letter as typing asserts the Cmd+A keydown but never dispatches the keyup (selectionCommentAction.test.ts:190)
• ⚠ onKeyDown starts a gesture on every arrow-key repeat including plain caret movement, and onGestureStart is wired to an unconditional cross-iframe text-selection-cleared post in sandboxRuntime.ts and the canvas runtime — ~30 messages/sec while an arrow key is held
• ⚠ build.mjs hand-copies the gate and anchor functions (the builder can't import the desktop workspace) and the copy already diverges — it drops the cleanup return. The only guard is a comment plus the Node harness, whose Element/MouseEvent stubs make every instanceof check unreachable, so the right-click and action-UI guards are untested there
• ⚠ computeCommentActionPlacement now positions the 180px expanded composer as well as the 28px action, centering it on the selection line instead of below it — a visible behavior change with no test on that branch
• ⚠ anchor.left now means the end-caret x and is passed into the placement function's right field; correct but reads as a transposition at five independent call sites

---

Review: Unify the selection comment action: shared settle gate, end-line anchoring, and one themed button across all surfaces

Replaces four independent selectionchange implementations (markdown annotations, HTML artifact bridge, canvas sandbox, published canvas runtime) with a shared selectionCommentAction.ts: a gesture settle gate so the action stops chasing the cursor mid-drag, a leaf-box anchor so it lands at the end of the last selected line instead of a wrapper's bounding box, and one button class + palette so the in-app and iframed actions can't drift. EditorSelection.anchor grows a bottom field and left is redefined as the end-caret x; a new theme message lets a running artifact iframe re-theme without reloading.

The consolidation is well-executed and unusually well-tested for a UI change — including a Node harness that drives the actually-built canvas runtime through a simulated drag. The main risk is that one shared selecting boolean now gates the action on every surface at once, so its unreachable transitions degrade all of them silently. Specifically, Cmd+A with the modifier released first leaves the gate stuck and the action never appears; the test suite stops one event short of catching it. Secondary concerns: arrow-key repeat drives a cross-iframe message per keydown, the canvas builder's hand-copied fork of the gate already diverges from the TS source with only a comment guarding sync, and the placement helper is now reused for the 180px composer without a test on that branch.

[puemos]

React Doctor follow-up: fixed the no-ref-current-in-render error in 44e0c57 by updating the callback ref from an effect. The eight warnings were already present at the PR base commit and are unchanged by this PR, so I left those unrelated refactors out of scope. A changed-scope React Doctor scan now reports zero errors.

[puemos]

/trunk merge

[puemos]

/trunk merge

[puemos]

/trunk merge

[deployment-status-posthog]

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-08-07 15:37 UTC	Run
prod-us	✅ Deployed	2026-08-07 15:53 UTC	Run
prod-eu	✅ Deployed	2026-08-07 15:55 UTC	Run