diff --git a/ui/src/components/DocumentAnnotationLayer.test.tsx b/ui/src/components/DocumentAnnotationLayer.test.tsx index 80c6a010d9a..3c8296b7e1e 100644 --- a/ui/src/components/DocumentAnnotationLayer.test.tsx +++ b/ui/src/components/DocumentAnnotationLayer.test.tsx @@ -7,10 +7,12 @@ import { DocumentAnnotationLayer } from "./DocumentAnnotationLayer"; const mockBuildAnchorFromContainerSelection = vi.hoisted(() => vi.fn()); const mockGetContainerTextOffset = vi.hoisted(() => vi.fn()); const mockRangesForNormalizedSpan = vi.hoisted(() => vi.fn()); +const mockIsCoarsePointerDevice = vi.hoisted(() => vi.fn(() => false)); vi.mock("@/lib/document-annotation-selection", () => ({ buildAnchorFromContainerSelection: mockBuildAnchorFromContainerSelection, getContainerTextOffset: mockGetContainerTextOffset, + isCoarsePointerDevice: mockIsCoarsePointerDevice, rangesForNormalizedSpan: mockRangesForNormalizedSpan, })); @@ -54,6 +56,7 @@ describe("DocumentAnnotationLayer", () => { document.body.appendChild(container); mockRangesForNormalizedSpan.mockReturnValue([makeRange([makeRect(8, 12, 80, 18)])]); rectSpy = vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue(makeRect(0, 0, 400, 300)); + mockIsCoarsePointerDevice.mockReturnValue(false); }); afterEach(async () => { @@ -297,4 +300,249 @@ describe("DocumentAnnotationLayer", () => { (globalThis as { CSS?: unknown }).CSS = originalCss; (globalThis as { Highlight?: unknown }).Highlight = originalHighlight; }); + + it("captures a coarse-pointer selection once after selectionchange settles", async () => { + mockIsCoarsePointerDevice.mockReturnValue(true); + const body = document.createElement("div"); + const textNode = document.createTextNode("Select this text."); + body.appendChild(textNode); + const range = document.createRange(); + range.setStart(textNode, 0); + range.setEnd(textNode, 6); + range.getBoundingClientRect = () => makeRect(12, 24, 64, 18); + vi.spyOn(window, "getSelection").mockReturnValue({ + rangeCount: 1, + isCollapsed: false, + getRangeAt: () => range, + } as unknown as Selection); + mockGetContainerTextOffset.mockReturnValue({ + startOffset: 0, + endOffset: 6, + containerText: "Select this text.", + selectedText: "Select", + }); + mockBuildAnchorFromContainerSelection.mockReturnValue({ + selector: { + quote: { exact: "Select", prefix: "", suffix: " this" }, + position: { normalizedStart: 0, normalizedEnd: 6, markdownStart: 0, markdownEnd: 6 }, + }, + }); + const onPendingAnchorChange = vi.fn(); + root = createRoot(container); + + await act(async () => { + root?.render( + , + ); + }); + + document.dispatchEvent(new Event("selectionchange")); + document.dispatchEvent(new Event("selectionchange")); + document.dispatchEvent(new Event("selectionchange")); + await new Promise((resolve) => setTimeout(resolve, 250)); + + expect(mockGetContainerTextOffset).not.toHaveBeenCalled(); + expect(mockBuildAnchorFromContainerSelection).not.toHaveBeenCalled(); + expect(onPendingAnchorChange).not.toHaveBeenCalled(); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 180)); + }); + + expect(mockGetContainerTextOffset).toHaveBeenCalledTimes(1); + expect(mockBuildAnchorFromContainerSelection).toHaveBeenCalledTimes(1); + expect(onPendingAnchorChange).toHaveBeenCalledTimes(1); + }); + + it("uses the short gesture-end delay and skips unchanged settled selections", async () => { + mockIsCoarsePointerDevice.mockReturnValue(true); + const body = document.createElement("div"); + const textNode = document.createTextNode("Select this text."); + body.appendChild(textNode); + const range = document.createRange(); + range.setStart(textNode, 0); + range.setEnd(textNode, 6); + range.getBoundingClientRect = () => makeRect(12, 24, 64, 18); + vi.spyOn(window, "getSelection").mockReturnValue({ + rangeCount: 1, + isCollapsed: false, + getRangeAt: () => range, + } as unknown as Selection); + mockGetContainerTextOffset.mockReturnValue({ + startOffset: 0, + endOffset: 6, + containerText: "Select this text.", + selectedText: "Select", + }); + mockBuildAnchorFromContainerSelection.mockReturnValue({ + selector: { + quote: { exact: "Select", prefix: "", suffix: " this" }, + position: { normalizedStart: 0, normalizedEnd: 6, markdownStart: 0, markdownEnd: 6 }, + }, + }); + const onPendingAnchorChange = vi.fn(); + root = createRoot(container); + + await act(async () => { + root?.render( + , + ); + }); + + document.dispatchEvent(new Event("selectionchange")); + document.dispatchEvent(new Event("pointerup")); + await new Promise((resolve) => setTimeout(resolve, 80)); + expect(mockBuildAnchorFromContainerSelection).not.toHaveBeenCalled(); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 60)); + }); + expect(mockBuildAnchorFromContainerSelection).toHaveBeenCalledTimes(1); + expect(onPendingAnchorChange).toHaveBeenCalledTimes(1); + + document.dispatchEvent(new Event("selectionchange")); + document.dispatchEvent(new Event("touchend")); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 140)); + }); + + expect(mockGetContainerTextOffset).toHaveBeenCalledTimes(1); + expect(mockBuildAnchorFromContainerSelection).toHaveBeenCalledTimes(1); + expect(onPendingAnchorChange).toHaveBeenCalledTimes(1); + }); + + it("places a near-top selection toolbar below the selection and within the viewport", async () => { + const body = document.createElement("div"); + const textNode = document.createTextNode("Select this text."); + body.appendChild(textNode); + const range = document.createRange(); + range.setStart(textNode, 0); + range.setEnd(textNode, 6); + range.getBoundingClientRect = () => makeRect(-40, 2, 40, 18); + vi.spyOn(window, "getSelection").mockReturnValue({ + rangeCount: 1, + isCollapsed: false, + getRangeAt: () => range, + } as unknown as Selection); + mockGetContainerTextOffset.mockReturnValue({ + startOffset: 0, + endOffset: 6, + containerText: "Select this text.", + selectedText: "Select", + }); + mockBuildAnchorFromContainerSelection.mockReturnValue({ + selector: { + quote: { exact: "Select", prefix: "", suffix: " this" }, + position: { normalizedStart: 0, normalizedEnd: 6, markdownStart: 0, markdownEnd: 6 }, + }, + }); + root = createRoot(container); + let capturedAnchor: React.ComponentProps["pendingAnchor"] = null; + const renderLayer = () => ( + { + capturedAnchor = anchor; + }} + onRequestComment={vi.fn()} + testWindow={{ innerWidth: 320, innerHeight: 640 }} + /> + ); + + await act(async () => { + root?.render(renderLayer()); + }); + await act(async () => { + document.dispatchEvent(new Event("selectionchange")); + }); + await act(async () => { + root?.render(renderLayer()); + }); + + const toolbar = container.querySelector('[data-testid="document-annotation-selection-toolbar"]'); + expect(toolbar?.style.top).toBe("28px"); + expect(toolbar?.style.left).toBe("8px"); + }); + + it("uses the settled pending anchor for an explicit shortcut request", async () => { + const body = document.createElement("div"); + const textNode = document.createTextNode("Select this text."); + body.appendChild(textNode); + const range = document.createRange(); + range.setStart(textNode, 0); + range.setEnd(textNode, 6); + range.getBoundingClientRect = () => makeRect(12, 24, 64, 18); + vi.spyOn(window, "getSelection").mockReturnValue({ + rangeCount: 1, + isCollapsed: false, + getRangeAt: () => range, + } as unknown as Selection); + mockGetContainerTextOffset.mockReturnValue({ + startOffset: 0, + endOffset: 6, + containerText: "Select this text.", + selectedText: "Select", + }); + mockBuildAnchorFromContainerSelection.mockReturnValue({ + selector: { + quote: { exact: "Select", prefix: "", suffix: " this" }, + position: { normalizedStart: 0, normalizedEnd: 6, markdownStart: 0, markdownEnd: 6 }, + }, + }); + const onRequestComment = vi.fn(); + let pendingAnchor: React.ComponentProps["pendingAnchor"] = null; + root = createRoot(container); + const renderLayer = (captureSelectionRequestId: number) => ( + { + pendingAnchor = anchor; + }} + onRequestComment={onRequestComment} + captureSelectionRequestId={captureSelectionRequestId} + /> + ); + + await act(async () => { + root?.render(renderLayer(0)); + }); + await act(async () => { + document.dispatchEvent(new Event("selectionchange")); + }); + await act(async () => { + root?.render(renderLayer(1)); + }); + + expect(mockBuildAnchorFromContainerSelection).toHaveBeenCalledTimes(1); + expect(onRequestComment).toHaveBeenCalledTimes(1); + expect(onRequestComment).toHaveBeenCalledWith(pendingAnchor); + }); }); diff --git a/ui/src/components/DocumentAnnotationLayer.tsx b/ui/src/components/DocumentAnnotationLayer.tsx index d9418aae7fe..b1077ace3a2 100644 --- a/ui/src/components/DocumentAnnotationLayer.tsx +++ b/ui/src/components/DocumentAnnotationLayer.tsx @@ -9,6 +9,7 @@ import { cn } from "@/lib/utils"; import { buildAnchorFromContainerSelection, getContainerTextOffset, + isCoarsePointerDevice, rangesForNormalizedSpan, } from "@/lib/document-annotation-selection"; import { @@ -66,6 +67,11 @@ export interface AnnotationLayerProps { /** Synthetic thread id used to render the in-progress (pending) comment highlight. */ const PENDING_HIGHLIGHT_THREAD_ID = "__paperclip-pending-annotation__"; +const COARSE_SELECTION_SETTLE_MS = 400; +const COARSE_GESTURE_END_CAPTURE_MS = 120; +const TOOLBAR_WIDTH = 160; +const TOOLBAR_HEIGHT = 36; +const TOOLBAR_VIEWPORT_GAP = 8; interface HighlightRect { threadId: string; @@ -86,6 +92,14 @@ interface ToolbarPosition { left: number; } +interface SelectionSnapshot { + startContainer: Node; + startOffset: number; + endContainer: Node; + endOffset: number; + selectedText: string; +} + type NativeHighlightKind = "open" | "focused" | "stale" | "resolved"; type NativeHighlightRanges = Record; @@ -241,12 +255,16 @@ export function DocumentAnnotationLayer({ hideResolved = true, captureSelectionRequestId, pendingHighlightText = null, + testWindow, }: AnnotationLayerProps) { const [highlightRects, setHighlightRects] = useState([]); const [hoveredThreadId, setHoveredThreadId] = useState(null); const [toolbarPosition, setToolbarPosition] = useState(null); const overlayRef = useRef(null); const lastCaptureSelectionRequestIdRef = useRef(0); + const selectionCaptureTimeoutRef = useRef(null); + const lastProcessedSelectionRef = useRef(null); + const hasProcessedSelectionRef = useRef(false); const reactId = useId(); const nativeHighlightInstanceId = useMemo( () => `document-annotation-${reactId.replace(/[^a-zA-Z0-9_-]/g, "")}`, @@ -410,7 +428,7 @@ export function DocumentAnnotationLayer({ }; }, [computeHighlightRects, containerRef, selectionDebugEnabled]); - const captureSelection = useCallback((): PendingAnchor | null => { + const captureSelection = useCallback((): PendingAnchor | null | undefined => { const container = containerRef.current; const overlay = overlayRef.current; if (!container || !overlay) return null; @@ -419,23 +437,88 @@ export function DocumentAnnotationLayer({ const range = selection.getRangeAt(0); if (!container.contains(range.commonAncestorContainer)) return null; if (selectionTouchesEditableElement(container, range)) return null; + const snapshot: SelectionSnapshot = { + startContainer: range.startContainer, + startOffset: range.startOffset, + endContainer: range.endContainer, + endOffset: range.endOffset, + selectedText: range.toString(), + }; + const previous = lastProcessedSelectionRef.current; + if ( + previous + && previous.startContainer === snapshot.startContainer + && previous.startOffset === snapshot.startOffset + && previous.endContainer === snapshot.endContainer + && previous.endOffset === snapshot.endOffset + && previous.selectedText === snapshot.selectedText + ) { + return undefined; + } + lastProcessedSelectionRef.current = snapshot; + hasProcessedSelectionRef.current = true; const containerOffset = getContainerTextOffset(container, range); if (!containerOffset) return null; const anchor = buildAnchorFromContainerSelection({ markdown, containerOffset }); if (!anchor) return null; const overlayRect = overlay.getBoundingClientRect(); const rect = range.getBoundingClientRect(); - const top = Math.max(0, rect.top - overlayRect.top - 36); - const left = Math.max(0, rect.left - overlayRect.left + rect.width / 2 - 80); + const visualViewport = window.visualViewport; + const viewportLeft = visualViewport?.offsetLeft ?? 0; + const viewportTop = visualViewport?.offsetTop ?? 0; + const viewportWidth = visualViewport?.width ?? testWindow?.innerWidth ?? window.innerWidth; + const viewportHeight = visualViewport?.height ?? testWindow?.innerHeight ?? window.innerHeight; + const preferredTop = rect.top - TOOLBAR_HEIGHT - TOOLBAR_VIEWPORT_GAP; + const viewportBottom = viewportTop + viewportHeight; + const screenTop = preferredTop < viewportTop + TOOLBAR_VIEWPORT_GAP + ? Math.min(rect.bottom + TOOLBAR_VIEWPORT_GAP, viewportBottom - TOOLBAR_HEIGHT - TOOLBAR_VIEWPORT_GAP) + : preferredTop; + const preferredLeft = rect.left + rect.width / 2 - TOOLBAR_WIDTH / 2; + const screenLeft = Math.min( + Math.max(preferredLeft, viewportLeft + TOOLBAR_VIEWPORT_GAP), + viewportLeft + viewportWidth - TOOLBAR_WIDTH - TOOLBAR_VIEWPORT_GAP, + ); + const top = Math.max(0, screenTop - overlayRect.top); + const left = Math.max(0, screenLeft - overlayRect.left); setToolbarPosition({ top, left }); return { selector: anchor.selector, selectedText: containerOffset.selectedText, }; - }, [containerRef, markdown]); + }, [containerRef, markdown, testWindow]); useEffect(() => { if (typeof document === "undefined") return; + const coarsePointer = isCoarsePointerDevice(); + const clearSelectionCaptureTimeout = () => { + if (selectionCaptureTimeoutRef.current === null) return; + window.clearTimeout(selectionCaptureTimeoutRef.current); + selectionCaptureTimeoutRef.current = null; + }; + const applySelectionChange = () => { + const captureStartedAt = selectionDebugEnabled ? performance.now() : 0; + const anchor = captureSelection(); + if (anchor === undefined) return; + if (selectionDebugEnabled) { + recordCaptureSelection(performance.now() - captureStartedAt, Boolean(anchor)); + } + if (!anchor) { + if (hasProcessedSelectionRef.current && lastProcessedSelectionRef.current === null) return; + lastProcessedSelectionRef.current = null; + hasProcessedSelectionRef.current = true; + onPendingAnchorChange(null); + setToolbarPosition(null); + return; + } + onPendingAnchorChange(anchor); + }; + const scheduleSelectionCapture = (delay: number) => { + clearSelectionCaptureTimeout(); + selectionCaptureTimeoutRef.current = window.setTimeout(() => { + selectionCaptureTimeoutRef.current = null; + applySelectionChange(); + }, delay); + }; const handleSelectionChange = () => { const selection = window.getSelection(); const range = selection && selection.rangeCount > 0 ? selection.getRangeAt(0) : null; @@ -443,20 +526,27 @@ export function DocumentAnnotationLayer({ selection && !selection.isCollapsed && range && containerRef.current?.contains(range.commonAncestorContainer), ); if (selectionDebugEnabled) recordSelectionChange(selectionIsActive); - const captureStartedAt = selectionDebugEnabled ? performance.now() : 0; - const anchor = captureSelection(); - if (selectionDebugEnabled) { - recordCaptureSelection(performance.now() - captureStartedAt, Boolean(anchor)); - } - if (!anchor) { - onPendingAnchorChange(null); - setToolbarPosition(null); + if (coarsePointer) { + scheduleSelectionCapture(COARSE_SELECTION_SETTLE_MS); return; } - onPendingAnchorChange(anchor); + applySelectionChange(); + }; + const handleGestureEnd = () => { + if (selectionCaptureTimeoutRef.current === null) return; + scheduleSelectionCapture(COARSE_GESTURE_END_CAPTURE_MS); }; document.addEventListener("selectionchange", handleSelectionChange); - return () => document.removeEventListener("selectionchange", handleSelectionChange); + if (coarsePointer) { + document.addEventListener("pointerup", handleGestureEnd); + document.addEventListener("touchend", handleGestureEnd); + } + return () => { + clearSelectionCaptureTimeout(); + document.removeEventListener("selectionchange", handleSelectionChange); + document.removeEventListener("pointerup", handleGestureEnd); + document.removeEventListener("touchend", handleGestureEnd); + }; }, [captureSelection, containerRef, onPendingAnchorChange, selectionDebugEnabled]); useEffect(() => { @@ -465,11 +555,11 @@ export function DocumentAnnotationLayer({ if (lastCaptureSelectionRequestIdRef.current === captureSelectionRequestId) return; lastCaptureSelectionRequestIdRef.current = captureSelectionRequestId; const anchor = captureSelection(); - if (anchor) { - onPendingAnchorChange(anchor); - onRequestComment(anchor); - } - }, [captureSelectionRequestId, captureSelection, onPendingAnchorChange, onRequestComment]); + const requestedAnchor = anchor === undefined ? pendingAnchor : anchor; + if (!requestedAnchor) return; + if (anchor !== undefined) onPendingAnchorChange(anchor); + onRequestComment(requestedAnchor); + }, [captureSelectionRequestId, captureSelection, onPendingAnchorChange, onRequestComment, pendingAnchor]); const handleAddComment = () => { if (pendingAnchor) onRequestComment(pendingAnchor); diff --git a/ui/src/components/DocumentAnnotationPanel.tsx b/ui/src/components/DocumentAnnotationPanel.tsx index e2c030ae691..c92684b2659 100644 --- a/ui/src/components/DocumentAnnotationPanel.tsx +++ b/ui/src/components/DocumentAnnotationPanel.tsx @@ -26,6 +26,7 @@ import { cn, relativeTime } from "@/lib/utils"; import { documentAnnotationsApi, type DocumentAnnotationTarget } from "@/api/document-annotations"; import { authApi } from "@/api/auth"; import { queryKeys } from "@/lib/queryKeys"; +import { hasActiveNativeSelection, isCoarsePointerDevice } from "@/lib/document-annotation-selection"; import { AgentIcon } from "./AgentIconPicker"; import { deriveInitials } from "./Identity"; import { MarkdownBody } from "./MarkdownBody"; @@ -71,6 +72,7 @@ export function DocumentAnnotationPanel(props: AnnotationPanelProps) { event.preventDefault()} className="paperclip-doc-annotation-sheet z-(--z-60) flex max-h-(--sz-88vh) flex-col rounded-none border-t border-border bg-popover p-0 text-popover-foreground shadow-2xl" > @@ -309,10 +311,10 @@ function AnnotationPanelBody(props: AnnotationPanelProps) { }, [props.open]); useEffect(() => { - if (props.pendingAnchor && props.open) { - composerRef.current?.focus(); - } - }, [props.open, props.pendingAnchor]); + if (!props.pendingAnchor || !props.open || props.isMobile) return; + if (isCoarsePointerDevice() || hasActiveNativeSelection()) return; + composerRef.current?.focus(); + }, [props.isMobile, props.open, props.pendingAnchor]); // Keep the comment list congruent with the document: when a thread becomes // focused — whether by clicking its highlight in the doc or by adding a new diff --git a/ui/src/components/IssueDocumentAnnotations.test.tsx b/ui/src/components/IssueDocumentAnnotations.test.tsx index 1af735cd53c..4998e87badb 100644 --- a/ui/src/components/IssueDocumentAnnotations.test.tsx +++ b/ui/src/components/IssueDocumentAnnotations.test.tsx @@ -1011,4 +1011,51 @@ describe("IssueDocumentAnnotations", () => { }); } }); + + it("does not auto-focus the composer after a coarse-pointer selection request", async () => { + const originalMatchMedia = window.matchMedia; + Object.defineProperty(window, "matchMedia", { + configurable: true, + value: vi.fn().mockImplementation((query: string) => ({ + matches: query === "(pointer: coarse)" || query === "(max-width: 767px)", + media: query, + onchange: null, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + addListener: vi.fn(), + removeListener: vi.fn(), + dispatchEvent: vi.fn(), + })), + }); + const focusSpy = vi.spyOn(HTMLTextAreaElement.prototype, "focus").mockImplementation(() => {}); + mockAnnotationsApi.list.mockResolvedValue([]); + const root = createRoot(container); + const queryClient = makeQueryClient(); + const doc = makeDoc(); + + try { + await act(async () => { + root.render( + + + , + ); + }); + await flush(); + await flush(); + + const selectButton = container.querySelector('[data-testid="mock-annotation-selection"]'); + await act(async () => selectButton?.click()); + await flush(); + + expect(container.querySelector('[data-testid="document-annotation-composer"]')).not.toBeNull(); + expect(focusSpy).not.toHaveBeenCalled(); + } finally { + focusSpy.mockRestore(); + Object.defineProperty(window, "matchMedia", { + configurable: true, + value: originalMatchMedia, + }); + } + }); }); diff --git a/ui/src/lib/document-annotation-selection.ts b/ui/src/lib/document-annotation-selection.ts index dfaf5c7906a..44349f469d9 100644 --- a/ui/src/lib/document-annotation-selection.ts +++ b/ui/src/lib/document-annotation-selection.ts @@ -19,6 +19,21 @@ export interface ContainerTextOffset { selectedText: string; } +export function isCoarsePointerDevice(targetWindow: Window = window): boolean { + const navigatorWithLegacyTouchPoints = targetWindow.navigator as Navigator & { + msMaxTouchPoints?: number; + }; + if ((navigatorWithLegacyTouchPoints.maxTouchPoints ?? 0) > 0) return true; + if ((navigatorWithLegacyTouchPoints.msMaxTouchPoints ?? 0) > 0) return true; + return typeof targetWindow.matchMedia === "function" + && targetWindow.matchMedia("(pointer: coarse)").matches; +} + +export function hasActiveNativeSelection(targetWindow: Window = window): boolean { + const selection = targetWindow.getSelection(); + return Boolean(selection && selection.rangeCount > 0 && !selection.isCollapsed); +} + export function getContainerTextOffset( container: HTMLElement, range: Range,