From 232c372dde0fd4917ddbc1e2829bdb714251fbce Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Fri, 7 Aug 2026 11:00:59 +0200 Subject: [PATCH 1/7] feat(comments): show selection action after highlight, calmer styling Generated-By: PostHog Code Task-Id: e93165cd-6893-45dd-8fa8-4e97299f5f06 --- .../canvas/freeform/sandboxRuntime.ts | 12 +- .../SelectionCommentOverlay.test.tsx | 37 ++++ .../components/SelectionCommentOverlay.tsx | 49 +++--- .../components/AnnotatedArtifactHtml.tsx | 22 ++- .../components/ArtifactPreview.test.tsx | 2 +- .../components/ArtifactTextAnnotations.tsx | 12 +- .../artifactHtmlCommentBridge.test.ts | 138 +++++++++++++++ .../components/artifactHtmlCommentBridge.ts | 34 +++- .../components/artifactPreviewDocument.ts | 16 +- .../components/selectionCommentAction.test.ts | 159 ++++++++++++++++++ .../components/selectionCommentAction.ts | 122 ++++++++++++++ 11 files changed, 565 insertions(+), 38 deletions(-) create mode 100644 products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts create mode 100644 products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.test.ts create mode 100644 products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts diff --git a/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts b/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts index 628faa1e8c5d..e193082cb832 100644 --- a/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts +++ b/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts @@ -6,6 +6,7 @@ import { FREEFORM_QUILL_CSS_URLS, } from "@posthog/core/canvas/freeformWhitelist"; import { resolveTextCommentAnchor } from "@posthog/core/comments/anchors"; +import { installSelectionSettleGate } from "@posthog/ui/features/sessions/components/selectionCommentAction"; // Builds the HTML document loaded into the freeform-canvas sandbox iframe. // @@ -329,7 +330,16 @@ export function buildSandboxDocument( }); }, 80); }; - document.addEventListener("selectionchange", reportTextSelection); + // Report the selection only once it settles, so the host's comment action + // doesn't chase the cursor mid-drag. The settle callback re-reads the live + // selection, which self-corrects clicks that didn't change the selection. + const selectionSettleGate = ${installSelectionSettleGate.toString()}; + selectionSettleGate(document, { + onGestureStart: clearTextSelection, + onSelectionSettled: reportTextSelection, + onIdleSelectionChange: reportTextSelection, + onGestureCancel: clearTextSelection, + }); const clearNativeTextSelection = () => { window.getSelection()?.removeAllRanges(); clearTextSelection(); diff --git a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx index b168517d738d..b0069e3367a3 100644 --- a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx +++ b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx @@ -25,6 +25,26 @@ vi.mock("@posthog/ui/features/canvas/components/MentionComposer", () => ({ import { SelectionCommentOverlay } from "./SelectionCommentOverlay"; +function renderCollapsed( + props: Partial[0]> = {}, +) { + return render( + , + ); +} + describe("SelectionCommentOverlay", () => { it("keeps the existing add-to-chat action available", () => { render( @@ -44,6 +64,23 @@ describe("SelectionCommentOverlay", () => { expect(screen.getByLabelText("Add to chat")).toBeInTheDocument(); }); + + it("shows no tooltip on the comment action, whose label is already visible", () => { + renderCollapsed({ actionLabel: "Add comment", showActionText: true }); + + fireEvent.focus(screen.getByLabelText("Add comment")); + + expect(screen.queryByRole("tooltip")).not.toBeInTheDocument(); + expect(screen.queryByText("Add comment")).not.toBeInTheDocument(); + }); + + it("keeps the tooltip on the icon-only action, which has no visible label", () => { + renderCollapsed(); + + fireEvent.focus(screen.getByLabelText("Add to chat")); + + expect(screen.getByRole("tooltip")).toHaveTextContent("Add to chat"); + }); it("prevents duplicate comment creation while submitting", async () => { let resolveSubmit: (() => void) | undefined; const onSubmit = vi.fn( diff --git a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx index ccd9e28386d9..bbfb54b3bbb7 100644 --- a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx +++ b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx @@ -138,28 +138,35 @@ function SelectionComposerCard({ }, [onDismiss]); if (!expanded) { + const action = ( + + ); + // The text button names itself; only the icon-only variant needs a label + // on hover. return createPortal( - - - , + showActionText ? ( + action + ) : ( + {action} + ), document.body, ); } diff --git a/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx b/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx index 302ae6ff7a40..6f904d5b2590 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx @@ -7,6 +7,7 @@ import type { UserBasic } from "@posthog/shared/domain-types"; import type { EditorSelection } from "@posthog/ui/features/code-editor/components/CodeMirrorEditor"; import { SelectionCommentOverlay } from "@posthog/ui/features/code-editor/components/SelectionCommentOverlay"; import { openExternalUrl } from "@posthog/ui/shell/openExternal"; +import { useThemeStore } from "@posthog/ui/shell/themeStore"; import { parseHttpsUrl } from "@posthog/ui/utils/posthogLinks"; import { useCallback, useMemo, useRef, useState } from "react"; import { ArtifactHtmlFrame } from "./artifactHtmlFrame"; @@ -23,6 +24,7 @@ import { type HighlightResolution, readCommentContext, } from "./commentViewTypes"; +import type { CommentSurfaceTheme } from "./selectionCommentAction"; function isFrameRect(value: unknown): value is ArtifactHtmlFrameRect { if (!value || typeof value !== "object") return false; @@ -60,6 +62,12 @@ export function AnnotatedArtifactHtml({ onResolutionsChange: (resolutions: Map) => void; }) { const channelRef = useRef(`artifact-comments-${crypto.randomUUID()}`); + const theme = useThemeStore( + (s): CommentSurfaceTheme => (s.isDarkMode ? "dark" : "light"), + ); + // Baked into the document at mount; live theme changes ride the `theme` + // message below so a flip doesn't tear down and reload the running preview. + const initialTheme = useRef(theme).current; const [pendingAnchor, setPendingAnchor] = useState( null, ); @@ -69,16 +77,18 @@ export function AnnotatedArtifactHtml({ scriptedArtifactHtmlDocument( html, commentsEnabled ? channelRef.current : undefined, + initialTheme, ), - [commentsEnabled, html], + [commentsEnabled, html, initialTheme], ); const fallbackDocument = useMemo( () => artifactHtmlDocument( html, commentsEnabled ? channelRef.current : undefined, + initialTheme, ), - [commentsEnabled, html], + [commentsEnabled, html, initialTheme], ); const bridgeItems = useMemo( @@ -102,6 +112,12 @@ export function AnnotatedArtifactHtml({ const messages = useMemo(() => { if (!commentsEnabled) return []; const next: Record[] = [ + { + marker: ARTIFACT_HTML_BRIDGE_MARKER, + channel: channelRef.current, + type: "theme", + theme, + }, { marker: ARTIFACT_HTML_BRIDGE_MARKER, channel: channelRef.current, @@ -118,7 +134,7 @@ export function AnnotatedArtifactHtml({ }); } return next; - }, [bridgeItems, commentsEnabled, locateRequest]); + }, [bridgeItems, commentsEnabled, locateRequest, theme]); const receive = useCallback( (value: unknown, frameBox: ArtifactHtmlFrameRect) => { diff --git a/products/desktop/packages/ui/src/features/sessions/components/ArtifactPreview.test.tsx b/products/desktop/packages/ui/src/features/sessions/components/ArtifactPreview.test.tsx index 250773ad3ec9..37b45f843b81 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/ArtifactPreview.test.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/ArtifactPreview.test.tsx @@ -806,7 +806,7 @@ describe("ArtifactPreview", () => { expect(document).toContain("__POSTHOG_ARTIFACT_COMMENT_BRIDGE__"); expect(document).toContain("posthog-artifact-comment-active"); expect(document).not.toContain("ph-artifact-comment-outline"); - expect(document).toContain("💬 Comment"); + expect(document).toContain('textContent="Comment"'); expect(document).toContain('var CHANNEL="test-channel"'); expect(document).toContain('d.type==="locate"'); expect(document).toContain('send("open-external",{href:link.href})'); diff --git a/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx b/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx index 14ad964b2dcc..a5cef3fe34ff 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx @@ -20,6 +20,7 @@ import { type HighlightResolution, readCommentContext, } from "./commentViewTypes"; +import { installSelectionSettleGate } from "./selectionCommentAction"; type HighlightRect = { id: string; @@ -334,14 +335,19 @@ export function ArtifactTextAnnotations({ }, }); }; - const handleSelectionChange = () => { + const scheduleUpdate = () => { cancelAnimationFrame(frame); frame = requestAnimationFrame(updateSelection); }; - document.addEventListener("selectionchange", handleSelectionChange); + const removeGate = installSelectionSettleGate(document, { + onGestureStart: clearOverlay, + onSelectionSettled: scheduleUpdate, + onIdleSelectionChange: scheduleUpdate, + onGestureCancel: clearOverlay, + }); return () => { cancelAnimationFrame(frame); - document.removeEventListener("selectionchange", handleSelectionChange); + removeGate(); }; }, [clearOverlay, containerRef, rootRef]); diff --git a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts new file mode 100644 index 000000000000..43f686d64836 --- /dev/null +++ b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts @@ -0,0 +1,138 @@ +// @ts-expect-error jsdom ships no bundled types; only the test harness needs it +import { JSDOM } from "jsdom"; +import { describe, expect, it } from "vitest"; +import { injectArtifactHtmlCommentBridge } from "./artifactHtmlCommentBridge"; +import { COMMENT_ACTION_BUTTON_THEMES } from "./selectionCommentAction"; + +const CHANNEL = "test-channel"; +const BRIDGE_MARKER = "__POSTHOG_ARTIFACT_COMMENT_BRIDGE__"; + +function loadBridgeDocument( + html: string, + theme: "light" | "dark" = "light", +): JSDOM { + const dom = new JSDOM(injectArtifactHtmlCommentBridge(html, CHANNEL, theme), { + runScripts: "dangerously", + url: "https://localhost/", + }); + // jsdom collapses layout boxes to zero; the bridge hides the action for + // zero-size ranges, which real documents never produce for text selections. + dom.window.Range.prototype.getBoundingClientRect = () => + ({ + top: 40, + left: 10, + right: 110, + bottom: 60, + width: 100, + height: 20, + }) as DOMRect; + return dom; +} + +function selectParagraph(dom: JSDOM): void { + const selection = dom.window.getSelection(); + const paragraph = dom.window.document.querySelector("p"); + if (!selection || !paragraph) throw new Error("test document needs a

"); + const range = dom.window.document.createRange(); + range.selectNodeContents(paragraph); + selection.addRange(range); +} + +function pressOn(dom: JSDOM, selector: string): void { + const element = dom.window.document.querySelector(selector); + if (!element) throw new Error(`missing ${selector}`); + element.dispatchEvent( + new dom.window.MouseEvent("pointerdown", { bubbles: true }), + ); +} + +function releaseOn(dom: JSDOM, selector: string): void { + const element = dom.window.document.querySelector(selector); + if (!element) throw new Error(`missing ${selector}`); + element.dispatchEvent( + new dom.window.MouseEvent("pointerup", { bubbles: true }), + ); +} + +function actionButton(dom: JSDOM): HTMLElement | null { + return dom.window.document.querySelector( + ".ph-comment-action-button", + ); +} + +describe("artifactHtmlCommentBridge", () => { + it("shows the comment action only after the selection settles, not mid-drag", () => { + const dom = loadBridgeDocument( + "

some selectable text here

", + ); + + pressOn(dom, "p"); + selectParagraph(dom); + dom.window.document.dispatchEvent(new dom.window.Event("selectionchange")); + expect(actionButton(dom)?.style.display ?? "none").toBe("none"); + + releaseOn(dom, "p"); + expect(actionButton(dom)?.style.display).toBe("flex"); + expect(actionButton(dom)?.textContent).toBe("Comment"); + dom.window.close(); + }); + + it("ignores presses on the action button itself", () => { + const dom = loadBridgeDocument( + "

some selectable text here

", + ); + + pressOn(dom, "p"); + selectParagraph(dom); + releaseOn(dom, "p"); + expect(actionButton(dom)?.style.display).toBe("flex"); + + pressOn(dom, ".ph-comment-action-button"); + expect(actionButton(dom)?.style.display).toBe("flex"); + dom.window.close(); + }); + + it("bakes the requested theme into the bridge styles", () => { + const dark = injectArtifactHtmlCommentBridge( + "", + CHANNEL, + "dark", + ); + const light = injectArtifactHtmlCommentBridge( + "", + CHANNEL, + "light", + ); + expect(dark).toContain( + `--ph-comment-action-bg:${COMMENT_ACTION_BUTTON_THEMES.dark.background}`, + ); + expect(light).toContain( + `--ph-comment-action-bg:${COMMENT_ACTION_BUTTON_THEMES.light.background}`, + ); + }); + + it("re-themes a running document from a host theme message", () => { + const dom = loadBridgeDocument( + "

text

", + "light", + ); + // jsdom's postMessage leaves event.source null; the bridge ignores those. + dom.window.dispatchEvent( + new dom.window.MessageEvent("message", { + data: { + marker: BRIDGE_MARKER, + channel: CHANNEL, + type: "theme", + theme: "dark", + }, + source: dom.window, + }), + ); + expect( + dom.window.document.documentElement.style.getPropertyValue( + "--ph-comment-action-bg", + ), + ).toBe(COMMENT_ACTION_BUTTON_THEMES.dark.background); + dom.window.close(); + }); +}); diff --git a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts index 59c162f5a373..5a109824acea 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts @@ -1,4 +1,12 @@ import { resolveTextCommentAnchor } from "@posthog/core/comments/anchors"; +import { + COMMENT_ACTION_BUTTON_THEMES, + type CommentSurfaceTheme, + commentActionButtonCss, + commentActionButtonCssVars, + installSelectionSettleGate, + setCommentActionTheme, +} from "./selectionCommentAction"; const BRIDGE_MARKER = "__POSTHOG_ARTIFACT_COMMENT_BRIDGE__"; @@ -6,7 +14,11 @@ const BRIDGE_MARKER = "__POSTHOG_ARTIFACT_COMMENT_BRIDGE__"; * Runs inside the isolated artifact document. It never receives credentials * or writes to the API; selection traffic uses a per-view host channel. */ -function artifactHtmlCommentBridge(channel: string, nonce?: string): string { +function artifactHtmlCommentBridge( + channel: string, + theme: CommentSurfaceTheme, + nonce?: string, +): string { const safeChannel = JSON.stringify(channel); const nonceAttribute = nonce ? ` nonce="${nonce}"` : ""; return `(function(){ @@ -22,19 +34,26 @@ function offsets(range){var a=document.createRange(),b=document.createRange();a. function makeAnchor(range){var all=text(),o=offsets(range),quote=all.slice(o.start,o.end);if(!quote.trim()||quote.length>10000)return null;return{kind:"text",quote:quote,prefix:all.slice(Math.max(0,o.start-32),o.start),suffix:all.slice(o.end,o.end+32),start:o.start,end:o.end};} function textIndex(){var w=document.createTreeWalker(document.body,NodeFilter.SHOW_TEXT),n,all="",entries=[];while((n=w.nextNode())){var start=all.length;all+=n.data;entries.push({node:n,start:start,end:all.length})}return{text:all,entries:entries}} function rangeAt(index,start,end){function find(offset){var low=0,high=index.entries.length-1,match=null;while(low<=high){var middle=(low+high)>>1,entry=index.entries[middle];if(offsetentry.end)low=middle+1;else{match=entry;high=middle-1}}return match}var sn=find(start),en=find(end);if(!sn||!en)return null;try{var r=document.createRange();r.setStart(sn.node,start-sn.start);r.setEnd(en.node,end-en.start);return r}catch(e){return null}} +var THEMES=${JSON.stringify(COMMENT_ACTION_BUTTON_THEMES)}; +var setActionTheme=${setCommentActionTheme.toString()}; var resolveAnchor=${resolveTextCommentAnchor.toString()}; function resolve(all,a){return resolveAnchor(all,a)} -function style(){var s=document.createElement("style");s.setAttribute("data-posthog-artifact-comments","");s.textContent="::highlight(posthog-artifact-comment){background:rgba(250,204,21,.32);color:inherit}::highlight(posthog-artifact-comment-active){background:rgba(250,204,21,.48);color:inherit}.ph-artifact-comment-button{position:fixed;z-index:2147483647;display:flex;align-items:center;gap:6px;height:34px;padding:0 13px;border:1px solid #ca8a04;border-radius:9px;background:#facc15;color:#1c1917;font:600 13px/1 -apple-system,BlinkMacSystemFont,'Segoe UI',sans-serif;box-shadow:0 3px 12px rgba(0,0,0,.3);cursor:pointer}.ph-artifact-comment-button:hover{background:#fde047}";(document.head||document.documentElement).appendChild(s)} +function style(){var s=document.createElement("style");s.setAttribute("data-posthog-artifact-comments","");s.textContent="::highlight(posthog-artifact-comment){background:rgba(250,204,21,.32);color:inherit}::highlight(posthog-artifact-comment-active){background:rgba(250,204,21,.48);color:inherit}:root{${commentActionButtonCssVars(theme)}}${commentActionButtonCss()}";(document.head||document.documentElement).appendChild(s)} function hide(){if(state.button)state.button.style.display="none"} -function button(){if(state.button&&state.button.isConnected)return state.button;var b=document.createElement("button");b.type="button";b.className="ph-artifact-comment-button";b.textContent="💬 Comment";b.style.display="none";b.addEventListener("mousedown",function(e){e.preventDefault();e.stopPropagation();var sel=window.getSelection();if(!sel||sel.isCollapsed||!sel.rangeCount)return hide();var range=sel.getRangeAt(0),anchor=makeAnchor(range);if(!anchor)return hide();var r=range.getBoundingClientRect(),br=b.getBoundingClientRect();send("selection",{anchor:anchor,rect:{top:r.top,left:r.left,right:r.right,bottom:r.bottom,width:r.width,height:r.height},triggerRect:{top:br.top,left:br.left,right:br.right,bottom:br.bottom,width:br.width,height:br.height}});hide();sel.removeAllRanges()});document.documentElement.appendChild(b);state.button=b;return b;} -function selectionChanged(){clearTimeout(state.timer);state.timer=setTimeout(function(){var sel=window.getSelection();if(!sel||sel.isCollapsed||!sel.rangeCount)return hide();var value=sel.toString().trim();if(value.length<2||value.length>10000)return hide();var r=sel.getRangeAt(0).getBoundingClientRect();if(!r||(r.width===0&&r.height===0))return hide();var b=button();b.style.left=Math.max(8,Math.min(innerWidth-110,r.left+r.width/2-50))+"px";b.style.top=Math.max(8,r.top-42)+"px";b.style.display="flex"},80)} +function button(){if(state.button&&state.button.isConnected)return state.button;var b=document.createElement("button");b.type="button";b.className="ph-comment-action-button";b.setAttribute("data-selection-comment-overlay","");b.textContent="Comment";b.style.display="none";b.addEventListener("mousedown",function(e){e.preventDefault();e.stopPropagation();var sel=window.getSelection();if(!sel||sel.isCollapsed||!sel.rangeCount)return hide();var range=sel.getRangeAt(0),anchor=makeAnchor(range);if(!anchor)return hide();var r=range.getBoundingClientRect(),br=b.getBoundingClientRect();send("selection",{anchor:anchor,rect:{top:r.top,left:r.left,right:r.right,bottom:r.bottom,width:r.width,height:r.height},triggerRect:{top:br.top,left:br.left,right:br.right,bottom:br.bottom,width:br.width,height:br.height}});hide();sel.removeAllRanges()});document.documentElement.appendChild(b);state.button=b;return b;} +function positionButton(){var sel=window.getSelection();if(!sel||sel.isCollapsed||!sel.rangeCount)return hide();var value=sel.toString().trim();if(value.length<2||value.length>10000)return hide();var r=sel.getRangeAt(0).getBoundingClientRect();if(!r||(r.width===0&&r.height===0))return hide();var b=button();b.style.left=Math.max(8,Math.min(innerWidth-110,r.left+r.width/2-50))+"px";b.style.top=Math.max(8,r.top-42)+"px";b.style.display="flex"} +function selectionChanged(){clearTimeout(state.timer);state.timer=setTimeout(positionButton,80)} function render(items){state.items=items||[];state.entries=[];var normal=supportsHighlights?new Highlight():null,active=supportsHighlights?new Highlight():null,resolutions=[],index=textIndex();state.items.forEach(function(item){var hit=resolve(index.text,item.anchor);if(!hit){resolutions.push({id:item.id,status:"orphaned"});return}var range=rangeAt(index,hit.start,hit.end);if(!range){resolutions.push({id:item.id,status:"orphaned"});return}state.entries.push({id:item.id,range:range,active:!!item.active});resolutions.push({id:item.id,status:hit.status});if(supportsHighlights){if(item.active)active.add(range);else normal.add(range)}});if(supportsHighlights){CSS.highlights.set("posthog-artifact-comment",normal);CSS.highlights.set("posthog-artifact-comment-active",active)}send("resolutions",{items:resolutions})} function locate(id){for(var i=0;i=r.left&&e.clientX<=r.right&&e.clientY>=r.top&&e.clientY<=r.bottom){e.preventDefault();e.stopPropagation();send("activate",{id:state.entries[i].id});return}}}},true); new MutationObserver(function(){if(!state.items.length||state.renderTimer)return;state.renderTimer=setTimeout(function(){state.renderTimer=0;render(state.items)},500)}).observe(document.body,{childList:true,characterData:true,subtree:true}); -window.addEventListener("message",function(e){if(e.source!==parent)return;var d=e.data;if(!d||d.marker!==MARKER||d.channel!==CHANNEL)return;if(d.type==="comments")render(d.items);else if(d.type==="locate"&&typeof d.id==="string")locate(d.id)}); +window.addEventListener("message",function(e){if(e.source!==parent)return;var d=e.data;if(!d||d.marker!==MARKER||d.channel!==CHANNEL)return;if(d.type==="comments")render(d.items);else if(d.type==="locate"&&typeof d.id==="string")locate(d.id);else if(d.type==="theme"&&(d.theme==="light"||d.theme==="dark"))setActionTheme(d.theme,THEMES)}); style();send("ready"); })();`; } @@ -42,9 +61,10 @@ style();send("ready"); export function injectArtifactHtmlCommentBridge( html: string, channel: string, + theme: CommentSurfaceTheme, nonce?: string, ): string { - const bridge = artifactHtmlCommentBridge(channel, nonce); + const bridge = artifactHtmlCommentBridge(channel, theme, nonce); const bodyEnd = html.toLowerCase().lastIndexOf(""); if (bodyEnd >= 0) { return `${html.slice(0, bodyEnd)}${bridge}${html.slice(bodyEnd)}`; diff --git a/products/desktop/packages/ui/src/features/sessions/components/artifactPreviewDocument.ts b/products/desktop/packages/ui/src/features/sessions/components/artifactPreviewDocument.ts index ffe20b2f36d2..9ccd9f98d515 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/artifactPreviewDocument.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/artifactPreviewDocument.ts @@ -1,6 +1,7 @@ import { getImageMimeType, isAllowedImageMimeType } from "@posthog/shared"; import { applyCspToHtml } from "../../mcp-apps/utils/mcp-app-csp"; import { injectArtifactHtmlCommentBridge } from "./artifactHtmlCommentBridge"; +import type { CommentSurfaceTheme } from "./selectionCommentAction"; function removeAutomaticRedirects(html: string): string { if (!/ { + it("suppresses selection reporting while the user drags, then reports the settled selection", () => { + const callbacks = { + onGestureStart: vi.fn(), + onSelectionSettled: vi.fn(), + onIdleSelectionChange: vi.fn(), + } satisfies SelectionSettleGateCallbacks; + const remove = installSelectionSettleGate(document, callbacks); + const target = eventTarget(); + + press(target); + expect(callbacks.onGestureStart).toHaveBeenCalledTimes(1); + + changeSelection(); + changeSelection(); + expect(callbacks.onIdleSelectionChange).not.toHaveBeenCalled(); + + release(target); + expect(callbacks.onSelectionSettled).toHaveBeenCalledTimes(1); + remove(); + }); + + it("reports immediately when the selection changes without a drag", () => { + const callbacks = { onIdleSelectionChange: vi.fn() }; + const remove = installSelectionSettleGate(document, callbacks); + + changeSelection(); + expect(callbacks.onIdleSelectionChange).toHaveBeenCalledTimes(1); + remove(); + }); + + it("ignores presses inside the action UI so pressing the comment button doesn't hide it", () => { + const callbacks = { + onGestureStart: vi.fn(), + onIdleSelectionChange: vi.fn(), + } satisfies SelectionSettleGateCallbacks; + const remove = installSelectionSettleGate(document, callbacks); + const overlay = document.createElement("div"); + overlay.setAttribute("data-selection-comment-overlay", ""); + const innerButton = document.createElement("button"); + overlay.appendChild(innerButton); + document.body.appendChild(overlay); + + press(innerButton); + expect(callbacks.onGestureStart).not.toHaveBeenCalled(); + + changeSelection(); + expect(callbacks.onIdleSelectionChange).toHaveBeenCalledTimes(1); + remove(); + overlay.remove(); + }); + + it("cancels the gesture on window blur so a release outside the window can't stick the action hidden", () => { + const callbacks = { + onGestureCancel: vi.fn(), + onSelectionSettled: vi.fn(), + } satisfies SelectionSettleGateCallbacks; + const remove = installSelectionSettleGate(document, callbacks); + const target = eventTarget(); + + press(target); + window.dispatchEvent(new Event("blur")); + expect(callbacks.onGestureCancel).toHaveBeenCalledTimes(1); + + release(target); + expect(callbacks.onSelectionSettled).not.toHaveBeenCalled(); + + changeSelection(); + expect(callbacks.onGestureCancel).toHaveBeenCalledTimes(1); + remove(); + }); + + it("settles on pointercancel so touch gesture interruptions still show the action", () => { + const callbacks = { onSelectionSettled: vi.fn() }; + const remove = installSelectionSettleGate(document, callbacks); + const target = eventTarget(); + + press(target); + target.dispatchEvent(new PointerEvent("pointercancel", { bubbles: true })); + expect(callbacks.onSelectionSettled).toHaveBeenCalledTimes(1); + remove(); + }); + + it("applies every theme variable the action button styles reference", () => { + for (const theme of ["light", "dark"] as const) { + setCommentActionTheme(theme, COMMENT_ACTION_BUTTON_THEMES); + const css = commentActionButtonCss(); + for (const match of css.matchAll( + /var\((--ph-comment-action-[a-z-]+)\)/g, + )) { + expect( + document.documentElement.style.getPropertyValue(match[1]), + `${theme} sets ${match[1]}`, + ).toBe(COMMENT_ACTION_BUTTON_THEMES[theme][cssVarField(match[1])]); + } + } + document.documentElement.style.cssText = ""; + }); + + it("falls back to the light palette for an unknown theme name", () => { + setCommentActionTheme("solarized", COMMENT_ACTION_BUTTON_THEMES); + expect( + document.documentElement.style.getPropertyValue("--ph-comment-action-bg"), + ).toBe(COMMENT_ACTION_BUTTON_THEMES.light.background); + document.documentElement.style.cssText = ""; + }); + + it("bakes the requested theme into the :root variable declarations", () => { + expect(commentActionButtonCssVars("dark")).toContain( + COMMENT_ACTION_BUTTON_THEMES.dark.background, + ); + expect(commentActionButtonCssVars("light")).toContain( + COMMENT_ACTION_BUTTON_THEMES.light.background, + ); + }); +}); + +function cssVarField( + variable: string, +): keyof (typeof COMMENT_ACTION_BUTTON_THEMES)["light"] { + const fields: Record< + string, + keyof (typeof COMMENT_ACTION_BUTTON_THEMES)["light"] + > = { + "--ph-comment-action-bg": "background", + "--ph-comment-action-fg": "color", + "--ph-comment-action-border": "border", + "--ph-comment-action-hover": "hoverBackground", + }; + return fields[variable]; +} diff --git a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts new file mode 100644 index 000000000000..b0d05c10908a --- /dev/null +++ b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts @@ -0,0 +1,122 @@ +// Shared behavior + look for the floating "Comment" action shown next to a +// text selection. Every surface (artifact annotations, HTML artifact bridge, +// canvas sandbox) wires the same pieces instead of re-implementing them: +// surfaces rendered into sandboxed iframes inject this module's functions via +// .toString(), so those must stay dependency-free and reference no imports. + +export type CommentSurfaceTheme = "light" | "dark"; + +type CommentActionButtonTheme = { + background: string; + color: string; + border: string; + hoverBackground: string; +}; + +// Neutral card colors so the action doesn't compete with the comment +// highlight color behind the selection. The values are hardcoded because they +// run inside opaque-origin sandbox iframes where the app's CSS variable tokens +// don't exist. +export const COMMENT_ACTION_BUTTON_THEMES: Record< + CommentSurfaceTheme, + CommentActionButtonTheme +> = { + light: { + background: "#ffffff", + color: "#1f2328", + border: "rgba(31, 35, 40, 0.16)", + hoverBackground: "#f5f5f4", + }, + dark: { + background: "#2b2b2e", + color: "#fafafa", + border: "rgba(255, 255, 255, 0.16)", + hoverBackground: "#3f3f43", + }, +}; + +// CSS variables let the host flip the theme of an already-running iframe +// document with a `theme` message instead of rebuilding (and reloading) it. +export function commentActionButtonCssVars(theme: CommentSurfaceTheme): string { + const palette = COMMENT_ACTION_BUTTON_THEMES[theme]; + return `--ph-comment-action-bg:${palette.background};--ph-comment-action-fg:${palette.color};--ph-comment-action-border:${palette.border};--ph-comment-action-hover:${palette.hoverBackground};`; +} + +export function commentActionButtonCss(): string { + return `.ph-comment-action-button{position:fixed;z-index:2147483647;display:flex;align-items:center;gap:6px;height:34px;padding:0 13px;border:1px solid var(--ph-comment-action-border);border-radius:8px;background:var(--ph-comment-action-bg);color:var(--ph-comment-action-fg);font:500 13px/1 -apple-system,BlinkMacSystemFont,'Segoe UI',sans-serif;box-shadow:0 3px 12px rgba(0,0,0,.18);cursor:pointer}.ph-comment-action-button:hover{background:var(--ph-comment-action-hover)}`; +} + +// Dependency-free: injected into iframe bridge scripts via .toString() and +// called with the baked-in COMMENT_ACTION_BUTTON_THEMES. +export function setCommentActionTheme( + theme: string, + themes: Record, +): void { + const palette = themes[theme] || themes.light; + if (!palette) return; + const style = document.documentElement.style; + style.setProperty("--ph-comment-action-bg", palette.background); + style.setProperty("--ph-comment-action-fg", palette.color); + style.setProperty("--ph-comment-action-border", palette.border); + style.setProperty("--ph-comment-action-hover", palette.hoverBackground); +} + +export type SelectionSettleGateCallbacks = { + // Pointer pressed outside the action UI; hide any visual anchored to the + // selection. + onGestureStart?: () => void; + // Pointer released after a gesture; re-read the selection, it is final. + onSelectionSettled?: () => void; + // Selection changed with no pointer down (keyboard, programmatic). + onIdleSelectionChange?: () => void; + // Gesture interrupted before pointerup (window blur). + onGestureCancel?: () => void; +}; + +// While the user drag-selects, the range keeps moving, so an action anchored +// to the live selection visibly chases the cursor. The gate suppresses +// selection reporting during the drag and reports the final range on +// pointerup. Clicks inside the action UI are excluded so pressing the action +// itself doesn't restart a gesture. +export function installSelectionSettleGate( + doc: Document, + callbacks: SelectionSettleGateCallbacks, +): () => void { + let dragging = false; + const onPointerDown = (event: Event) => { + if ( + event.target instanceof Element && + event.target.closest("[data-selection-comment-overlay]") + ) { + return; + } + dragging = true; + callbacks.onGestureStart?.(); + }; + const settle = () => { + if (!dragging) return; + dragging = false; + callbacks.onSelectionSettled?.(); + }; + const cancel = () => { + if (!dragging) return; + dragging = false; + callbacks.onGestureCancel?.(); + }; + const onSelectionChange = () => { + if (dragging) return; + callbacks.onIdleSelectionChange?.(); + }; + doc.addEventListener("pointerdown", onPointerDown, true); + doc.addEventListener("pointerup", settle, true); + doc.addEventListener("pointercancel", settle, true); + doc.addEventListener("selectionchange", onSelectionChange); + doc.defaultView?.addEventListener("blur", cancel); + return () => { + doc.removeEventListener("pointerdown", onPointerDown, true); + doc.removeEventListener("pointerup", settle, true); + doc.removeEventListener("pointercancel", settle, true); + doc.removeEventListener("selectionchange", onSelectionChange); + doc.defaultView?.removeEventListener("blur", cancel); + }; +} From 5c9e79879167f2f7c2747929874472d7a1eb7bc5 Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Fri, 7 Aug 2026 11:48:59 +0200 Subject: [PATCH 2/7] feat(comments): anchor selection action to selection end, pill styling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Placement: the floating comment action now anchors like Google Docs — just right of the selection's end caret, vertically centered on the end line; it flips below the line when the right edge has no room and above near the viewport bottom. The shared computeCommentActionPlacement() applies everywhere: the code editor previously anchored to the START of the end line (CodeMirrorEditor), text artifacts and canvases to the whole-range bounding box, and the HTML-artifact bridge centered above the range. Styling: one pill look shared by the in-app trigger and the sandboxed iframe CSS (border-radius 999px, app-gray card tokens, chat-bubble icon, 600-weight 12px label). Hover now has a real background shift plus shadow lift and cursor: pointer — previously the desktop Quill button kept cursor: default and its hover background was nearly identical to rest, so hovering looked dead. The canvas runtime stops hand-rolling its own button CSS and uses the shared theme/css/placement helpers like the artifact bridge. Generated-By: PostHog Code Task-Id: e93165cd-6893-45dd-8fa8-4e97299f5f06 --- .../freeform/CanvasSelectionCommentAction.tsx | 8 +-- .../canvas/freeform/sandboxRuntime.ts | 5 +- .../components/CodeMirrorEditor.tsx | 11 +-- .../SelectionCommentOverlay.test.tsx | 8 +-- .../components/SelectionCommentOverlay.tsx | 36 +++++----- .../components/AnnotatedArtifactHtml.tsx | 8 +-- .../components/AnnotatedArtifactImage.tsx | 6 +- .../components/ArtifactPreview.test.tsx | 11 +-- .../components/ArtifactTextAnnotations.tsx | 11 ++- .../artifactHtmlCommentBridge.test.ts | 7 ++ .../components/artifactHtmlCommentBridge.ts | 8 ++- .../components/selectionCommentAction.test.ts | 48 +++++++++++++ .../components/selectionCommentAction.ts | 72 +++++++++++++++---- 13 files changed, 175 insertions(+), 64 deletions(-) diff --git a/products/desktop/packages/ui/src/features/canvas/freeform/CanvasSelectionCommentAction.tsx b/products/desktop/packages/ui/src/features/canvas/freeform/CanvasSelectionCommentAction.tsx index 2b238bcebc33..2ebf12204030 100644 --- a/products/desktop/packages/ui/src/features/canvas/freeform/CanvasSelectionCommentAction.tsx +++ b/products/desktop/packages/ui/src/features/canvas/freeform/CanvasSelectionCommentAction.tsx @@ -46,11 +46,9 @@ export function CanvasSelectionCommentAction({ fromLine: selection.start + 1, toLine: selection.end + 1, anchor: { - top: selection.rect.bottom, - left: Math.max( - 8, - Math.min(selection.rect.right, window.innerWidth - 440), - ), + top: selection.rect.top, + left: selection.rect.right, + bottom: selection.rect.bottom, }, } : null diff --git a/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts b/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts index e193082cb832..75d5ba5eed0b 100644 --- a/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts +++ b/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts @@ -316,7 +316,10 @@ export function buildSandboxDocument( clearTextSelection(); return; } - const rect = range.getBoundingClientRect(); + // The END line's rect, so the host anchors the comment action where the + // pointer was released rather than at the whole-range bounding box. + const clientRects = range.getClientRects(); + const rect = clientRects.length ? clientRects[clientRects.length - 1] : range.getBoundingClientRect(); post({ type: "text-selection", selection: { diff --git a/products/desktop/packages/ui/src/features/code-editor/components/CodeMirrorEditor.tsx b/products/desktop/packages/ui/src/features/code-editor/components/CodeMirrorEditor.tsx index fde71b123a89..a5671325b14d 100644 --- a/products/desktop/packages/ui/src/features/code-editor/components/CodeMirrorEditor.tsx +++ b/products/desktop/packages/ui/src/features/code-editor/components/CodeMirrorEditor.tsx @@ -41,8 +41,8 @@ export interface EditorSelection { /** 1-based line numbers. */ fromLine: number; toLine: number; - /** Viewport pixel anchor below the selection, or null when off-screen. */ - anchor: { top: number; left: number } | null; + /** Viewport rect of the selection's end caret (end-line top/bottom + end column x), or null when off-screen. */ + anchor: { top: number; left: number; bottom: number } | null; } interface CodeMirrorEditorProps { @@ -100,13 +100,16 @@ export function CodeMirrorEditor({ return; } const endRect = update.view.coordsAtPos(sel.to); - const startRect = update.view.coordsAtPos(doc.lineAt(sel.to).from); cb({ text: doc.sliceString(sel.from, sel.to), fromLine: doc.lineAt(sel.from).number, toLine: doc.lineAt(sel.to).number, anchor: endRect - ? { top: endRect.bottom, left: (startRect ?? endRect).left } + ? { + top: endRect.top, + left: endRect.right, + bottom: endRect.bottom, + } : null, }); }), diff --git a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx index b0069e3367a3..e0a783324527 100644 --- a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx +++ b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx @@ -34,7 +34,7 @@ function renderCollapsed( text: "selected", fromLine: 1, toLine: 1, - anchor: { top: 20, left: 20 }, + anchor: { top: 20, left: 20, bottom: 38 }, }} open filePath="report.md" @@ -53,7 +53,7 @@ describe("SelectionCommentOverlay", () => { text: "selected", fromLine: 1, toLine: 1, - anchor: { top: 20, left: 20 }, + anchor: { top: 20, left: 20, bottom: 38 }, }} open filePath="report.md" @@ -95,7 +95,7 @@ describe("SelectionCommentOverlay", () => { text: "selected", fromLine: 1, toLine: 1, - anchor: { top: 20, left: 20 }, + anchor: { top: 20, left: 20, bottom: 38 }, }} open filePath="report.md" @@ -130,7 +130,7 @@ describe("SelectionCommentOverlay", () => { text: "selected", fromLine: 1, toLine: 1, - anchor: { top: 20, left: 20 }, + anchor: { top: 20, left: 20, bottom: 38 }, }} open filePath="report.md" diff --git a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx index bbfb54b3bbb7..84066452fb8d 100644 --- a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx +++ b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx @@ -1,9 +1,9 @@ import { ChatCircle, Plus } from "@phosphor-icons/react"; -import { Button } from "@posthog/quill"; import type { UserBasic } from "@posthog/shared/domain-types"; import type { EditorSelection } from "@posthog/ui/features/code-editor/components/CodeMirrorEditor"; import { CommentAnnotation } from "@posthog/ui/features/code-review/components/CommentAnnotation"; import { CommentComposer } from "@posthog/ui/features/sessions/components/CommentComposer"; +import { computeCommentActionPlacement } from "@posthog/ui/features/sessions/components/selectionCommentAction"; import { Tooltip } from "@posthog/ui/primitives/Tooltip"; import { useCallback, useEffect, useState } from "react"; import { createPortal } from "react-dom"; @@ -88,7 +88,7 @@ function SelectionComposerCard({ initiallyExpanded, members, }: { - anchor: { top: number; left: number }; + anchor: { top: number; left: number; bottom: number }; fromLine: number; toLine: number; filePath: string; @@ -109,18 +109,14 @@ function SelectionComposerCard({ const expanded = initiallyExpanded || userExpanded; const [draft, setDraft] = useState(""); const [submitting, setSubmitting] = useState(false); - const overlayWidth = expanded ? Math.min(420, window.innerWidth * 0.8) : 120; - const overlayHeight = expanded ? 180 : 36; - const style = { - top: Math.max( - 8, - Math.min(anchor.top + 4, window.innerHeight - overlayHeight - 8), - ), - left: Math.max( - 8, - Math.min(anchor.left, window.innerWidth - overlayWidth - 8), - ), - }; + const actionSize = expanded + ? { width: Math.min(420, window.innerWidth * 0.8), height: 180 } + : { width: showActionText ? 116 : 30, height: 30 }; + const style = computeCommentActionPlacement( + { top: anchor.top, right: anchor.left, bottom: anchor.bottom }, + { width: window.innerWidth, height: window.innerHeight }, + actionSize, + ); useEffect(() => { const dismissOutside = (event: PointerEvent) => { @@ -139,13 +135,15 @@ function SelectionComposerCard({ if (!expanded) { const action = ( - + ); // The text button names itself; only the icon-only variant needs a label // on hover. diff --git a/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx b/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx index 6f904d5b2590..cdfad9d1bda6 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx @@ -176,11 +176,9 @@ export function AnnotatedArtifactHtml({ fromLine: parsed.data.start + 1, toLine: parsed.data.end + 1, anchor: { - top: frameBox.top + data.triggerRect.bottom, - left: Math.min( - frameBox.left + data.triggerRect.right + 6, - window.innerWidth - 440, - ), + top: frameBox.top + data.triggerRect.top, + left: frameBox.left + data.triggerRect.left, + bottom: frameBox.top + data.triggerRect.bottom, }, }); }, diff --git a/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactImage.tsx b/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactImage.tsx index 712e8816dd8d..2649b7f79ea4 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactImage.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactImage.tsx @@ -64,10 +64,8 @@ function ImageCommentCreationLayer({ text: "Image region", fromLine: 1, toLine: 1, - anchor: { - top: clientY + 4, - left: Math.min(clientX + 4, window.innerWidth - 440), - }, + // Point anchor at the click: the composer opens next to it. + anchor: { top: clientY, left: clientX, bottom: clientY }, }); }} /> diff --git a/products/desktop/packages/ui/src/features/sessions/components/ArtifactPreview.test.tsx b/products/desktop/packages/ui/src/features/sessions/components/ArtifactPreview.test.tsx index 37b45f843b81..fc2485284281 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/ArtifactPreview.test.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/ArtifactPreview.test.tsx @@ -484,7 +484,8 @@ describe("ArtifactPreview", () => { const heading = screen.getByRole("heading", { name: "Report" }); const range = document.createRange(); range.selectNodeContents(heading); - range.getBoundingClientRect = () => ({ bottom: 20, right: 120 }) as DOMRect; + range.getBoundingClientRect = () => + ({ top: 0, left: 20, right: 120, bottom: 20 }) as DOMRect; window.getSelection()?.removeAllRanges(); window.getSelection()?.addRange(range); fireEvent.mouseUp(heading); @@ -526,7 +527,8 @@ describe("ArtifactPreview", () => { const heading = screen.getByRole("heading", { name: "Report" }); const range = document.createRange(); range.selectNodeContents(heading); - range.getBoundingClientRect = () => ({ bottom: 20, right: 120 }) as DOMRect; + range.getBoundingClientRect = () => + ({ top: 0, left: 20, right: 120, bottom: 20 }) as DOMRect; window.getSelection()?.removeAllRanges(); window.getSelection()?.addRange(range); fireEvent.mouseUp(heading); @@ -558,7 +560,8 @@ describe("ArtifactPreview", () => { const heading = screen.getByRole("heading", { name: "Report" }); const range = document.createRange(); range.selectNodeContents(heading); - range.getBoundingClientRect = () => ({ bottom: 20, right: 120 }) as DOMRect; + range.getBoundingClientRect = () => + ({ top: 0, left: 20, right: 120, bottom: 20 }) as DOMRect; window.getSelection()?.removeAllRanges(); window.getSelection()?.addRange(range); fireEvent.mouseUp(heading); @@ -806,7 +809,7 @@ describe("ArtifactPreview", () => { expect(document).toContain("__POSTHOG_ARTIFACT_COMMENT_BRIDGE__"); expect(document).toContain("posthog-artifact-comment-active"); expect(document).not.toContain("ph-artifact-comment-outline"); - expect(document).toContain('textContent="Comment"'); + expect(document).toContain("Comment"); expect(document).toContain('var CHANNEL="test-channel"'); expect(document).toContain('d.type==="locate"'); expect(document).toContain('send("open-external",{href:link.href})'); diff --git a/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx b/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx index a5cef3fe34ff..39aa5291507b 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx @@ -323,15 +323,20 @@ export function ArtifactTextAnnotations({ offsets.end, ); if (!anchor) return; - const box = range.getBoundingClientRect(); + // Anchor to the selection's end line, where the pointer was released. + const clientRects = range.getClientRects?.() ?? []; + const endRect = clientRects.length + ? clientRects[clientRects.length - 1] + : range.getBoundingClientRect(); setPendingAnchor(anchor); setSelection({ text: anchor.quote, fromLine: offsets.start + 1, toLine: offsets.end + 1, anchor: { - top: box.bottom, - left: Math.min(box.right, window.innerWidth - 440), + top: endRect.top, + left: endRect.right, + bottom: endRect.bottom, }, }); }; diff --git a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts index 43f686d64836..852c7b08adbf 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts @@ -26,6 +26,9 @@ function loadBridgeDocument( width: 100, height: 20, }) as DOMRect; + dom.window.Range.prototype.getClientRects = function () { + return [this.getBoundingClientRect()] as unknown as DOMRectList; + }; return dom; } @@ -74,6 +77,10 @@ describe("artifactHtmlCommentBridge", () => { releaseOn(dom, "p"); expect(actionButton(dom)?.style.display).toBe("flex"); expect(actionButton(dom)?.textContent).toBe("Comment"); + // Anchored right of the selection end (110 + 8), centered on the end + // line (50 - 15). + expect(actionButton(dom)?.style.left).toBe("118px"); + expect(actionButton(dom)?.style.top).toBe("35px"); dom.window.close(); }); diff --git a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts index 5a109824acea..1cc5d6739cbd 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts @@ -1,9 +1,11 @@ import { resolveTextCommentAnchor } from "@posthog/core/comments/anchors"; import { COMMENT_ACTION_BUTTON_THEMES, + COMMENT_ACTION_ICON_SVG, type CommentSurfaceTheme, commentActionButtonCss, commentActionButtonCssVars, + computeCommentActionPlacement, installSelectionSettleGate, setCommentActionTheme, } from "./selectionCommentAction"; @@ -35,13 +37,15 @@ function makeAnchor(range){var all=text(),o=offsets(range),quote=all.slice(o.sta function textIndex(){var w=document.createTreeWalker(document.body,NodeFilter.SHOW_TEXT),n,all="",entries=[];while((n=w.nextNode())){var start=all.length;all+=n.data;entries.push({node:n,start:start,end:all.length})}return{text:all,entries:entries}} function rangeAt(index,start,end){function find(offset){var low=0,high=index.entries.length-1,match=null;while(low<=high){var middle=(low+high)>>1,entry=index.entries[middle];if(offsetentry.end)low=middle+1;else{match=entry;high=middle-1}}return match}var sn=find(start),en=find(end);if(!sn||!en)return null;try{var r=document.createRange();r.setStart(sn.node,start-sn.start);r.setEnd(en.node,end-en.start);return r}catch(e){return null}} var THEMES=${JSON.stringify(COMMENT_ACTION_BUTTON_THEMES)}; +var ACTION_ICON=${JSON.stringify(COMMENT_ACTION_ICON_SVG)}; var setActionTheme=${setCommentActionTheme.toString()}; +var placeAction=${computeCommentActionPlacement.toString()}; var resolveAnchor=${resolveTextCommentAnchor.toString()}; function resolve(all,a){return resolveAnchor(all,a)} function style(){var s=document.createElement("style");s.setAttribute("data-posthog-artifact-comments","");s.textContent="::highlight(posthog-artifact-comment){background:rgba(250,204,21,.32);color:inherit}::highlight(posthog-artifact-comment-active){background:rgba(250,204,21,.48);color:inherit}:root{${commentActionButtonCssVars(theme)}}${commentActionButtonCss()}";(document.head||document.documentElement).appendChild(s)} function hide(){if(state.button)state.button.style.display="none"} -function button(){if(state.button&&state.button.isConnected)return state.button;var b=document.createElement("button");b.type="button";b.className="ph-comment-action-button";b.setAttribute("data-selection-comment-overlay","");b.textContent="Comment";b.style.display="none";b.addEventListener("mousedown",function(e){e.preventDefault();e.stopPropagation();var sel=window.getSelection();if(!sel||sel.isCollapsed||!sel.rangeCount)return hide();var range=sel.getRangeAt(0),anchor=makeAnchor(range);if(!anchor)return hide();var r=range.getBoundingClientRect(),br=b.getBoundingClientRect();send("selection",{anchor:anchor,rect:{top:r.top,left:r.left,right:r.right,bottom:r.bottom,width:r.width,height:r.height},triggerRect:{top:br.top,left:br.left,right:br.right,bottom:br.bottom,width:br.width,height:br.height}});hide();sel.removeAllRanges()});document.documentElement.appendChild(b);state.button=b;return b;} -function positionButton(){var sel=window.getSelection();if(!sel||sel.isCollapsed||!sel.rangeCount)return hide();var value=sel.toString().trim();if(value.length<2||value.length>10000)return hide();var r=sel.getRangeAt(0).getBoundingClientRect();if(!r||(r.width===0&&r.height===0))return hide();var b=button();b.style.left=Math.max(8,Math.min(innerWidth-110,r.left+r.width/2-50))+"px";b.style.top=Math.max(8,r.top-42)+"px";b.style.display="flex"} +function button(){if(state.button&&state.button.isConnected)return state.button;var b=document.createElement("button");b.type="button";b.className="ph-comment-action-button";b.setAttribute("data-selection-comment-overlay","");b.setAttribute("aria-label","Comment");b.innerHTML=ACTION_ICON+"Comment";b.style.display="none";b.addEventListener("mousedown",function(e){e.preventDefault();e.stopPropagation();var sel=window.getSelection();if(!sel||sel.isCollapsed||!sel.rangeCount)return hide();var range=sel.getRangeAt(0),anchor=makeAnchor(range);if(!anchor)return hide();var r=range.getBoundingClientRect(),br=b.getBoundingClientRect();send("selection",{anchor:anchor,rect:{top:r.top,left:r.left,right:r.right,bottom:r.bottom,width:r.width,height:r.height},triggerRect:{top:br.top,left:br.left,right:br.right,bottom:br.bottom,width:br.width,height:br.height}});hide();sel.removeAllRanges()});document.documentElement.appendChild(b);state.button=b;return b;} +function positionButton(){var sel=window.getSelection();if(!sel||sel.isCollapsed||!sel.rangeCount)return hide();var value=sel.toString().trim();if(value.length<2||value.length>10000)return hide();var range=sel.getRangeAt(0);var rects=range.getClientRects?range.getClientRects():[];var r=rects.length?rects[rects.length-1]:range.getBoundingClientRect();if(!r||(r.width===0&&r.height===0))return hide();var b=button();var box=b.getBoundingClientRect();var pos=placeAction({top:r.top,right:r.right,bottom:r.bottom},{width:innerWidth,height:innerHeight},{width:box.width||120,height:box.height||30});b.style.left=pos.left+"px";b.style.top=pos.top+"px";b.style.display="flex"} function selectionChanged(){clearTimeout(state.timer);state.timer=setTimeout(positionButton,80)} function render(items){state.items=items||[];state.entries=[];var normal=supportsHighlights?new Highlight():null,active=supportsHighlights?new Highlight():null,resolutions=[],index=textIndex();state.items.forEach(function(item){var hit=resolve(index.text,item.anchor);if(!hit){resolutions.push({id:item.id,status:"orphaned"});return}var range=rangeAt(index,hit.start,hit.end);if(!range){resolutions.push({id:item.id,status:"orphaned"});return}state.entries.push({id:item.id,range:range,active:!!item.active});resolutions.push({id:item.id,status:hit.status});if(supportsHighlights){if(item.active)active.add(range);else normal.add(range)}});if(supportsHighlights){CSS.highlights.set("posthog-artifact-comment",normal);CSS.highlights.set("posthog-artifact-comment-active",active)}send("resolutions",{items:resolutions})} function locate(id){for(var i=0;i { + const bounds = { width: 1000, height: 700 }; + const action = { width: 120, height: 30 }; + + it("sits right of the selection end, vertically centered on the end line", () => { + expect( + computeCommentActionPlacement( + { top: 100, right: 400, bottom: 120 }, + bounds, + action, + ), + ).toEqual({ top: 95, left: 408 }); + }); + + it("drops below the end line when the right edge has no room, right-aligned to the caret", () => { + expect( + computeCommentActionPlacement( + { top: 100, right: 950, bottom: 120 }, + bounds, + action, + ), + ).toEqual({ top: 126, left: 830 }); + }); + + it("flips above the end line when below would leave the viewport", () => { + expect( + computeCommentActionPlacement( + { top: 100, right: 950, bottom: 120 }, + { width: 1000, height: 140 }, + action, + ), + ).toEqual({ top: 64, left: 830 }); + }); + + it("clamps to the viewport margins for selections hugging the edges", () => { + expect( + computeCommentActionPlacement( + { top: 4, right: 2, bottom: 12 }, + bounds, + action, + ), + ).toEqual({ top: 8, left: 10 }); + }); +}); diff --git a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts index b0d05c10908a..d2f79452eca8 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts @@ -11,39 +11,53 @@ type CommentActionButtonTheme = { color: string; border: string; hoverBackground: string; + shadow: string; + hoverShadow: string; }; // Neutral card colors so the action doesn't compete with the comment -// highlight color behind the selection. The values are hardcoded because they -// run inside opaque-origin sandbox iframes where the app's CSS variable tokens -// don't exist. +// highlight color behind the selection. The values mirror the app's gray +// tokens (gray-2 bg, gray-5 border, gray-4 hover) and are hardcoded because +// they run inside opaque-origin sandbox iframes where the app's CSS variable +// tokens don't exist. export const COMMENT_ACTION_BUTTON_THEMES: Record< CommentSurfaceTheme, CommentActionButtonTheme > = { light: { - background: "#ffffff", - color: "#1f2328", - border: "rgba(31, 35, 40, 0.16)", - hoverBackground: "#f5f5f4", + background: "#eceee8", + color: "#0d0d0d", + border: "#cbd0c3", + hoverBackground: "#d8dbd1", + shadow: "0 1px 2px rgba(0,0,0,0.08),0 4px 12px rgba(0,0,0,0.12)", + hoverShadow: "0 2px 4px rgba(0,0,0,0.1),0 6px 16px rgba(0,0,0,0.16)", }, dark: { - background: "#2b2b2e", - color: "#fafafa", - border: "rgba(255, 255, 255, 0.16)", - hoverBackground: "#3f3f43", + background: "#18181f", + color: "#e6e6e6", + border: "#2a2a37", + hoverBackground: "#24243e", + shadow: "0 1px 2px rgba(0,0,0,0.5),0 4px 12px rgba(0,0,0,0.5)", + hoverShadow: "0 2px 4px rgba(0,0,0,0.5),0 6px 16px rgba(0,0,0,0.65)", }, }; +// Bold-weight chat bubble (Phosphor ChatCircle), so the iframed action carries +// the same icon the in-app buttons render with @phosphor-icons/react. +export const COMMENT_ACTION_ICON_SVG = + ''; + // CSS variables let the host flip the theme of an already-running iframe // document with a `theme` message instead of rebuilding (and reloading) it. export function commentActionButtonCssVars(theme: CommentSurfaceTheme): string { const palette = COMMENT_ACTION_BUTTON_THEMES[theme]; - return `--ph-comment-action-bg:${palette.background};--ph-comment-action-fg:${palette.color};--ph-comment-action-border:${palette.border};--ph-comment-action-hover:${palette.hoverBackground};`; + return `--ph-comment-action-bg:${palette.background};--ph-comment-action-fg:${palette.color};--ph-comment-action-border:${palette.border};--ph-comment-action-hover:${palette.hoverBackground};--ph-comment-action-shadow:${palette.shadow};--ph-comment-action-hover-shadow:${palette.hoverShadow};`; } +// A neutral pill that reads as a card floating above the content: solid themed +// background (nothing bleeds through), visible hover feedback, pointer cursor. export function commentActionButtonCss(): string { - return `.ph-comment-action-button{position:fixed;z-index:2147483647;display:flex;align-items:center;gap:6px;height:34px;padding:0 13px;border:1px solid var(--ph-comment-action-border);border-radius:8px;background:var(--ph-comment-action-bg);color:var(--ph-comment-action-fg);font:500 13px/1 -apple-system,BlinkMacSystemFont,'Segoe UI',sans-serif;box-shadow:0 3px 12px rgba(0,0,0,.18);cursor:pointer}.ph-comment-action-button:hover{background:var(--ph-comment-action-hover)}`; + return `.ph-comment-action-button{position:fixed;z-index:2147483647;display:flex;align-items:center;gap:6px;height:30px;padding:0 12px;border:1px solid var(--ph-comment-action-border);border-radius:999px;background:var(--ph-comment-action-bg);color:var(--ph-comment-action-fg);font:600 12px/1 -apple-system,BlinkMacSystemFont,'Segoe UI',sans-serif;box-shadow:var(--ph-comment-action-shadow);cursor:pointer;user-select:none;transition:background-color .12s,box-shadow .12s}.ph-comment-action-button:hover{background:var(--ph-comment-action-hover);box-shadow:var(--ph-comment-action-hover-shadow)}.ph-comment-action-button:focus-visible{outline:2px solid var(--ph-comment-action-border);outline-offset:2px}.ph-comment-action-button svg{flex:none}`; } // Dependency-free: injected into iframe bridge scripts via .toString() and @@ -59,6 +73,38 @@ export function setCommentActionTheme( style.setProperty("--ph-comment-action-fg", palette.color); style.setProperty("--ph-comment-action-border", palette.border); style.setProperty("--ph-comment-action-hover", palette.hoverBackground); + style.setProperty("--ph-comment-action-shadow", palette.shadow); + style.setProperty("--ph-comment-action-hover-shadow", palette.hoverShadow); +} + +// Where the action sits relative to the selection's end line (Google Docs +// style): just right of the caret, vertically centered on the line. When the +// right edge has no room it drops below the end line instead, keeping its +// right edge at the caret; near the viewport bottom it flips above the line. +// `rect` is the selection's end line, `bounds` the viewport/container, `action` +// the action element's measured size. +export function computeCommentActionPlacement( + rect: { top: number; right: number; bottom: number }, + bounds: { width: number; height: number }, + action: { width: number; height: number }, +): { top: number; left: number } { + const MARGIN = 8; + const lineMiddle = rect.top + (rect.bottom - rect.top) / 2; + let left = rect.right + MARGIN; + let top = lineMiddle - action.height / 2; + if (left + action.width > bounds.width - MARGIN) { + left = Math.max(rect.right - action.width, MARGIN); + top = rect.bottom + 6; + if (top + action.height > bounds.height - MARGIN) { + top = rect.top - action.height - 6; + } + } + const maxLeft = Math.max(bounds.width - action.width - MARGIN, MARGIN); + const maxTop = Math.max(bounds.height - action.height - MARGIN, MARGIN); + return { + left: Math.min(Math.max(left, MARGIN), maxLeft), + top: Math.min(Math.max(top, MARGIN), maxTop), + }; } export type SelectionSettleGateCallbacks = { From 259d18e6ad1a73cdc96d9541b7755f2de6b9e6b1 Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Fri, 7 Aug 2026 12:03:52 +0200 Subject: [PATCH 3/7] fix(comments): anchor selection action to the last selected line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Range spanning block elements reports the wrapper boxes (paragraph, blockquote, list) alongside the text line boxes, and a wrapper can come last in DOM order. Taking the last box — or the bounding box — therefore anchored the action to the wrapper's right edge, which is why a multi-line selection put the Comment button far from where the selection ended. The shared commentActionAnchorRect() now keeps only leaf boxes (those enclosing no other box) and picks the visually lowest, right-most one: the end of the last selected line. All three range-based surfaces use it. The action is also its own component now, SelectionCommentActionButton, instead of a Quill button. It renders the same stylesheet the sandboxed iframes inject, so markdown artifacts, HTML artifacts, canvases and the code editor share one look and cannot drift. Hover changes the background only — the shadow no longer jumps — and the palette is explicit per theme rather than inherited from ambient tokens. Generated-By: PostHog Code Task-Id: e93165cd-6893-45dd-8fa8-4e97299f5f06 --- .../canvas/freeform/sandboxRuntime.ts | 9 ++- .../components/SelectionCommentOverlay.tsx | 23 +++--- .../components/ArtifactTextAnnotations.tsx | 13 +-- .../SelectionCommentActionButton.tsx | 79 +++++++++++++++++++ .../artifactHtmlCommentBridge.test.ts | 4 +- .../components/artifactHtmlCommentBridge.ts | 4 +- .../components/selectionCommentAction.test.ts | 38 ++++++++- .../components/selectionCommentAction.ts | 76 ++++++++++++++---- 8 files changed, 204 insertions(+), 42 deletions(-) create mode 100644 products/desktop/packages/ui/src/features/sessions/components/SelectionCommentActionButton.tsx diff --git a/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts b/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts index 75d5ba5eed0b..751696acc764 100644 --- a/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts +++ b/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts @@ -6,7 +6,10 @@ import { FREEFORM_QUILL_CSS_URLS, } from "@posthog/core/canvas/freeformWhitelist"; import { resolveTextCommentAnchor } from "@posthog/core/comments/anchors"; -import { installSelectionSettleGate } from "@posthog/ui/features/sessions/components/selectionCommentAction"; +import { + commentActionAnchorRect, + installSelectionSettleGate, +} from "@posthog/ui/features/sessions/components/selectionCommentAction"; // Builds the HTML document loaded into the freeform-canvas sandbox iframe. // @@ -287,6 +290,7 @@ export function buildSandboxDocument( true, ); + const selectionAnchorRect = ${commentActionAnchorRect.toString()}; const clearTextSelection = () => post({ type: "text-selection-cleared" }); let selectionTimer = 0; const reportTextSelection = () => { @@ -318,8 +322,7 @@ export function buildSandboxDocument( } // The END line's rect, so the host anchors the comment action where the // pointer was released rather than at the whole-range bounding box. - const clientRects = range.getClientRects(); - const rect = clientRects.length ? clientRects[clientRects.length - 1] : range.getBoundingClientRect(); + const rect = selectionAnchorRect(range.getClientRects(), range.getBoundingClientRect()); post({ type: "text-selection", selection: { diff --git a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx index 84066452fb8d..3ea7490b6c61 100644 --- a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx +++ b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx @@ -3,6 +3,7 @@ import type { UserBasic } from "@posthog/shared/domain-types"; import type { EditorSelection } from "@posthog/ui/features/code-editor/components/CodeMirrorEditor"; import { CommentAnnotation } from "@posthog/ui/features/code-review/components/CommentAnnotation"; import { CommentComposer } from "@posthog/ui/features/sessions/components/CommentComposer"; +import { SelectionCommentActionButton } from "@posthog/ui/features/sessions/components/SelectionCommentActionButton"; import { computeCommentActionPlacement } from "@posthog/ui/features/sessions/components/selectionCommentAction"; import { Tooltip } from "@posthog/ui/primitives/Tooltip"; import { useCallback, useEffect, useState } from "react"; @@ -111,7 +112,7 @@ function SelectionComposerCard({ const [submitting, setSubmitting] = useState(false); const actionSize = expanded ? { width: Math.min(420, window.innerWidth * 0.8), height: 180 } - : { width: showActionText ? 116 : 30, height: 30 }; + : { width: showActionText ? 104 : 28, height: 28 }; const style = computeCommentActionPlacement( { top: anchor.top, right: anchor.left, bottom: anchor.bottom }, { width: window.innerWidth, height: window.innerHeight }, @@ -135,27 +136,21 @@ function SelectionComposerCard({ if (!expanded) { const action = ( - + ); // The text button names itself; only the icon-only variant needs a label // on hover. diff --git a/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx b/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx index 39aa5291507b..8edf1401d4de 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx @@ -20,7 +20,10 @@ import { type HighlightResolution, readCommentContext, } from "./commentViewTypes"; -import { installSelectionSettleGate } from "./selectionCommentAction"; +import { + commentActionAnchorRect, + installSelectionSettleGate, +} from "./selectionCommentAction"; type HighlightRect = { id: string; @@ -324,10 +327,10 @@ export function ArtifactTextAnnotations({ ); if (!anchor) return; // Anchor to the selection's end line, where the pointer was released. - const clientRects = range.getClientRects?.() ?? []; - const endRect = clientRects.length - ? clientRects[clientRects.length - 1] - : range.getBoundingClientRect(); + const endRect = commentActionAnchorRect( + range.getClientRects?.() ?? [], + range.getBoundingClientRect(), + ); setPendingAnchor(anchor); setSelection({ text: anchor.quote, diff --git a/products/desktop/packages/ui/src/features/sessions/components/SelectionCommentActionButton.tsx b/products/desktop/packages/ui/src/features/sessions/components/SelectionCommentActionButton.tsx new file mode 100644 index 000000000000..4b9baaa4f4fb --- /dev/null +++ b/products/desktop/packages/ui/src/features/sessions/components/SelectionCommentActionButton.tsx @@ -0,0 +1,79 @@ +import { useThemeStore } from "@posthog/ui/shell/themeStore"; +import type { ComponentPropsWithoutRef, CSSProperties, ReactNode } from "react"; +import { forwardRef, useEffect } from "react"; +import { + COMMENT_ACTION_BUTTON_THEMES, + type CommentSurfaceTheme, + commentActionButtonCss, +} from "./selectionCommentAction"; + +const STYLE_ELEMENT_ID = "ph-comment-action-styles"; + +// The app renders the same stylesheet the sandboxed iframes inject, so the +// action cannot drift between markdown artifacts, HTML artifacts and canvases. +function useCommentActionStyles(): void { + useEffect(() => { + if (document.getElementById(STYLE_ELEMENT_ID)) return; + const style = document.createElement("style"); + style.id = STYLE_ELEMENT_ID; + style.textContent = commentActionButtonCss(); + document.head.appendChild(style); + }, []); +} + +function paletteVars(theme: CommentSurfaceTheme): CSSProperties { + const palette = COMMENT_ACTION_BUTTON_THEMES[theme]; + return { + "--ph-comment-action-bg": palette.background, + "--ph-comment-action-fg": palette.color, + "--ph-comment-action-border": palette.border, + "--ph-comment-action-hover": palette.hoverBackground, + "--ph-comment-action-shadow": palette.shadow, + } as CSSProperties; +} + +interface SelectionCommentActionButtonProps + extends Omit, "style" | "className"> { + label: string; + /** Icon-only trigger: a square button with no visible label. */ + iconOnly?: boolean; + children: ReactNode; + position: { top: number; left: number }; +} + +/** + * The floating action offered next to a text selection. Deliberately not a + * Quill button: it has to look identical inside sandboxed artifact iframes, + * which cannot load the app's stylesheets, so both sides share one class and + * one palette. + */ +export const SelectionCommentActionButton = forwardRef< + HTMLButtonElement, + SelectionCommentActionButtonProps +>(function SelectionCommentActionButton( + { label, iconOnly = false, children, position, ...buttonProps }, + ref, +) { + useCommentActionStyles(); + const theme = useThemeStore( + (state): CommentSurfaceTheme => (state.isDarkMode ? "dark" : "light"), + ); + return ( + + ); +}); diff --git a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts index 852c7b08adbf..25830e06a217 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts @@ -78,9 +78,9 @@ describe("artifactHtmlCommentBridge", () => { expect(actionButton(dom)?.style.display).toBe("flex"); expect(actionButton(dom)?.textContent).toBe("Comment"); // Anchored right of the selection end (110 + 8), centered on the end - // line (50 - 15). + // line (50 - 14). expect(actionButton(dom)?.style.left).toBe("118px"); - expect(actionButton(dom)?.style.top).toBe("35px"); + expect(actionButton(dom)?.style.top).toBe("36px"); dom.window.close(); }); diff --git a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts index 1cc5d6739cbd..fd67c88b466b 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts @@ -3,6 +3,7 @@ import { COMMENT_ACTION_BUTTON_THEMES, COMMENT_ACTION_ICON_SVG, type CommentSurfaceTheme, + commentActionAnchorRect, commentActionButtonCss, commentActionButtonCssVars, computeCommentActionPlacement, @@ -40,12 +41,13 @@ var THEMES=${JSON.stringify(COMMENT_ACTION_BUTTON_THEMES)}; var ACTION_ICON=${JSON.stringify(COMMENT_ACTION_ICON_SVG)}; var setActionTheme=${setCommentActionTheme.toString()}; var placeAction=${computeCommentActionPlacement.toString()}; +var anchorRect=${commentActionAnchorRect.toString()}; var resolveAnchor=${resolveTextCommentAnchor.toString()}; function resolve(all,a){return resolveAnchor(all,a)} function style(){var s=document.createElement("style");s.setAttribute("data-posthog-artifact-comments","");s.textContent="::highlight(posthog-artifact-comment){background:rgba(250,204,21,.32);color:inherit}::highlight(posthog-artifact-comment-active){background:rgba(250,204,21,.48);color:inherit}:root{${commentActionButtonCssVars(theme)}}${commentActionButtonCss()}";(document.head||document.documentElement).appendChild(s)} function hide(){if(state.button)state.button.style.display="none"} function button(){if(state.button&&state.button.isConnected)return state.button;var b=document.createElement("button");b.type="button";b.className="ph-comment-action-button";b.setAttribute("data-selection-comment-overlay","");b.setAttribute("aria-label","Comment");b.innerHTML=ACTION_ICON+"Comment";b.style.display="none";b.addEventListener("mousedown",function(e){e.preventDefault();e.stopPropagation();var sel=window.getSelection();if(!sel||sel.isCollapsed||!sel.rangeCount)return hide();var range=sel.getRangeAt(0),anchor=makeAnchor(range);if(!anchor)return hide();var r=range.getBoundingClientRect(),br=b.getBoundingClientRect();send("selection",{anchor:anchor,rect:{top:r.top,left:r.left,right:r.right,bottom:r.bottom,width:r.width,height:r.height},triggerRect:{top:br.top,left:br.left,right:br.right,bottom:br.bottom,width:br.width,height:br.height}});hide();sel.removeAllRanges()});document.documentElement.appendChild(b);state.button=b;return b;} -function positionButton(){var sel=window.getSelection();if(!sel||sel.isCollapsed||!sel.rangeCount)return hide();var value=sel.toString().trim();if(value.length<2||value.length>10000)return hide();var range=sel.getRangeAt(0);var rects=range.getClientRects?range.getClientRects():[];var r=rects.length?rects[rects.length-1]:range.getBoundingClientRect();if(!r||(r.width===0&&r.height===0))return hide();var b=button();var box=b.getBoundingClientRect();var pos=placeAction({top:r.top,right:r.right,bottom:r.bottom},{width:innerWidth,height:innerHeight},{width:box.width||120,height:box.height||30});b.style.left=pos.left+"px";b.style.top=pos.top+"px";b.style.display="flex"} +function positionButton(){var sel=window.getSelection();if(!sel||sel.isCollapsed||!sel.rangeCount)return hide();var value=sel.toString().trim();if(value.length<2||value.length>10000)return hide();var range=sel.getRangeAt(0);var r=anchorRect(range.getClientRects?range.getClientRects():[],range.getBoundingClientRect());if(!r||(r.width===0&&r.height===0))return hide();var b=button();var box=b.getBoundingClientRect();var pos=placeAction({top:r.top,right:r.right,bottom:r.bottom},{width:innerWidth,height:innerHeight},{width:box.width||104,height:box.height||28});b.style.left=pos.left+"px";b.style.top=pos.top+"px";b.style.display="flex"} function selectionChanged(){clearTimeout(state.timer);state.timer=setTimeout(positionButton,80)} function render(items){state.items=items||[];state.entries=[];var normal=supportsHighlights?new Highlight():null,active=supportsHighlights?new Highlight():null,resolutions=[],index=textIndex();state.items.forEach(function(item){var hit=resolve(index.text,item.anchor);if(!hit){resolutions.push({id:item.id,status:"orphaned"});return}var range=rangeAt(index,hit.start,hit.end);if(!range){resolutions.push({id:item.id,status:"orphaned"});return}state.entries.push({id:item.id,range:range,active:!!item.active});resolutions.push({id:item.id,status:hit.status});if(supportsHighlights){if(item.active)active.add(range);else normal.add(range)}});if(supportsHighlights){CSS.highlights.set("posthog-artifact-comment",normal);CSS.highlights.set("posthog-artifact-comment-active",active)}send("resolutions",{items:resolutions})} function locate(id){for(var i=0;i { + const box = (left: number, top: number, right: number, bottom: number) => ({ + left, + top, + right, + bottom, + width: right - left, + height: bottom - top, + }); + const fallback = box(0, 0, 10, 10); + + it("ignores the wrapper box a multi-line selection reports, anchoring to the last line", () => { + const firstLine = box(600, 340, 1213, 364); + const lastLine = box(628, 447, 866, 470); + // A blockquote/paragraph wrapper encloses every line box and, in DOM order, + // can come last — the case that threw the action to the far right. + const wrapper = box(578, 310, 1274, 596); + + expect( + commentActionAnchorRect([firstLine, lastLine, wrapper], fallback), + ).toBe(lastLine); + }); + + it("takes the right-most box when the selection ends on a line split across elements", () => { + const plain = box(100, 40, 180, 60); + const bolded = box(180, 40, 240, 60); + + expect(commentActionAnchorRect([bolded, plain], fallback)).toBe(bolded); + }); + + it("falls back when the range reports no boxes at all", () => { + expect(commentActionAnchorRect([], fallback)).toBe(fallback); + expect(commentActionAnchorRect([box(5, 5, 5, 5)], fallback)).toBe(fallback); + }); +}); + describe("computeCommentActionPlacement", () => { const bounds = { width: 1000, height: 700 }; const action = { width: 120, height: 30 }; diff --git a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts index d2f79452eca8..a87c47e4c4ee 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts @@ -12,7 +12,6 @@ type CommentActionButtonTheme = { border: string; hoverBackground: string; shadow: string; - hoverShadow: string; }; // Neutral card colors so the action doesn't compete with the comment @@ -25,20 +24,18 @@ export const COMMENT_ACTION_BUTTON_THEMES: Record< CommentActionButtonTheme > = { light: { - background: "#eceee8", - color: "#0d0d0d", + background: "#ffffff", + color: "#1b1d1a", border: "#cbd0c3", - hoverBackground: "#d8dbd1", - shadow: "0 1px 2px rgba(0,0,0,0.08),0 4px 12px rgba(0,0,0,0.12)", - hoverShadow: "0 2px 4px rgba(0,0,0,0.1),0 6px 16px rgba(0,0,0,0.16)", + hoverBackground: "#eceee8", + shadow: "0 1px 2px rgba(0,0,0,0.10),0 2px 6px rgba(0,0,0,0.08)", }, dark: { - background: "#18181f", + background: "#24242e", color: "#e6e6e6", - border: "#2a2a37", - hoverBackground: "#24243e", - shadow: "0 1px 2px rgba(0,0,0,0.5),0 4px 12px rgba(0,0,0,0.5)", - hoverShadow: "0 2px 4px rgba(0,0,0,0.5),0 6px 16px rgba(0,0,0,0.65)", + border: "#3a3a4c", + hoverBackground: "#31313f", + shadow: "0 1px 2px rgba(0,0,0,0.45),0 2px 6px rgba(0,0,0,0.35)", }, }; @@ -51,13 +48,16 @@ export const COMMENT_ACTION_ICON_SVG = // document with a `theme` message instead of rebuilding (and reloading) it. export function commentActionButtonCssVars(theme: CommentSurfaceTheme): string { const palette = COMMENT_ACTION_BUTTON_THEMES[theme]; - return `--ph-comment-action-bg:${palette.background};--ph-comment-action-fg:${palette.color};--ph-comment-action-border:${palette.border};--ph-comment-action-hover:${palette.hoverBackground};--ph-comment-action-shadow:${palette.shadow};--ph-comment-action-hover-shadow:${palette.hoverShadow};`; + return `--ph-comment-action-bg:${palette.background};--ph-comment-action-fg:${palette.color};--ph-comment-action-border:${palette.border};--ph-comment-action-hover:${palette.hoverBackground};--ph-comment-action-shadow:${palette.shadow};`; } -// A neutral pill that reads as a card floating above the content: solid themed -// background (nothing bleeds through), visible hover feedback, pointer cursor. +// The one comment action look, shared by every surface: the app renders this +// class directly and the sandboxed iframes inject the same rules, so a markdown +// artifact, an HTML artifact and a canvas cannot drift apart. Solid themed +// background (nothing bleeds through), background-only hover (no shadow jump), +// pointer cursor. export function commentActionButtonCss(): string { - return `.ph-comment-action-button{position:fixed;z-index:2147483647;display:flex;align-items:center;gap:6px;height:30px;padding:0 12px;border:1px solid var(--ph-comment-action-border);border-radius:999px;background:var(--ph-comment-action-bg);color:var(--ph-comment-action-fg);font:600 12px/1 -apple-system,BlinkMacSystemFont,'Segoe UI',sans-serif;box-shadow:var(--ph-comment-action-shadow);cursor:pointer;user-select:none;transition:background-color .12s,box-shadow .12s}.ph-comment-action-button:hover{background:var(--ph-comment-action-hover);box-shadow:var(--ph-comment-action-hover-shadow)}.ph-comment-action-button:focus-visible{outline:2px solid var(--ph-comment-action-border);outline-offset:2px}.ph-comment-action-button svg{flex:none}`; + return `.ph-comment-action-button{position:fixed;z-index:2147483647;display:inline-flex;align-items:center;gap:6px;height:28px;padding:0 10px;margin:0;border:1px solid var(--ph-comment-action-border);border-radius:8px;background:var(--ph-comment-action-bg);color:var(--ph-comment-action-fg);font:500 12px/1 -apple-system,BlinkMacSystemFont,'Segoe UI',Roboto,sans-serif;box-shadow:var(--ph-comment-action-shadow);cursor:pointer;user-select:none;-webkit-user-select:none;transition:background-color .1s ease}.ph-comment-action-button:hover{background:var(--ph-comment-action-hover)}.ph-comment-action-button:active{background:var(--ph-comment-action-hover)}.ph-comment-action-button:focus-visible{outline:2px solid var(--ph-comment-action-border);outline-offset:2px}.ph-comment-action-button--icon{width:28px;padding:0;justify-content:center}.ph-comment-action-button svg{flex:none;display:block}`; } // Dependency-free: injected into iframe bridge scripts via .toString() and @@ -74,7 +74,51 @@ export function setCommentActionTheme( style.setProperty("--ph-comment-action-border", palette.border); style.setProperty("--ph-comment-action-hover", palette.hoverBackground); style.setProperty("--ph-comment-action-shadow", palette.shadow); - style.setProperty("--ph-comment-action-hover-shadow", palette.hoverShadow); +} + +type CommentActionBox = { + top: number; + right: number; + bottom: number; + left: number; + width: number; + height: number; +}; + +// Which box the action anchors to. A Range spanning block elements reports the +// wrapper boxes (paragraph, blockquote, list) alongside the text line boxes, so +// neither the bounding box nor the last entry marks where the user stopped +// selecting. Keep the leaf boxes — those that enclose no other box — and take +// the visually lowest, then right-most one: the end of the last selected line. +export function commentActionAnchorRect( + rects: ArrayLike, + fallback: T, +): T { + const boxes: T[] = []; + for (let index = 0; index < rects.length; index++) { + const rect = rects[index]; + if (rect.width > 0 || rect.height > 0) boxes.push(rect); + } + if (boxes.length === 0) return fallback; + const EPSILON = 0.5; + const area = (box: T) => box.width * box.height; + const encloses = (outer: T, inner: T) => + area(outer) > area(inner) + 1 && + outer.left <= inner.left + EPSILON && + outer.right >= inner.right - EPSILON && + outer.top <= inner.top + EPSILON && + outer.bottom >= inner.bottom - EPSILON; + const leaves = boxes.filter( + (box) => !boxes.some((other) => other !== box && encloses(box, other)), + ); + const pool = leaves.length > 0 ? leaves : boxes; + let best = pool[0]; + for (const box of pool) { + const lower = box.bottom > best.bottom + EPSILON; + const sameLine = Math.abs(box.bottom - best.bottom) <= EPSILON; + if (lower || (sameLine && box.right > best.right)) best = box; + } + return best; } // Where the action sits relative to the selection's end line (Google Docs From aa212b7a35dbc72236f6b7de6b7fca93fd21150c Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Fri, 7 Aug 2026 12:14:40 +0200 Subject: [PATCH 4/7] fix(comments): show the selection action only once the selection settles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate started on pointerdown and reported synchronously on pointerup, which let the action appear mid-selection. Reworked around the pattern the established editor toolbars use (Floating UI's text-selection example, TipTap's BubbleMenu, and the common selectstart/pointerup idiom): - selectstart also starts the gate, so drags whose pointerdown never reaches the document still hide the action. - The report waits two animation frames after pointerup. Browsers commit the selection after the pointerup handler runs, so reading it synchronously returned the mid-gesture range — which both showed the action early and anchored it to the wrong place. - pointercancel now cancels instead of settling. Trackpads fire it mid-drag, which popped the action up while the user was still selecting. - Keyboard selections are held until keyup rather than reported on every Shift+Arrow tick; a plain letter no longer reads as select-all. - Secondary mouse buttons no longer start a gesture. Scrolling the artifact container now re-anchors the action to the live selection instead of leaving it at stale viewport coordinates. Verified with a real mouse drag in headless Chromium: the action is absent at six sampled points across the drag, appears on release 8px right of the selection end, with its vertical center within a pixel of the end line's. Generated-By: PostHog Code Task-Id: e93165cd-6893-45dd-8fa8-4e97299f5f06 --- .../components/ArtifactTextAnnotations.tsx | 4 + .../components/selectionCommentAction.test.ts | 131 ++++++++++++++--- .../components/selectionCommentAction.ts | 139 ++++++++++++++---- 3 files changed, 218 insertions(+), 56 deletions(-) diff --git a/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx b/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx index 8edf1401d4de..77b10591b4fb 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx @@ -353,8 +353,12 @@ export function ArtifactTextAnnotations({ onIdleSelectionChange: scheduleUpdate, onGestureCancel: clearOverlay, }); + // Scrolling moves the selection but not the fixed-position action, so + // re-anchor it to the live selection instead of leaving it behind. + container.addEventListener("scroll", scheduleUpdate, { passive: true }); return () => { cancelAnimationFrame(frame); + container.removeEventListener("scroll", scheduleUpdate); removeGate(); }; }, [clearOverlay, containerRef, rootRef]); diff --git a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.test.ts b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.test.ts index c2c22c364ea3..319788d30cd4 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.test.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.test.ts @@ -28,8 +28,14 @@ function changeSelection(): void { document.dispatchEvent(new Event("selectionchange")); } +function settleFrames(): Promise { + return new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }); +} + describe("selectionCommentAction", () => { - it("suppresses selection reporting while the user drags, then reports the settled selection", () => { + it("suppresses selection reporting while the user drags, then reports the settled selection", async () => { const callbacks = { onGestureStart: vi.fn(), onSelectionSettled: vi.fn(), @@ -46,11 +52,35 @@ describe("selectionCommentAction", () => { expect(callbacks.onIdleSelectionChange).not.toHaveBeenCalled(); release(target); + // The browser commits the range after pointerup, so the report waits. + expect(callbacks.onSelectionSettled).not.toHaveBeenCalled(); + await settleFrames(); + expect(callbacks.onSelectionSettled).toHaveBeenCalledTimes(1); + remove(); + }); + + it("hides on selectstart, for drags whose pointerdown never reached the document", async () => { + const callbacks = { + onGestureStart: vi.fn(), + onIdleSelectionChange: vi.fn(), + onSelectionSettled: vi.fn(), + } satisfies SelectionSettleGateCallbacks; + const remove = installSelectionSettleGate(document, callbacks); + const target = eventTarget(); + + target.dispatchEvent(new Event("selectstart", { bubbles: true })); + expect(callbacks.onGestureStart).toHaveBeenCalledTimes(1); + + changeSelection(); + expect(callbacks.onIdleSelectionChange).not.toHaveBeenCalled(); + + release(target); + await settleFrames(); expect(callbacks.onSelectionSettled).toHaveBeenCalledTimes(1); remove(); }); - it("reports immediately when the selection changes without a drag", () => { + it("reports immediately when the selection changes without a gesture", () => { const callbacks = { onIdleSelectionChange: vi.fn() }; const remove = installSelectionSettleGate(document, callbacks); @@ -80,7 +110,19 @@ describe("selectionCommentAction", () => { overlay.remove(); }); - it("cancels the gesture on window blur so a release outside the window can't stick the action hidden", () => { + it("ignores secondary buttons, which open menus instead of selecting", () => { + const callbacks = { onGestureStart: vi.fn() }; + const remove = installSelectionSettleGate(document, callbacks); + const target = eventTarget(); + + target.dispatchEvent( + new PointerEvent("pointerdown", { bubbles: true, button: 2 }), + ); + expect(callbacks.onGestureStart).not.toHaveBeenCalled(); + remove(); + }); + + it("cancels the gesture on window blur so a release outside the window can't stick the action hidden", async () => { const callbacks = { onGestureCancel: vi.fn(), onSelectionSettled: vi.fn(), @@ -93,24 +135,65 @@ describe("selectionCommentAction", () => { expect(callbacks.onGestureCancel).toHaveBeenCalledTimes(1); release(target); + await settleFrames(); expect(callbacks.onSelectionSettled).not.toHaveBeenCalled(); - - changeSelection(); - expect(callbacks.onGestureCancel).toHaveBeenCalledTimes(1); remove(); }); - it("settles on pointercancel so touch gesture interruptions still show the action", () => { - const callbacks = { onSelectionSettled: vi.fn() }; + it("cancels on pointercancel, which trackpads fire mid-drag", async () => { + const callbacks = { + onSelectionSettled: vi.fn(), + onGestureCancel: vi.fn(), + } satisfies SelectionSettleGateCallbacks; const remove = installSelectionSettleGate(document, callbacks); const target = eventTarget(); press(target); target.dispatchEvent(new PointerEvent("pointercancel", { bubbles: true })); + await settleFrames(); + expect(callbacks.onGestureCancel).toHaveBeenCalledTimes(1); + expect(callbacks.onSelectionSettled).not.toHaveBeenCalled(); + remove(); + }); + + it("holds keyboard selections until the key is released", async () => { + const callbacks = { + onGestureStart: vi.fn(), + onSelectionSettled: vi.fn(), + onIdleSelectionChange: vi.fn(), + } satisfies SelectionSettleGateCallbacks; + const remove = installSelectionSettleGate(document, callbacks); + + document.dispatchEvent( + new KeyboardEvent("keydown", { key: "ArrowRight", bubbles: true }), + ); + changeSelection(); + expect(callbacks.onIdleSelectionChange).not.toHaveBeenCalled(); + + document.dispatchEvent( + new KeyboardEvent("keyup", { key: "ArrowRight", bubbles: true }), + ); + await settleFrames(); expect(callbacks.onSelectionSettled).toHaveBeenCalledTimes(1); remove(); }); + it("treats a plain letter as typing, not as select-all", () => { + const callbacks = { onGestureStart: vi.fn() }; + const remove = installSelectionSettleGate(document, callbacks); + + document.dispatchEvent( + new KeyboardEvent("keydown", { key: "a", bubbles: true }), + ); + expect(callbacks.onGestureStart).not.toHaveBeenCalled(); + + document.dispatchEvent( + new KeyboardEvent("keydown", { key: "a", metaKey: true, bubbles: true }), + ); + expect(callbacks.onGestureStart).toHaveBeenCalledTimes(1); + remove(); + }); + it("applies every theme variable the action button styles reference", () => { for (const theme of ["light", "dark"] as const) { setCommentActionTheme(theme, COMMENT_ACTION_BUTTON_THEMES); @@ -145,22 +228,6 @@ describe("selectionCommentAction", () => { }); }); -function cssVarField( - variable: string, -): keyof (typeof COMMENT_ACTION_BUTTON_THEMES)["light"] { - const fields: Record< - string, - keyof (typeof COMMENT_ACTION_BUTTON_THEMES)["light"] - > = { - "--ph-comment-action-bg": "background", - "--ph-comment-action-fg": "color", - "--ph-comment-action-border": "border", - "--ph-comment-action-hover": "hoverBackground", - "--ph-comment-action-shadow": "shadow", - }; - return fields[variable]; -} - describe("commentActionAnchorRect", () => { const box = (left: number, top: number, right: number, bottom: number) => ({ left, @@ -241,3 +308,19 @@ describe("computeCommentActionPlacement", () => { ).toEqual({ top: 8, left: 10 }); }); }); + +function cssVarField( + variable: string, +): keyof (typeof COMMENT_ACTION_BUTTON_THEMES)["light"] { + const fields: Record< + string, + keyof (typeof COMMENT_ACTION_BUTTON_THEMES)["light"] + > = { + "--ph-comment-action-bg": "background", + "--ph-comment-action-fg": "color", + "--ph-comment-action-border": "border", + "--ph-comment-action-hover": "hoverBackground", + "--ph-comment-action-shadow": "shadow", + }; + return fields[variable]; +} diff --git a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts index a87c47e4c4ee..6ea30cd82806 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts @@ -152,61 +152,136 @@ export function computeCommentActionPlacement( } export type SelectionSettleGateCallbacks = { - // Pointer pressed outside the action UI; hide any visual anchored to the - // selection. + // A selection gesture started outside the action UI; hide any visual + // anchored to the selection. onGestureStart?: () => void; - // Pointer released after a gesture; re-read the selection, it is final. + // The gesture finished and the browser has committed the range; re-read the + // selection, it is final. onSelectionSettled?: () => void; - // Selection changed with no pointer down (keyboard, programmatic). + // Selection changed outside a gesture (programmatic, or a click that + // collapsed it). onIdleSelectionChange?: () => void; - // Gesture interrupted before pointerup (window blur). + // Gesture interrupted before it finished (pointercancel, window blur). onGestureCancel?: () => void; }; -// While the user drag-selects, the range keeps moving, so an action anchored -// to the live selection visibly chases the cursor. The gate suppresses -// selection reporting during the drag and reports the final range on -// pointerup. Clicks inside the action UI are excluded so pressing the action -// itself doesn't restart a gesture. +// While the user selects, the range keeps moving, so an action anchored to the +// live selection chases the cursor. The gate reports only settled selections, +// following the pattern the editor toolbars converged on: +// +// selectstart / pointerdown / selection keydown -> hide +// selectionchange -> ignore while gesturing +// pointerup / selection keyup -> report, two frames later +// pointercancel / blur -> cancel +// +// The two frames matter: the browser commits the selection AFTER the pointerup +// handler runs, so reading it synchronously returns the mid-gesture range and +// anchors the action to the wrong place. Presses inside the action UI are +// ignored so using the action can't start a gesture. export function installSelectionSettleGate( doc: Document, callbacks: SelectionSettleGateCallbacks, ): () => void { - let dragging = false; - const onPointerDown = (event: Event) => { - if ( - event.target instanceof Element && - event.target.closest("[data-selection-comment-overlay]") - ) { - return; + const view = doc.defaultView; + let selecting = false; + let frame = 0; + + // Keys that move or extend a selection. "a" only counts with a modifier, so + // typing the letter doesn't read as select-all. + const isSelectionKey = (event: KeyboardEvent) => { + if (event.key === "a" || event.key === "A") { + return event.metaKey || event.ctrlKey; } - dragging = true; - callbacks.onGestureStart?.(); + return ( + event.key === "Shift" || + event.key === "Home" || + event.key === "End" || + event.key === "PageUp" || + event.key === "PageDown" || + event.key.startsWith("Arrow") + ); + }; + + const cancelFrame = () => { + if (frame && view?.cancelAnimationFrame) view.cancelAnimationFrame(frame); + frame = 0; }; const settle = () => { - if (!dragging) return; - dragging = false; - callbacks.onSelectionSettled?.(); + cancelFrame(); + const request = view?.requestAnimationFrame; + if (!request) { + callbacks.onSelectionSettled?.(); + return; + } + frame = request.call(view, () => { + frame = request.call(view, () => { + frame = 0; + callbacks.onSelectionSettled?.(); + }); + }); + }; + const inActionUi = (target: EventTarget | null) => + target instanceof Element && + !!target.closest("[data-selection-comment-overlay]"); + const startGesture = () => { + selecting = true; + cancelFrame(); + callbacks.onGestureStart?.(); }; - const cancel = () => { - if (!dragging) return; - dragging = false; + const cancelGesture = () => { + if (!selecting) return; + selecting = false; + cancelFrame(); callbacks.onGestureCancel?.(); }; + + const onPointerDown = (event: Event) => { + // Secondary buttons open menus; they don't select. + if (event instanceof MouseEvent && event.button > 0) return; + if (inActionUi(event.target)) return; + startGesture(); + }; + // Catches drags whose pointerdown we never saw, and keyboard selections. + const onSelectStart = (event: Event) => { + if (inActionUi(event.target)) return; + startGesture(); + }; + const onPointerUp = (event: Event) => { + if (event instanceof MouseEvent && event.button > 0) return; + if (!selecting) return; + selecting = false; + settle(); + }; + const onKeyDown = (event: KeyboardEvent) => { + if (isSelectionKey(event)) startGesture(); + }; + const onKeyUp = (event: KeyboardEvent) => { + if (!selecting || !isSelectionKey(event)) return; + selecting = false; + settle(); + }; const onSelectionChange = () => { - if (dragging) return; + if (selecting) return; callbacks.onIdleSelectionChange?.(); }; + doc.addEventListener("pointerdown", onPointerDown, true); - doc.addEventListener("pointerup", settle, true); - doc.addEventListener("pointercancel", settle, true); + doc.addEventListener("selectstart", onSelectStart, true); + doc.addEventListener("pointerup", onPointerUp, true); + doc.addEventListener("pointercancel", cancelGesture, true); + doc.addEventListener("keydown", onKeyDown, true); + doc.addEventListener("keyup", onKeyUp, true); doc.addEventListener("selectionchange", onSelectionChange); - doc.defaultView?.addEventListener("blur", cancel); + view?.addEventListener("blur", cancelGesture); return () => { + cancelFrame(); doc.removeEventListener("pointerdown", onPointerDown, true); - doc.removeEventListener("pointerup", settle, true); - doc.removeEventListener("pointercancel", settle, true); + doc.removeEventListener("selectstart", onSelectStart, true); + doc.removeEventListener("pointerup", onPointerUp, true); + doc.removeEventListener("pointercancel", cancelGesture, true); + doc.removeEventListener("keydown", onKeyDown, true); + doc.removeEventListener("keyup", onKeyUp, true); doc.removeEventListener("selectionchange", onSelectionChange); - doc.defaultView?.removeEventListener("blur", cancel); + view?.removeEventListener("blur", cancelGesture); }; } From 0ebde75a065e13d32cb4d5a24bbfa2d172556daf Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Fri, 7 Aug 2026 12:57:04 +0200 Subject: [PATCH 5/7] fix(comments): settle the selection action in published canvases too A published canvas runs the runtime baked into its artifact by the cloud builder, not the desktop sandbox document, so it never picked up the settle gate or the end-line anchor: the action tracked the cursor mid-drag and anchored to the whole-range bounding box. Port both into build.mjs, cancel a pending report when a new gesture starts (on both runtimes), and drive the emitted artifact runtime through a simulated drag in the builder tests, replacing the string pins that had frozen the old selectionchange-only behavior. Already-built canvases keep the old runtime until they are rebuilt. Generated-By: PostHog Code Task-Id: 17b5c25a-9f90-4267-a486-15c4debd3074 --- .../backend/tests/test_cloud_builder.py | 129 ++++++++++++++-- .../canvas/packages/canvas_builder/build.mjs | 143 +++++++++++++++++- .../canvas/freeform/sandboxRuntime.ts | 10 +- 3 files changed, 267 insertions(+), 15 deletions(-) diff --git a/products/canvas/backend/tests/test_cloud_builder.py b/products/canvas/backend/tests/test_cloud_builder.py index 926eb2f0178c..64702296203b 100644 --- a/products/canvas/backend/tests/test_cloud_builder.py +++ b/products/canvas/backend/tests/test_cloud_builder.py @@ -103,8 +103,6 @@ def test_runtime_uses_the_document_bound_message_port(self) -> None: self.assertIn("event.preventDefault();event.stopPropagation()", runtime) self.assertIn("if(!items.length||timer)return", runtime) self.assertNotIn("clearTimeout(timer);timer=setTimeout(()=>render(items),100)", runtime) - self.assertIn('document.addEventListener("selectionchange"', runtime) - self.assertNotIn('document.addEventListener("mouseup"', runtime) self.assertIn('event.data?.type==="clear-text-selection"', runtime) self.assertIn("getSelection()?.removeAllRanges()", runtime) self.assertIn("if(selection&&!selection.isCollapsed)return", runtime) @@ -118,6 +116,122 @@ def test_runtime_bounds_host_side_effects(self) -> None: self.assertIn('url.hostname.endsWith(".posthog.com")', runtime) self.assertIn("serialized.length>16384", runtime) + def _run_runtime_harness(self, runtime: str, harness: str) -> None: + with tempfile.TemporaryDirectory() as directory: + (Path(directory) / "runtime.js").write_text(runtime) + (Path(directory) / "harness.mjs").write_text(harness) + process = subprocess.run( + [node_executable(), str(Path(directory) / "harness.mjs")], + capture_output=True, + text=True, + timeout=30, + ) + self.assertEqual(process.returncode, 0, process.stderr) + + def test_runtime_reports_the_selection_once_it_settles(self) -> None: + result = run_cloud_builder(self._project('document.body.textContent = "Hello"')) + + runtime = next(file["content"] for file in result["files"] if file["path"] == "assets/canvas-runtime.js") + harness = """ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; + +// A paragraph wrapper box plus two line boxes. The selection ends on the short +// second line, so only a leaf-box anchor reports right=150, bottom=40. +const WRAPPER = { left: 0, top: 0, right: 400, bottom: 60, width: 400, height: 60 }; +const FIRST_LINE = { left: 10, top: 0, right: 390, bottom: 20, width: 380, height: 20 }; +const LAST_LINE = { left: 10, top: 20, right: 150, bottom: 40, width: 140, height: 20 }; + +const listeners = { message: [] }; +const documentListeners = {}; +// Ranges are created in report order: the text before the selection, the text +// through its end, then the whole document. +const rangeStrings = ["Hello ", "Hello world", "Hello world!"]; +let rangesCreated = 0; + +const container = { nodeType: 1 }; +const selectionRange = { + startContainer: container, + startOffset: 0, + endContainer: container, + endOffset: 0, + getClientRects: () => [WRAPPER, FIRST_LINE, LAST_LINE], + getBoundingClientRect: () => WRAPPER, +}; + +globalThis.window = globalThis; +globalThis.parent = {}; +globalThis.location = { hash: "" }; +globalThis.Element = class Element {}; +globalThis.MouseEvent = class MouseEvent {}; +globalThis.MutationObserver = class { + observe() {} +}; +globalThis.requestAnimationFrame = (fn) => setTimeout(fn, 0); +globalThis.cancelAnimationFrame = (id) => clearTimeout(id); +globalThis.addEventListener = (type, handler) => (listeners[type] ??= []).push(handler); +globalThis.getSelection = () => ({ + isCollapsed: false, + rangeCount: 1, + getRangeAt: () => selectionRange, + removeAllRanges: () => {}, +}); +globalThis.document = { + readyState: "complete", + body: { contains: () => true }, + head: { appendChild: () => {} }, + documentElement: { classList: { toggle: () => {} }, style: {} }, + defaultView: globalThis, + createElement: () => ({}), + createTreeWalker: () => ({ nextNode: () => null }), + createRange: () => { + const value = rangeStrings[rangesCreated++ % rangeStrings.length]; + return { selectNodeContents: () => {}, setEnd: () => {}, toString: () => value }; + }, + addEventListener: (type, handler) => (documentListeners[type] ??= []).push(handler), +}; + +new Function(readFileSync(new URL("./runtime.js", import.meta.url), "utf8"))(); + +const bridge = new MessageChannel(); +const received = []; +bridge.port1.addEventListener("message", (event) => received.push(event.data)); +bridge.port1.start(); +for (const handler of listeners.message) { + handler({ source: globalThis.parent, data: { channel: "posthog-canvas", type: "connect" }, ports: [bridge.port2] }); +} + +const fire = (type, event = {}) => { + for (const handler of documentListeners[type] ?? []) handler(event); +}; +const settle = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); +const selections = () => received.filter((message) => message.type === "text-selection"); + +// A drag: press, several selection updates, release. The gaps are longer than +// the runtime's own 80ms debounce, so a runtime reporting on raw +// selectionchange would have published a mid-drag selection by now. +fire("pointerdown", { target: null, button: 0 }); +for (let step = 0; step < 3; step++) { + fire("selectionchange", {}); + await settle(120); +} +assert.deepEqual(selections(), [], "the runtime reported a selection while the drag was still in progress"); + +fire("pointerup", { target: null, button: 0 }); +await settle(200); + +const reports = selections(); +assert.equal(reports.length, 1, `expected one settled report, got ${reports.length}`); +assert.equal(reports[0].selection.quote, "world"); +assert.deepEqual( + reports[0].selection.rect, + { top: LAST_LINE.top, right: LAST_LINE.right, bottom: LAST_LINE.bottom, left: LAST_LINE.left }, + "the runtime anchored the action to the whole-range box instead of the last selected line" +); +bridge.port1.close(); +""" + self._run_runtime_harness(runtime, harness) + def test_runtime_applies_the_host_theme(self) -> None: result = run_cloud_builder(self._project('document.body.textContent = "Hello"')) @@ -178,16 +292,7 @@ def test_runtime_applies_the_host_theme(self) -> None: assert.deepEqual(toggles, [["dark", true], ["dark", false]]); bridge.port1.close(); """ - with tempfile.TemporaryDirectory() as directory: - (Path(directory) / "runtime.js").write_text(runtime) - (Path(directory) / "harness.mjs").write_text(harness) - process = subprocess.run( - [node_executable(), str(Path(directory) / "harness.mjs")], - capture_output=True, - text=True, - timeout=30, - ) - self.assertEqual(process.returncode, 0, process.stderr) + self._run_runtime_harness(runtime, harness) def test_freezes_declared_capabilities_into_manifest(self) -> None: project = self._project('document.body.textContent = "Hello"') diff --git a/products/canvas/packages/canvas_builder/build.mjs b/products/canvas/packages/canvas_builder/build.mjs index 39d9e6a58729..184452eb53b8 100644 --- a/products/canvas/packages/canvas_builder/build.mjs +++ b/products/canvas/packages/canvas_builder/build.mjs @@ -24,7 +24,148 @@ const forbiddenHtml = /(?:src|href)\s*=\s*["']\s*(javascript|data:text\/html|vbs const extensions = ['', '.ts', '.tsx', '.js', '.jsx', '.css', '.json', '.svg', '.txt'] const runtimePath = 'assets/canvas-runtime.js' const runtime = `(()=>{const channel="posthog-canvas",pending=new Map;let sequence=0,port;const post=(message)=>port?.postMessage({channel,...message});const call=(method,payload)=>new Promise((resolve,reject)=>{const id=String(++sequence);const timer=setTimeout(()=>{pending.delete(id);reject(new Error("Canvas request timed out"));},30000);pending.set(id,{resolve,reject,timer});post({type:"data-request",id,method,payload});});const applyTheme=(theme)=>{if(theme!=="dark"&&theme!=="light")return;const dark=theme==="dark";document.documentElement.classList.toggle("dark",dark);document.documentElement.style.colorScheme=dark?"dark":"light";};applyTheme(new URLSearchParams(location.hash.slice(1)).get("theme"));const receive=(event)=>{if(event.data?.channel!==channel)return;if(event.data.type==="set-theme"){applyTheme(event.data.theme);return}if(event.data.type!=="data-response")return;const request=pending.get(event.data.id);if(!request)return;pending.delete(event.data.id);clearTimeout(request.timer);event.data.ok?request.resolve(event.data.result):request.reject(new Error(event.data.error??"Canvas request failed"));};const capture=(event,properties,distinctId)=>{const normalized=properties??{};let serialized;try{serialized=JSON.stringify(normalized)}catch{throw new Error("Canvas capture properties must be serializable")};if(typeof serialized!=="string"||serialized.length>16384)throw new Error("Canvas capture properties are too large");return call("capture",{event,properties:normalized,distinctId})};const openExternal=(value)=>{const url=new URL(value);if(url.protocol!=="https:"||!(url.hostname==="posthog.com"||url.hostname.endsWith(".posthog.com")))throw new Error("Canvas external URL is not allowed");post({type:"open-external",url:url.href})};window.ph={loadInsight:(shortId,options)=>call("loadInsight",{shortId,dateRange:options?.dateRange}),query:(query,params)=>call("query",typeof query==="string"?{hogql:query,params:params??{}}:{query,params:params??{}}),capture,openExternal};addEventListener("message",(event)=>{if(port||event.source!==parent||event.data?.channel!==channel||event.data?.type!=="connect"||!event.ports[0])return;port=event.ports[0];port.addEventListener("message",receive);port.start();if(document.readyState!=="loading")post({type:"ready"});if(document.readyState==="complete")post({type:"rendered"});});addEventListener("error",(event)=>post({type:"error",message:event.message||"Canvas runtime error",stack:event.error?.stack}));addEventListener("unhandledrejection",(event)=>post({type:"error",message:event.reason instanceof Error?event.reason.message:String(event.reason),stack:event.reason instanceof Error?event.reason.stack:undefined}));addEventListener("DOMContentLoaded",()=>post({type:"ready"}));addEventListener("load",()=>post({type:"rendered"}));})();` -const selectionRuntime = `(()=>{const channel="posthog-canvas";let port,timer=0;const post=message=>port?.postMessage({channel,...message}),clear=()=>post({type:"text-selection-cleared"}),clearNative=()=>{getSelection()?.removeAllRanges();clear()},report=()=>{clearTimeout(timer);timer=setTimeout(()=>{const selection=getSelection();if(!selection||selection.isCollapsed||selection.rangeCount===0){clear();return}const range=selection.getRangeAt(0);if(!document.body.contains(range.startContainer)||!document.body.contains(range.endContainer)){clear();return}const before=document.createRange();before.selectNodeContents(document.body);before.setEnd(range.startContainer,range.startOffset);const through=document.createRange();through.selectNodeContents(document.body);through.setEnd(range.endContainer,range.endOffset);const whole=document.createRange();whole.selectNodeContents(document.body);const text=whole.toString(),start=before.toString().length,end=through.toString().length,quote=text.slice(start,end);if(!quote.trim()||quote.length>10000){clear();return}const rect=range.getBoundingClientRect();post({type:"text-selection",selection:{quote,prefix:text.slice(Math.max(0,start-32),start),suffix:text.slice(end,end+32),start,end,rect:{top:rect.top,right:rect.right,bottom:rect.bottom,left:rect.left}}})},80)};addEventListener("message",event=>{if(port||event.source!==parent||event.data?.channel!==channel||event.data?.type!=="connect"||!event.ports[0])return;port=event.ports[0];port.addEventListener("message",event=>{if(event.data?.channel===channel&&event.data?.type==="clear-text-selection")clearNative()});port.start()});document.addEventListener("selectionchange",report)})();` +// A published canvas runs the runtime baked into its artifact by this builder, +// while an unpublished one runs the desktop's sandbox document — two copies of +// the same selection behavior. The pair below is ported from the desktop's +// selectionCommentAction.ts (commentActionAnchorRect, installSelectionSettleGate) +// because this builder ships inside its own build image and cannot import that +// workspace. Change one, change the other: test_cloud_builder.py drives this +// copy through a simulated drag with the desktop suite's expectations. + +// Which box the comment action anchors to. A Range spanning block elements also +// reports the wrapper boxes (paragraph, blockquote, list), so neither the +// bounding box nor the last entry marks where the user stopped selecting. Keep +// the leaf boxes — those enclosing no other box — and take the lowest, then +// right-most one: the end of the last selected line. +function commentActionAnchorRect(rects, fallback) { + const boxes = [] + for (let index = 0; index < rects.length; index++) { + const rect = rects[index] + if (rect.width > 0 || rect.height > 0) boxes.push(rect) + } + if (boxes.length === 0) return fallback + const EPSILON = 0.5 + const area = (box) => box.width * box.height + const encloses = (outer, inner) => + area(outer) > area(inner) + 1 && + outer.left <= inner.left + EPSILON && + outer.right >= inner.right - EPSILON && + outer.top <= inner.top + EPSILON && + outer.bottom >= inner.bottom - EPSILON + const leaves = boxes.filter((box) => !boxes.some((other) => other !== box && encloses(box, other))) + const pool = leaves.length > 0 ? leaves : boxes + let best = pool[0] + for (const box of pool) { + const lower = box.bottom > best.bottom + EPSILON + const sameLine = Math.abs(box.bottom - best.bottom) <= EPSILON + if (lower || (sameLine && box.right > best.right)) best = box + } + return best +} + +// While the user selects, the range keeps moving, so an action anchored to the +// live selection chases the cursor. The gate reports only settled selections: +// +// selectstart / pointerdown / selection keydown -> hide +// selectionchange -> ignore while gesturing +// pointerup / selection keyup -> report, two frames later +// pointercancel / blur -> cancel +// +// The two frames matter: the browser commits the selection AFTER the pointerup +// handler runs, so reading it synchronously returns the mid-gesture range. +function installSelectionSettleGate(doc, callbacks) { + const view = doc.defaultView + let selecting = false + let frame = 0 + + // Keys that move or extend a selection. "a" only counts with a modifier, so + // typing the letter doesn't read as select-all. + const isSelectionKey = (event) => { + if (event.key === 'a' || event.key === 'A') { + return event.metaKey || event.ctrlKey + } + return ( + event.key === 'Shift' || + event.key === 'Home' || + event.key === 'End' || + event.key === 'PageUp' || + event.key === 'PageDown' || + event.key.startsWith('Arrow') + ) + } + + const cancelFrame = () => { + if (frame && view?.cancelAnimationFrame) view.cancelAnimationFrame(frame) + frame = 0 + } + const settle = () => { + cancelFrame() + const request = view?.requestAnimationFrame + if (!request) { + callbacks.onSelectionSettled?.() + return + } + frame = request.call(view, () => { + frame = request.call(view, () => { + frame = 0 + callbacks.onSelectionSettled?.() + }) + }) + } + const inActionUi = (target) => target instanceof Element && !!target.closest('[data-selection-comment-overlay]') + const startGesture = () => { + selecting = true + cancelFrame() + callbacks.onGestureStart?.() + } + const cancelGesture = () => { + if (!selecting) return + selecting = false + cancelFrame() + callbacks.onGestureCancel?.() + } + + const onPointerDown = (event) => { + // Secondary buttons open menus; they don't select. + if (event instanceof MouseEvent && event.button > 0) return + if (inActionUi(event.target)) return + startGesture() + } + // Catches drags whose pointerdown we never saw, and keyboard selections. + const onSelectStart = (event) => { + if (inActionUi(event.target)) return + startGesture() + } + const onPointerUp = (event) => { + if (event instanceof MouseEvent && event.button > 0) return + if (!selecting) return + selecting = false + settle() + } + const onKeyDown = (event) => { + if (isSelectionKey(event)) startGesture() + } + const onKeyUp = (event) => { + if (!selecting || !isSelectionKey(event)) return + selecting = false + settle() + } + const onSelectionChange = () => { + if (selecting) return + callbacks.onIdleSelectionChange?.() + } + + doc.addEventListener('pointerdown', onPointerDown, true) + doc.addEventListener('selectstart', onSelectStart, true) + doc.addEventListener('pointerup', onPointerUp, true) + doc.addEventListener('pointercancel', cancelGesture, true) + doc.addEventListener('keydown', onKeyDown, true) + doc.addEventListener('keyup', onKeyUp, true) + doc.addEventListener('selectionchange', onSelectionChange) + view?.addEventListener('blur', cancelGesture) +} + +const selectionRuntime = `(()=>{const channel="posthog-canvas";let port,timer=0;const anchorRect=${commentActionAnchorRect.toString()};const settleGate=${installSelectionSettleGate.toString()};const post=message=>port?.postMessage({channel,...message}),clear=()=>post({type:"text-selection-cleared"}),clearNative=()=>{getSelection()?.removeAllRanges();clear()},report=()=>{clearTimeout(timer);timer=setTimeout(()=>{const selection=getSelection();if(!selection||selection.isCollapsed||selection.rangeCount===0){clear();return}const range=selection.getRangeAt(0);if(!document.body.contains(range.startContainer)||!document.body.contains(range.endContainer)){clear();return}const before=document.createRange();before.selectNodeContents(document.body);before.setEnd(range.startContainer,range.startOffset);const through=document.createRange();through.selectNodeContents(document.body);through.setEnd(range.endContainer,range.endOffset);const whole=document.createRange();whole.selectNodeContents(document.body);const text=whole.toString(),start=before.toString().length,end=through.toString().length,quote=text.slice(start,end);if(!quote.trim()||quote.length>10000){clear();return}const rect=anchorRect(range.getClientRects?range.getClientRects():[],range.getBoundingClientRect());post({type:"text-selection",selection:{quote,prefix:text.slice(Math.max(0,start-32),start),suffix:text.slice(end,end+32),start,end,rect:{top:rect.top,right:rect.right,bottom:rect.bottom,left:rect.left}}})},80)};addEventListener("message",event=>{if(port||event.source!==parent||event.data?.channel!==channel||event.data?.type!=="connect"||!event.ports[0])return;port=event.ports[0];port.addEventListener("message",event=>{if(event.data?.channel===channel&&event.data?.type==="clear-text-selection")clearNative()});port.start()});const abort=()=>{clearTimeout(timer);clear()};settleGate(document,{onGestureStart:abort,onSelectionSettled:report,onIdleSelectionChange:report,onGestureCancel:abort})})();` const highlightRuntime = `(()=>{const channel="posthog-canvas",style=document.createElement("style");style.textContent="::highlight(posthog-canvas-comment){background:rgba(250,204,21,.32);color:inherit}::highlight(posthog-canvas-comment-active){background:rgba(250,204,21,.48);color:inherit}";document.head.appendChild(style);let items=[],ranges=[],port,timer=0;const indexText=()=>{const walker=document.createTreeWalker(document.body,NodeFilter.SHOW_TEXT),entries=[];let text="";for(let node=walker.nextNode();node;node=walker.nextNode()){const start=text.length;text+=node.data;entries.push({node,start,end:text.length})}return{text,entries}},rangeAt=(index,start,end)=>{const find=offset=>{let low=0,high=index.entries.length-1,match=null;while(low<=high){const middle=low+high>>1,entry=index.entries[middle];if(offsetentry.end)low=middle+1;else{match=entry;high=middle-1}}return match},startEntry=find(start),endEntry=find(end);if(!startEntry||!endEntry)return null;const range=document.createRange();range.setStart(startEntry.node,start-startEntry.start);range.setEnd(endEntry.node,end-endEntry.start);return range},resolve=(text,anchor)=>{if(text.slice(anchor.start,anchor.end)===anchor.quote)return{start:anchor.start,end:anchor.end};const matches=[];for(let start=text.indexOf(anchor.quote);start>=0;start=text.indexOf(anchor.quote,start+Math.max(anchor.quote.length,1))){const end=start+anchor.quote.length,prefix=text.slice(Math.max(0,start-anchor.prefix.length),start),suffix=text.slice(end,end+anchor.suffix.length);matches.push({start,end,score:(anchor.prefix&&prefix===anchor.prefix?2:0)+(anchor.suffix&&suffix===anchor.suffix?2:0)})}if(matches.length===1)return matches[0];matches.sort((a,b)=>b.score-a.score);return matches[0]?.score&&matches[0].score!==matches[1]?.score?matches[0]:null},render=next=>{items=next||[];ranges=[];if(!window.Highlight||!window.CSS||!CSS.highlights)return;const normal=new Highlight,active=new Highlight,index=indexText();for(const item of items){const hit=resolve(index.text,item.anchor),range=hit&&rangeAt(index,hit.start,hit.end);if(range){ranges.push({id:item.id,range});(item.active?active:normal).add(range)}}CSS.highlights.set("posthog-canvas-comment",normal);CSS.highlights.set("posthog-canvas-comment-active",active)};addEventListener("message",event=>{if(port||event.source!==parent||event.data?.channel!==channel||event.data?.type!=="connect"||!event.ports[0])return;port=event.ports[0];port.addEventListener("message",event=>{if(event.data?.channel===channel&&event.data?.type==="set-comment-highlights")render(event.data.highlights)});port.start()});document.addEventListener("click",event=>{const selection=getSelection();if(selection&&!selection.isCollapsed)return;for(const item of ranges)for(const rect of item.range.getClientRects())if(event.clientX>=rect.left&&event.clientX<=rect.right&&event.clientY>=rect.top&&event.clientY<=rect.bottom){event.preventDefault();event.stopPropagation();port?.postMessage({channel,type:"comment-activate",id:item.id});return}},true);new MutationObserver(()=>{if(!items.length||timer)return;timer=setTimeout(()=>{timer=0;render(items)},500)}).observe(document.body,{childList:true,characterData:true,subtree:true})})();` // Selection and highlight runtimes extend the shared canvas bridge. const platformStylesheet = ` diff --git a/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts b/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts index 751696acc764..6fa1a52f6b8e 100644 --- a/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts +++ b/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts @@ -340,11 +340,17 @@ export function buildSandboxDocument( // doesn't chase the cursor mid-drag. The settle callback re-reads the live // selection, which self-corrects clicks that didn't change the selection. const selectionSettleGate = ${installSelectionSettleGate.toString()}; + // Dropping the pending report matters: a new drag started within the + // debounce window would otherwise publish the previous selection mid-drag. + const abortTextSelection = () => { + clearTimeout(selectionTimer); + clearTextSelection(); + }; selectionSettleGate(document, { - onGestureStart: clearTextSelection, + onGestureStart: abortTextSelection, onSelectionSettled: reportTextSelection, onIdleSelectionChange: reportTextSelection, - onGestureCancel: clearTextSelection, + onGestureCancel: abortTextSelection, }); const clearNativeTextSelection = () => { window.getSelection()?.removeAllRanges(); From 44e0c575709366bccde172abd6ab8aad2ea09c4c Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Fri, 7 Aug 2026 14:37:13 +0200 Subject: [PATCH 6/7] fix(comments): address selection action review feedback Fix select-all settling, repeated clear messages, and composer placement. Keep the published canvas runtime in sync and exercise its pointer guards. Generated-By: PostHog Code Task-Id: c8373a88-43af-4fbd-958a-b6385a2d6849 --- .../backend/tests/test_cloud_builder.py | 41 ++++++++++-- .../canvas/packages/canvas_builder/build.mjs | 49 +++++++++----- .../freeform/CanvasSelectionCommentAction.tsx | 2 +- .../canvas/freeform/sandboxRuntime.ts | 9 ++- .../components/CodeMirrorEditor.tsx | 10 +-- .../SelectionCommentOverlay.test.tsx | 8 +-- .../components/SelectionCommentOverlay.tsx | 5 +- .../components/AnnotatedArtifactHtml.tsx | 2 +- .../components/AnnotatedArtifactImage.tsx | 2 +- .../components/ArtifactTextAnnotations.tsx | 10 +-- .../artifactHtmlCommentBridge.test.ts | 23 +++---- .../components/artifactHtmlCommentBridge.ts | 14 ++-- .../components/artifactPreviewDocument.ts | 18 +++-- .../components/selectionCommentAction.test.ts | 33 ++++++++-- .../components/selectionCommentAction.ts | 66 +++++++++++-------- 15 files changed, 197 insertions(+), 95 deletions(-) diff --git a/products/canvas/backend/tests/test_cloud_builder.py b/products/canvas/backend/tests/test_cloud_builder.py index 64702296203b..2f8d7bf2f2fc 100644 --- a/products/canvas/backend/tests/test_cloud_builder.py +++ b/products/canvas/backend/tests/test_cloud_builder.py @@ -162,8 +162,18 @@ def test_runtime_reports_the_selection_once_it_settles(self) -> None: globalThis.window = globalThis; globalThis.parent = {}; globalThis.location = { hash: "" }; -globalThis.Element = class Element {}; -globalThis.MouseEvent = class MouseEvent {}; +globalThis.Element = class Element { + constructor(inOverlay = false) { this.inOverlay = inOverlay; } + closest(selector) { + return this.inOverlay && selector === "[data-selection-comment-overlay]" ? this : null; + } +}; +globalThis.MouseEvent = class MouseEvent { + constructor(button, target) { + this.button = button; + this.target = target; + } +}; globalThis.MutationObserver = class { observe() {} }; @@ -206,18 +216,34 @@ def test_runtime_reports_the_selection_once_it_settles(self) -> None: }; const settle = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); const selections = () => received.filter((message) => message.type === "text-selection"); +const clears = () => received.filter((message) => message.type === "text-selection-cleared"); +const outside = new Element(); +const overlay = new Element(true); + +// Secondary presses must not open the gesture gate, so the following idle +// selection change still publishes normally. +fire("pointerdown", new MouseEvent(2, outside)); +fire("selectionchange", {}); +await settle(120); +assert.equal(selections().length, 1, "right-click left the selection gate open"); + +// The published action stays visible when its own UI receives pointerdown. +fire("pointerdown", new MouseEvent(0, overlay)); +await settle(20); +assert.equal(clears().length, 0, "pressing the comment action cleared its selection"); +received.length = 0; // A drag: press, several selection updates, release. The gaps are longer than // the runtime's own 80ms debounce, so a runtime reporting on raw // selectionchange would have published a mid-drag selection by now. -fire("pointerdown", { target: null, button: 0 }); +fire("pointerdown", new MouseEvent(0, outside)); for (let step = 0; step < 3; step++) { fire("selectionchange", {}); await settle(120); } assert.deepEqual(selections(), [], "the runtime reported a selection while the drag was still in progress"); -fire("pointerup", { target: null, button: 0 }); +fire("pointerup", new MouseEvent(0, outside)); await settle(200); const reports = selections(); @@ -228,6 +254,13 @@ def test_runtime_reports_the_selection_once_it_settles(self) -> None: { top: LAST_LINE.top, right: LAST_LINE.right, bottom: LAST_LINE.bottom, left: LAST_LINE.left }, "the runtime anchored the action to the whole-range box instead of the last selected line" ); + +fire("scroll", {}); +await settle(20); +assert.equal(clears().length, 2, "scrolling did not clear the published selection"); +fire("scroll", {}); +await settle(20); +assert.equal(clears().length, 2, "repeated scroll sent a duplicate clear message"); bridge.port1.close(); """ self._run_runtime_harness(runtime, harness) diff --git a/products/canvas/packages/canvas_builder/build.mjs b/products/canvas/packages/canvas_builder/build.mjs index 184452eb53b8..6ec5bbb49c73 100644 --- a/products/canvas/packages/canvas_builder/build.mjs +++ b/products/canvas/packages/canvas_builder/build.mjs @@ -34,9 +34,9 @@ const runtime = `(()=>{const channel="posthog-canvas",pending=new Map;let sequen // Which box the comment action anchors to. A Range spanning block elements also // reports the wrapper boxes (paragraph, blockquote, list), so neither the -// bounding box nor the last entry marks where the user stopped selecting. Keep -// the leaf boxes — those enclosing no other box — and take the lowest, then -// right-most one: the end of the last selected line. +// bounding box nor the last entry marks where the user stopped selecting. +// Check boxes from visually last to first and take the first leaf box, which +// avoids scanning every pair for normal selections. function commentActionAnchorRect(rects, fallback) { const boxes = [] for (let index = 0; index < rects.length; index++) { @@ -52,15 +52,16 @@ function commentActionAnchorRect(rects, fallback) { outer.right >= inner.right - EPSILON && outer.top <= inner.top + EPSILON && outer.bottom >= inner.bottom - EPSILON - const leaves = boxes.filter((box) => !boxes.some((other) => other !== box && encloses(box, other))) - const pool = leaves.length > 0 ? leaves : boxes - let best = pool[0] - for (const box of pool) { - const lower = box.bottom > best.bottom + EPSILON - const sameLine = Math.abs(box.bottom - best.bottom) <= EPSILON - if (lower || (sameLine && box.right > best.right)) best = box + const candidates = boxes.slice().sort((left, right) => { + const verticalDistance = right.bottom - left.bottom + return Math.abs(verticalDistance) > EPSILON ? verticalDistance : right.right - left.right + }) + for (const box of candidates) { + if (!boxes.some((other) => other !== box && encloses(box, other))) { + return box + } } - return best + return candidates[0] } // While the user selects, the range keeps moving, so an action anchored to the @@ -76,6 +77,7 @@ function commentActionAnchorRect(rects, fallback) { function installSelectionSettleGate(doc, callbacks) { const view = doc.defaultView let selecting = false + let keyGesture = false let frame = 0 // Keys that move or extend a selection. "a" only counts with a modifier, so @@ -114,6 +116,7 @@ function installSelectionSettleGate(doc, callbacks) { } const inActionUi = (target) => target instanceof Element && !!target.closest('[data-selection-comment-overlay]') const startGesture = () => { + if (selecting) return selecting = true cancelFrame() callbacks.onGestureStart?.() @@ -121,6 +124,7 @@ function installSelectionSettleGate(doc, callbacks) { const cancelGesture = () => { if (!selecting) return selecting = false + keyGesture = false cancelFrame() callbacks.onGestureCancel?.() } @@ -140,14 +144,18 @@ function installSelectionSettleGate(doc, callbacks) { if (event instanceof MouseEvent && event.button > 0) return if (!selecting) return selecting = false + keyGesture = false settle() } const onKeyDown = (event) => { - if (isSelectionKey(event)) startGesture() + if (!isSelectionKey(event)) return + keyGesture = true + startGesture() } - const onKeyUp = (event) => { - if (!selecting || !isSelectionKey(event)) return + const onKeyUp = () => { + if (!selecting || !keyGesture) return selecting = false + keyGesture = false settle() } const onSelectionChange = () => { @@ -163,9 +171,20 @@ function installSelectionSettleGate(doc, callbacks) { doc.addEventListener('keyup', onKeyUp, true) doc.addEventListener('selectionchange', onSelectionChange) view?.addEventListener('blur', cancelGesture) + return () => { + cancelFrame() + doc.removeEventListener('pointerdown', onPointerDown, true) + doc.removeEventListener('selectstart', onSelectStart, true) + doc.removeEventListener('pointerup', onPointerUp, true) + doc.removeEventListener('pointercancel', cancelGesture, true) + doc.removeEventListener('keydown', onKeyDown, true) + doc.removeEventListener('keyup', onKeyUp, true) + doc.removeEventListener('selectionchange', onSelectionChange) + view?.removeEventListener('blur', cancelGesture) + } } -const selectionRuntime = `(()=>{const channel="posthog-canvas";let port,timer=0;const anchorRect=${commentActionAnchorRect.toString()};const settleGate=${installSelectionSettleGate.toString()};const post=message=>port?.postMessage({channel,...message}),clear=()=>post({type:"text-selection-cleared"}),clearNative=()=>{getSelection()?.removeAllRanges();clear()},report=()=>{clearTimeout(timer);timer=setTimeout(()=>{const selection=getSelection();if(!selection||selection.isCollapsed||selection.rangeCount===0){clear();return}const range=selection.getRangeAt(0);if(!document.body.contains(range.startContainer)||!document.body.contains(range.endContainer)){clear();return}const before=document.createRange();before.selectNodeContents(document.body);before.setEnd(range.startContainer,range.startOffset);const through=document.createRange();through.selectNodeContents(document.body);through.setEnd(range.endContainer,range.endOffset);const whole=document.createRange();whole.selectNodeContents(document.body);const text=whole.toString(),start=before.toString().length,end=through.toString().length,quote=text.slice(start,end);if(!quote.trim()||quote.length>10000){clear();return}const rect=anchorRect(range.getClientRects?range.getClientRects():[],range.getBoundingClientRect());post({type:"text-selection",selection:{quote,prefix:text.slice(Math.max(0,start-32),start),suffix:text.slice(end,end+32),start,end,rect:{top:rect.top,right:rect.right,bottom:rect.bottom,left:rect.left}}})},80)};addEventListener("message",event=>{if(port||event.source!==parent||event.data?.channel!==channel||event.data?.type!=="connect"||!event.ports[0])return;port=event.ports[0];port.addEventListener("message",event=>{if(event.data?.channel===channel&&event.data?.type==="clear-text-selection")clearNative()});port.start()});const abort=()=>{clearTimeout(timer);clear()};settleGate(document,{onGestureStart:abort,onSelectionSettled:report,onIdleSelectionChange:report,onGestureCancel:abort})})();` +const selectionRuntime = `(()=>{const channel="posthog-canvas";let port,timer=0,published=false;const anchorRect=${commentActionAnchorRect.toString()};const settleGate=${installSelectionSettleGate.toString()};const post=message=>port?.postMessage({channel,...message}),clear=()=>{if(!published)return;published=false;post({type:"text-selection-cleared"})},clearNative=()=>{getSelection()?.removeAllRanges();clear()},report=()=>{clearTimeout(timer);timer=setTimeout(()=>{const selection=getSelection();if(!selection||selection.isCollapsed||selection.rangeCount===0){clear();return}const range=selection.getRangeAt(0);if(!document.body.contains(range.startContainer)||!document.body.contains(range.endContainer)){clear();return}const before=document.createRange();before.selectNodeContents(document.body);before.setEnd(range.startContainer,range.startOffset);const through=document.createRange();through.selectNodeContents(document.body);through.setEnd(range.endContainer,range.endOffset);const whole=document.createRange();whole.selectNodeContents(document.body);const text=whole.toString(),start=before.toString().length,end=through.toString().length,quote=text.slice(start,end);if(!quote.trim()||quote.length>10000){clear();return}const rect=anchorRect(range.getClientRects?range.getClientRects():[],range.getBoundingClientRect());published=true;post({type:"text-selection",selection:{quote,prefix:text.slice(Math.max(0,start-32),start),suffix:text.slice(end,end+32),start,end,rect:{top:rect.top,right:rect.right,bottom:rect.bottom,left:rect.left}}})},80)};addEventListener("message",event=>{if(port||event.source!==parent||event.data?.channel!==channel||event.data?.type!=="connect"||!event.ports[0])return;port=event.ports[0];port.addEventListener("message",event=>{if(event.data?.channel===channel&&event.data?.type==="clear-text-selection")clearNative()});port.start()});const abort=()=>{clearTimeout(timer);clear()};settleGate(document,{onGestureStart:abort,onSelectionSettled:report,onIdleSelectionChange:report,onGestureCancel:abort});document.addEventListener("scroll",abort,true)})();` const highlightRuntime = `(()=>{const channel="posthog-canvas",style=document.createElement("style");style.textContent="::highlight(posthog-canvas-comment){background:rgba(250,204,21,.32);color:inherit}::highlight(posthog-canvas-comment-active){background:rgba(250,204,21,.48);color:inherit}";document.head.appendChild(style);let items=[],ranges=[],port,timer=0;const indexText=()=>{const walker=document.createTreeWalker(document.body,NodeFilter.SHOW_TEXT),entries=[];let text="";for(let node=walker.nextNode();node;node=walker.nextNode()){const start=text.length;text+=node.data;entries.push({node,start,end:text.length})}return{text,entries}},rangeAt=(index,start,end)=>{const find=offset=>{let low=0,high=index.entries.length-1,match=null;while(low<=high){const middle=low+high>>1,entry=index.entries[middle];if(offsetentry.end)low=middle+1;else{match=entry;high=middle-1}}return match},startEntry=find(start),endEntry=find(end);if(!startEntry||!endEntry)return null;const range=document.createRange();range.setStart(startEntry.node,start-startEntry.start);range.setEnd(endEntry.node,end-endEntry.start);return range},resolve=(text,anchor)=>{if(text.slice(anchor.start,anchor.end)===anchor.quote)return{start:anchor.start,end:anchor.end};const matches=[];for(let start=text.indexOf(anchor.quote);start>=0;start=text.indexOf(anchor.quote,start+Math.max(anchor.quote.length,1))){const end=start+anchor.quote.length,prefix=text.slice(Math.max(0,start-anchor.prefix.length),start),suffix=text.slice(end,end+anchor.suffix.length);matches.push({start,end,score:(anchor.prefix&&prefix===anchor.prefix?2:0)+(anchor.suffix&&suffix===anchor.suffix?2:0)})}if(matches.length===1)return matches[0];matches.sort((a,b)=>b.score-a.score);return matches[0]?.score&&matches[0].score!==matches[1]?.score?matches[0]:null},render=next=>{items=next||[];ranges=[];if(!window.Highlight||!window.CSS||!CSS.highlights)return;const normal=new Highlight,active=new Highlight,index=indexText();for(const item of items){const hit=resolve(index.text,item.anchor),range=hit&&rangeAt(index,hit.start,hit.end);if(range){ranges.push({id:item.id,range});(item.active?active:normal).add(range)}}CSS.highlights.set("posthog-canvas-comment",normal);CSS.highlights.set("posthog-canvas-comment-active",active)};addEventListener("message",event=>{if(port||event.source!==parent||event.data?.channel!==channel||event.data?.type!=="connect"||!event.ports[0])return;port=event.ports[0];port.addEventListener("message",event=>{if(event.data?.channel===channel&&event.data?.type==="set-comment-highlights")render(event.data.highlights)});port.start()});document.addEventListener("click",event=>{const selection=getSelection();if(selection&&!selection.isCollapsed)return;for(const item of ranges)for(const rect of item.range.getClientRects())if(event.clientX>=rect.left&&event.clientX<=rect.right&&event.clientY>=rect.top&&event.clientY<=rect.bottom){event.preventDefault();event.stopPropagation();port?.postMessage({channel,type:"comment-activate",id:item.id});return}},true);new MutationObserver(()=>{if(!items.length||timer)return;timer=setTimeout(()=>{timer=0;render(items)},500)}).observe(document.body,{childList:true,characterData:true,subtree:true})})();` // Selection and highlight runtimes extend the shared canvas bridge. const platformStylesheet = ` diff --git a/products/desktop/packages/ui/src/features/canvas/freeform/CanvasSelectionCommentAction.tsx b/products/desktop/packages/ui/src/features/canvas/freeform/CanvasSelectionCommentAction.tsx index 2ebf12204030..3e5818798acc 100644 --- a/products/desktop/packages/ui/src/features/canvas/freeform/CanvasSelectionCommentAction.tsx +++ b/products/desktop/packages/ui/src/features/canvas/freeform/CanvasSelectionCommentAction.tsx @@ -47,7 +47,7 @@ export function CanvasSelectionCommentAction({ toLine: selection.end + 1, anchor: { top: selection.rect.top, - left: selection.rect.right, + endX: selection.rect.right, bottom: selection.rect.bottom, }, } diff --git a/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts b/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts index 6fa1a52f6b8e..a9842f9e7dd4 100644 --- a/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts +++ b/products/desktop/packages/ui/src/features/canvas/freeform/sandboxRuntime.ts @@ -291,7 +291,12 @@ export function buildSandboxDocument( ); const selectionAnchorRect = ${commentActionAnchorRect.toString()}; - const clearTextSelection = () => post({ type: "text-selection-cleared" }); + let textSelectionPublished = false; + const clearTextSelection = () => { + if (!textSelectionPublished) return; + textSelectionPublished = false; + post({ type: "text-selection-cleared" }); + }; let selectionTimer = 0; const reportTextSelection = () => { clearTimeout(selectionTimer); @@ -323,6 +328,7 @@ export function buildSandboxDocument( // The END line's rect, so the host anchors the comment action where the // pointer was released rather than at the whole-range bounding box. const rect = selectionAnchorRect(range.getClientRects(), range.getBoundingClientRect()); + textSelectionPublished = true; post({ type: "text-selection", selection: { @@ -352,6 +358,7 @@ export function buildSandboxDocument( onIdleSelectionChange: reportTextSelection, onGestureCancel: abortTextSelection, }); + document.addEventListener("scroll", abortTextSelection, true); const clearNativeTextSelection = () => { window.getSelection()?.removeAllRanges(); clearTextSelection(); diff --git a/products/desktop/packages/ui/src/features/code-editor/components/CodeMirrorEditor.tsx b/products/desktop/packages/ui/src/features/code-editor/components/CodeMirrorEditor.tsx index a5671325b14d..d57daa283eda 100644 --- a/products/desktop/packages/ui/src/features/code-editor/components/CodeMirrorEditor.tsx +++ b/products/desktop/packages/ui/src/features/code-editor/components/CodeMirrorEditor.tsx @@ -41,8 +41,8 @@ export interface EditorSelection { /** 1-based line numbers. */ fromLine: number; toLine: number; - /** Viewport rect of the selection's end caret (end-line top/bottom + end column x), or null when off-screen. */ - anchor: { top: number; left: number; bottom: number } | null; + /** Viewport position of the selection's end caret, or null when off-screen. */ + anchor: { top: number; endX: number; bottom: number } | null; } interface CodeMirrorEditorProps { @@ -75,7 +75,9 @@ export function CodeMirrorEditor({ // Ref-stable listener: a changing extension would tear down the editor. const onSelectionChangeRef = useRef(onSelectionChange); - onSelectionChangeRef.current = onSelectionChange; + useEffect(() => { + onSelectionChangeRef.current = onSelectionChange; + }, [onSelectionChange]); const selectionExtension = useMemo( () => EditorView.updateListener.of((update) => { @@ -107,7 +109,7 @@ export function CodeMirrorEditor({ anchor: endRect ? { top: endRect.top, - left: endRect.right, + endX: endRect.right, bottom: endRect.bottom, } : null, diff --git a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx index e0a783324527..c3ab018e0719 100644 --- a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx +++ b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.test.tsx @@ -34,7 +34,7 @@ function renderCollapsed( text: "selected", fromLine: 1, toLine: 1, - anchor: { top: 20, left: 20, bottom: 38 }, + anchor: { top: 20, endX: 20, bottom: 38 }, }} open filePath="report.md" @@ -53,7 +53,7 @@ describe("SelectionCommentOverlay", () => { text: "selected", fromLine: 1, toLine: 1, - anchor: { top: 20, left: 20, bottom: 38 }, + anchor: { top: 20, endX: 20, bottom: 38 }, }} open filePath="report.md" @@ -95,7 +95,7 @@ describe("SelectionCommentOverlay", () => { text: "selected", fromLine: 1, toLine: 1, - anchor: { top: 20, left: 20, bottom: 38 }, + anchor: { top: 20, endX: 20, bottom: 38 }, }} open filePath="report.md" @@ -130,7 +130,7 @@ describe("SelectionCommentOverlay", () => { text: "selected", fromLine: 1, toLine: 1, - anchor: { top: 20, left: 20, bottom: 38 }, + anchor: { top: 20, endX: 20, bottom: 38 }, }} open filePath="report.md" diff --git a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx index 3ea7490b6c61..115fc82b8de7 100644 --- a/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx +++ b/products/desktop/packages/ui/src/features/code-editor/components/SelectionCommentOverlay.tsx @@ -89,7 +89,7 @@ function SelectionComposerCard({ initiallyExpanded, members, }: { - anchor: { top: number; left: number; bottom: number }; + anchor: { top: number; endX: number; bottom: number }; fromLine: number; toLine: number; filePath: string; @@ -114,9 +114,10 @@ function SelectionComposerCard({ ? { width: Math.min(420, window.innerWidth * 0.8), height: 180 } : { width: showActionText ? 104 : 28, height: 28 }; const style = computeCommentActionPlacement( - { top: anchor.top, right: anchor.left, bottom: anchor.bottom }, + { top: anchor.top, right: anchor.endX, bottom: anchor.bottom }, { width: window.innerWidth, height: window.innerHeight }, actionSize, + expanded ? "below" : "center", ); useEffect(() => { diff --git a/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx b/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx index cdfad9d1bda6..b64e87517138 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactHtml.tsx @@ -177,7 +177,7 @@ export function AnnotatedArtifactHtml({ toLine: parsed.data.end + 1, anchor: { top: frameBox.top + data.triggerRect.top, - left: frameBox.left + data.triggerRect.left, + endX: frameBox.left + data.triggerRect.left, bottom: frameBox.top + data.triggerRect.bottom, }, }); diff --git a/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactImage.tsx b/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactImage.tsx index 2649b7f79ea4..012e3d8aab87 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactImage.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/AnnotatedArtifactImage.tsx @@ -65,7 +65,7 @@ function ImageCommentCreationLayer({ fromLine: 1, toLine: 1, // Point anchor at the click: the composer opens next to it. - anchor: { top: clientY, left: clientX, bottom: clientY }, + anchor: { top: clientY, endX: clientX, bottom: clientY }, }); }} /> diff --git a/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx b/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx index 77b10591b4fb..21c948ae4faf 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx @@ -338,7 +338,7 @@ export function ArtifactTextAnnotations({ toLine: offsets.end + 1, anchor: { top: endRect.top, - left: endRect.right, + endX: endRect.right, bottom: endRect.bottom, }, }); @@ -353,12 +353,12 @@ export function ArtifactTextAnnotations({ onIdleSelectionChange: scheduleUpdate, onGestureCancel: clearOverlay, }); - // Scrolling moves the selection but not the fixed-position action, so - // re-anchor it to the live selection instead of leaving it behind. - container.addEventListener("scroll", scheduleUpdate, { passive: true }); + // Hide on scroll because the HTML and canvas surfaces cannot keep a + // fixed-position host action anchored during an iframe scroll. + container.addEventListener("scroll", clearOverlay, { passive: true }); return () => { cancelAnimationFrame(frame); - container.removeEventListener("scroll", scheduleUpdate); + container.removeEventListener("scroll", clearOverlay); removeGate(); }; }, [clearOverlay, containerRef, rootRef]); diff --git a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts index 25830e06a217..52b2ebe53821 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.test.ts @@ -11,10 +11,13 @@ function loadBridgeDocument( html: string, theme: "light" | "dark" = "light", ): JSDOM { - const dom = new JSDOM(injectArtifactHtmlCommentBridge(html, CHANNEL, theme), { - runScripts: "dangerously", - url: "https://localhost/", - }); + const dom = new JSDOM( + injectArtifactHtmlCommentBridge(html, { channel: CHANNEL, theme }), + { + runScripts: "dangerously", + url: "https://localhost/", + }, + ); // jsdom collapses layout boxes to zero; the bridge hides the action for // zero-size ranges, which real documents never produce for text selections. dom.window.Range.prototype.getBoundingClientRect = () => @@ -100,15 +103,13 @@ describe("artifactHtmlCommentBridge", () => { }); it("bakes the requested theme into the bridge styles", () => { - const dark = injectArtifactHtmlCommentBridge( - "", - CHANNEL, - "dark", - ); + const dark = injectArtifactHtmlCommentBridge("", { + channel: CHANNEL, + theme: "dark", + }); const light = injectArtifactHtmlCommentBridge( "", - CHANNEL, - "light", + { channel: CHANNEL, theme: "light" }, ); expect(dark).toContain( `--ph-comment-action-bg:${COMMENT_ACTION_BUTTON_THEMES.dark.background}`, diff --git a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts index fd67c88b466b..644465d3d11b 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/artifactHtmlCommentBridge.ts @@ -66,11 +66,17 @@ style();send("ready"); export function injectArtifactHtmlCommentBridge( html: string, - channel: string, - theme: CommentSurfaceTheme, - nonce?: string, + options: { + channel: string; + theme: CommentSurfaceTheme; + nonce?: string; + }, ): string { - const bridge = artifactHtmlCommentBridge(channel, theme, nonce); + const bridge = artifactHtmlCommentBridge( + options.channel, + options.theme, + options.nonce, + ); const bodyEnd = html.toLowerCase().lastIndexOf(""); if (bodyEnd >= 0) { return `${html.slice(0, bodyEnd)}${bridge}${html.slice(bodyEnd)}`; diff --git a/products/desktop/packages/ui/src/features/sessions/components/artifactPreviewDocument.ts b/products/desktop/packages/ui/src/features/sessions/components/artifactPreviewDocument.ts index 9ccd9f98d515..dd222c26f5b9 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/artifactPreviewDocument.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/artifactPreviewDocument.ts @@ -29,12 +29,11 @@ export function artifactHtmlDocument( } const nonce = crypto.randomUUID(); return applyCspToHtml( - injectArtifactHtmlCommentBridge( - safeHtml, - commentBridgeChannel, - commentSurfaceTheme, + injectArtifactHtmlCommentBridge(safeHtml, { + channel: commentBridgeChannel, + theme: commentSurfaceTheme, nonce, - ), + }), undefined, nonce, ); @@ -48,11 +47,10 @@ export function scriptedArtifactHtmlDocument( const safeHtml = removeAutomaticRedirects(html); if (!commentBridgeChannel) return applyCspToHtml(safeHtml); return applyCspToHtml( - injectArtifactHtmlCommentBridge( - safeHtml, - commentBridgeChannel, - commentSurfaceTheme, - ), + injectArtifactHtmlCommentBridge(safeHtml, { + channel: commentBridgeChannel, + theme: commentSurfaceTheme, + }), ); } export async function artifactPreviewBlob( diff --git a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.test.ts b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.test.ts index 319788d30cd4..a09da6dc1879 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.test.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.test.ts @@ -164,9 +164,12 @@ describe("selectionCommentAction", () => { } satisfies SelectionSettleGateCallbacks; const remove = installSelectionSettleGate(document, callbacks); - document.dispatchEvent( - new KeyboardEvent("keydown", { key: "ArrowRight", bubbles: true }), - ); + for (let repeat = 0; repeat < 3; repeat++) { + document.dispatchEvent( + new KeyboardEvent("keydown", { key: "ArrowRight", bubbles: true }), + ); + } + expect(callbacks.onGestureStart).toHaveBeenCalledTimes(1); changeSelection(); expect(callbacks.onIdleSelectionChange).not.toHaveBeenCalled(); @@ -178,8 +181,11 @@ describe("selectionCommentAction", () => { remove(); }); - it("treats a plain letter as typing, not as select-all", () => { - const callbacks = { onGestureStart: vi.fn() }; + it("distinguishes typing from select-all and settles after the modifier is released", async () => { + const callbacks = { + onGestureStart: vi.fn(), + onSelectionSettled: vi.fn(), + } satisfies SelectionSettleGateCallbacks; const remove = installSelectionSettleGate(document, callbacks); document.dispatchEvent( @@ -191,6 +197,12 @@ describe("selectionCommentAction", () => { new KeyboardEvent("keydown", { key: "a", metaKey: true, bubbles: true }), ); expect(callbacks.onGestureStart).toHaveBeenCalledTimes(1); + + document.dispatchEvent( + new KeyboardEvent("keyup", { key: "a", metaKey: false, bubbles: true }), + ); + await settleFrames(); + expect(callbacks.onSelectionSettled).toHaveBeenCalledTimes(1); remove(); }); @@ -298,6 +310,17 @@ describe("computeCommentActionPlacement", () => { ).toEqual({ top: 64, left: 830 }); }); + it("places the expanded composer below the selection", () => { + expect( + computeCommentActionPlacement( + { top: 100, right: 400, bottom: 120 }, + bounds, + { width: 420, height: 180 }, + "below", + ), + ).toEqual({ top: 126, left: 408 }); + }); + it("clamps to the viewport margins for selections hugging the edges", () => { expect( computeCommentActionPlacement( diff --git a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts index 6ea30cd82806..f1b1e554fa0a 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts @@ -88,8 +88,8 @@ type CommentActionBox = { // Which box the action anchors to. A Range spanning block elements reports the // wrapper boxes (paragraph, blockquote, list) alongside the text line boxes, so // neither the bounding box nor the last entry marks where the user stopped -// selecting. Keep the leaf boxes — those that enclose no other box — and take -// the visually lowest, then right-most one: the end of the last selected line. +// selecting. Check boxes from visually last to first and take the first leaf +// box, which avoids scanning every pair for normal selections. export function commentActionAnchorRect( rects: ArrayLike, fallback: T, @@ -108,40 +108,45 @@ export function commentActionAnchorRect( outer.right >= inner.right - EPSILON && outer.top <= inner.top + EPSILON && outer.bottom >= inner.bottom - EPSILON; - const leaves = boxes.filter( - (box) => !boxes.some((other) => other !== box && encloses(box, other)), - ); - const pool = leaves.length > 0 ? leaves : boxes; - let best = pool[0]; - for (const box of pool) { - const lower = box.bottom > best.bottom + EPSILON; - const sameLine = Math.abs(box.bottom - best.bottom) <= EPSILON; - if (lower || (sameLine && box.right > best.right)) best = box; + const candidates = boxes.slice().sort((left, right) => { + const verticalDistance = right.bottom - left.bottom; + return Math.abs(verticalDistance) > EPSILON + ? verticalDistance + : right.right - left.right; + }); + for (const box of candidates) { + if (!boxes.some((other) => other !== box && encloses(box, other))) { + return box; + } } - return best; + return candidates[0]; } -// Where the action sits relative to the selection's end line (Google Docs -// style): just right of the caret, vertically centered on the line. When the -// right edge has no room it drops below the end line instead, keeping its -// right edge at the caret; near the viewport bottom it flips above the line. -// `rect` is the selection's end line, `bounds` the viewport/container, `action` -// the action element's measured size. +// Where the floating action or composer sits relative to the selection's end +// line. Actions center on the caret line; composers sit below it. When the +// right edge has no room, the element stays aligned to the caret; near the +// viewport bottom it flips above the line. export function computeCommentActionPlacement( rect: { top: number; right: number; bottom: number }, bounds: { width: number; height: number }, action: { width: number; height: number }, + alignment: "center" | "below" = "center", ): { top: number; left: number } { const MARGIN = 8; const lineMiddle = rect.top + (rect.bottom - rect.top) / 2; let left = rect.right + MARGIN; - let top = lineMiddle - action.height / 2; - if (left + action.width > bounds.width - MARGIN) { + let top = + alignment === "below" ? rect.bottom + 6 : lineMiddle - action.height / 2; + const shouldDropBelow = left + action.width > bounds.width - MARGIN; + if (shouldDropBelow) { left = Math.max(rect.right - action.width, MARGIN); - top = rect.bottom + 6; - if (top + action.height > bounds.height - MARGIN) { - top = rect.top - action.height - 6; - } + if (alignment === "center") top = rect.bottom + 6; + } + if ( + (alignment === "below" || shouldDropBelow) && + top + action.height > bounds.height - MARGIN + ) { + top = rect.top - action.height - 6; } const maxLeft = Math.max(bounds.width - action.width - MARGIN, MARGIN); const maxTop = Math.max(bounds.height - action.height - MARGIN, MARGIN); @@ -184,6 +189,7 @@ export function installSelectionSettleGate( ): () => void { const view = doc.defaultView; let selecting = false; + let keyGesture = false; let frame = 0; // Keys that move or extend a selection. "a" only counts with a modifier, so @@ -224,6 +230,7 @@ export function installSelectionSettleGate( target instanceof Element && !!target.closest("[data-selection-comment-overlay]"); const startGesture = () => { + if (selecting) return; selecting = true; cancelFrame(); callbacks.onGestureStart?.(); @@ -231,6 +238,7 @@ export function installSelectionSettleGate( const cancelGesture = () => { if (!selecting) return; selecting = false; + keyGesture = false; cancelFrame(); callbacks.onGestureCancel?.(); }; @@ -250,14 +258,18 @@ export function installSelectionSettleGate( if (event instanceof MouseEvent && event.button > 0) return; if (!selecting) return; selecting = false; + keyGesture = false; settle(); }; const onKeyDown = (event: KeyboardEvent) => { - if (isSelectionKey(event)) startGesture(); + if (!isSelectionKey(event)) return; + keyGesture = true; + startGesture(); }; - const onKeyUp = (event: KeyboardEvent) => { - if (!selecting || !isSelectionKey(event)) return; + const onKeyUp = () => { + if (!selecting || !keyGesture) return; selecting = false; + keyGesture = false; settle(); }; const onSelectionChange = () => { From 70016bf08da77dd1623250226272d1d737992a3c Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Fri, 7 Aug 2026 15:38:28 +0200 Subject: [PATCH 7/7] fix(comments): satisfy canvas builder lint Add braces to the shared selection helpers and their canvas builder copy so the root Oxlint curly rule passes in the Trunk merge queue. Generated-By: PostHog Code Task-Id: c8373a88-43af-4fbd-958a-b6385a2d6849 --- .../canvas/packages/canvas_builder/build.mjs | 52 ++++++++++++++----- .../components/selectionCommentAction.ts | 52 ++++++++++++++----- 2 files changed, 78 insertions(+), 26 deletions(-) diff --git a/products/canvas/packages/canvas_builder/build.mjs b/products/canvas/packages/canvas_builder/build.mjs index 6ec5bbb49c73..dcc44d3c381f 100644 --- a/products/canvas/packages/canvas_builder/build.mjs +++ b/products/canvas/packages/canvas_builder/build.mjs @@ -41,9 +41,13 @@ function commentActionAnchorRect(rects, fallback) { const boxes = [] for (let index = 0; index < rects.length; index++) { const rect = rects[index] - if (rect.width > 0 || rect.height > 0) boxes.push(rect) + if (rect.width > 0 || rect.height > 0) { + boxes.push(rect) + } + } + if (boxes.length === 0) { + return fallback } - if (boxes.length === 0) return fallback const EPSILON = 0.5 const area = (box) => box.width * box.height const encloses = (outer, inner) => @@ -97,7 +101,9 @@ function installSelectionSettleGate(doc, callbacks) { } const cancelFrame = () => { - if (frame && view?.cancelAnimationFrame) view.cancelAnimationFrame(frame) + if (frame && view?.cancelAnimationFrame) { + view.cancelAnimationFrame(frame) + } frame = 0 } const settle = () => { @@ -116,13 +122,17 @@ function installSelectionSettleGate(doc, callbacks) { } const inActionUi = (target) => target instanceof Element && !!target.closest('[data-selection-comment-overlay]') const startGesture = () => { - if (selecting) return + if (selecting) { + return + } selecting = true cancelFrame() callbacks.onGestureStart?.() } const cancelGesture = () => { - if (!selecting) return + if (!selecting) { + return + } selecting = false keyGesture = false cancelFrame() @@ -131,35 +141,51 @@ function installSelectionSettleGate(doc, callbacks) { const onPointerDown = (event) => { // Secondary buttons open menus; they don't select. - if (event instanceof MouseEvent && event.button > 0) return - if (inActionUi(event.target)) return + if (event instanceof MouseEvent && event.button > 0) { + return + } + if (inActionUi(event.target)) { + return + } startGesture() } // Catches drags whose pointerdown we never saw, and keyboard selections. const onSelectStart = (event) => { - if (inActionUi(event.target)) return + if (inActionUi(event.target)) { + return + } startGesture() } const onPointerUp = (event) => { - if (event instanceof MouseEvent && event.button > 0) return - if (!selecting) return + if (event instanceof MouseEvent && event.button > 0) { + return + } + if (!selecting) { + return + } selecting = false keyGesture = false settle() } const onKeyDown = (event) => { - if (!isSelectionKey(event)) return + if (!isSelectionKey(event)) { + return + } keyGesture = true startGesture() } const onKeyUp = () => { - if (!selecting || !keyGesture) return + if (!selecting || !keyGesture) { + return + } selecting = false keyGesture = false settle() } const onSelectionChange = () => { - if (selecting) return + if (selecting) { + return + } callbacks.onIdleSelectionChange?.() } diff --git a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts index f1b1e554fa0a..0818d0ba9ee8 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts +++ b/products/desktop/packages/ui/src/features/sessions/components/selectionCommentAction.ts @@ -97,9 +97,13 @@ export function commentActionAnchorRect( const boxes: T[] = []; for (let index = 0; index < rects.length; index++) { const rect = rects[index]; - if (rect.width > 0 || rect.height > 0) boxes.push(rect); + if (rect.width > 0 || rect.height > 0) { + boxes.push(rect); + } + } + if (boxes.length === 0) { + return fallback; } - if (boxes.length === 0) return fallback; const EPSILON = 0.5; const area = (box: T) => box.width * box.height; const encloses = (outer: T, inner: T) => @@ -209,7 +213,9 @@ export function installSelectionSettleGate( }; const cancelFrame = () => { - if (frame && view?.cancelAnimationFrame) view.cancelAnimationFrame(frame); + if (frame && view?.cancelAnimationFrame) { + view.cancelAnimationFrame(frame); + } frame = 0; }; const settle = () => { @@ -230,13 +236,17 @@ export function installSelectionSettleGate( target instanceof Element && !!target.closest("[data-selection-comment-overlay]"); const startGesture = () => { - if (selecting) return; + if (selecting) { + return; + } selecting = true; cancelFrame(); callbacks.onGestureStart?.(); }; const cancelGesture = () => { - if (!selecting) return; + if (!selecting) { + return; + } selecting = false; keyGesture = false; cancelFrame(); @@ -245,35 +255,51 @@ export function installSelectionSettleGate( const onPointerDown = (event: Event) => { // Secondary buttons open menus; they don't select. - if (event instanceof MouseEvent && event.button > 0) return; - if (inActionUi(event.target)) return; + if (event instanceof MouseEvent && event.button > 0) { + return; + } + if (inActionUi(event.target)) { + return; + } startGesture(); }; // Catches drags whose pointerdown we never saw, and keyboard selections. const onSelectStart = (event: Event) => { - if (inActionUi(event.target)) return; + if (inActionUi(event.target)) { + return; + } startGesture(); }; const onPointerUp = (event: Event) => { - if (event instanceof MouseEvent && event.button > 0) return; - if (!selecting) return; + if (event instanceof MouseEvent && event.button > 0) { + return; + } + if (!selecting) { + return; + } selecting = false; keyGesture = false; settle(); }; const onKeyDown = (event: KeyboardEvent) => { - if (!isSelectionKey(event)) return; + if (!isSelectionKey(event)) { + return; + } keyGesture = true; startGesture(); }; const onKeyUp = () => { - if (!selecting || !keyGesture) return; + if (!selecting || !keyGesture) { + return; + } selecting = false; keyGesture = false; settle(); }; const onSelectionChange = () => { - if (selecting) return; + if (selecting) { + return; + } callbacks.onIdleSelectionChange?.(); };