diff --git a/ui/src/components/DocumentAnnotationLayer.test.tsx b/ui/src/components/DocumentAnnotationLayer.test.tsx index 4e49a5424ef..fbc075fe481 100644 --- a/ui/src/components/DocumentAnnotationLayer.test.tsx +++ b/ui/src/components/DocumentAnnotationLayer.test.tsx @@ -30,6 +30,13 @@ function makeRect(left: number, top: number, width: number, height: number): DOM } as DOMRect; } +function makeRange(rects: DOMRect[], commonAncestorContainer: Node = document.createTextNode("")): Range { + return { + commonAncestorContainer, + getClientRects: () => rects, + } as unknown as Range; +} + describe("DocumentAnnotationLayer", () => { let container: HTMLDivElement; let rectSpy: ReturnType; @@ -38,11 +45,7 @@ describe("DocumentAnnotationLayer", () => { beforeEach(() => { container = document.createElement("div"); document.body.appendChild(container); - mockRangesForNormalizedSpan.mockReturnValue([ - { - getClientRects: () => [makeRect(8, 12, 80, 18)], - }, - ]); + mockRangesForNormalizedSpan.mockReturnValue([makeRange([makeRect(8, 12, 80, 18)])]); rectSpy = vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue(makeRect(0, 0, 400, 300)); }); @@ -96,4 +99,40 @@ describe("DocumentAnnotationLayer", () => { expect(backgroundClasses.some((className) => className.startsWith("dark:bg-yellow-"))).toBe(true); } }); + + it("does not render highlights for text clipped by folded document content", async () => { + const body = document.createElement("div"); + const clippedContent = document.createElement("div"); + clippedContent.className = "fold-curtain__content"; + const hiddenText = document.createTextNode("Hidden folded text"); + clippedContent.appendChild(hiddenText); + body.appendChild(clippedContent); + mockRangesForNormalizedSpan.mockReturnValue([makeRange([makeRect(8, 60, 80, 18)], hiddenText)]); + rectSpy.mockImplementation(function (this: HTMLElement) { + if (this === clippedContent) return makeRect(0, 0, 400, 40); + return makeRect(0, 0, 400, 120); + }); + root = createRoot(container); + + await act(async () => { + root?.render( + , + ); + await new Promise((resolve) => window.requestAnimationFrame(resolve)); + }); + + expect(container.querySelector(".paperclip-doc-annotation-highlight")).toBeNull(); + expect(container.querySelector(".paperclip-doc-annotation-hit-target")).toBeNull(); + }); }); diff --git a/ui/src/components/DocumentAnnotationLayer.tsx b/ui/src/components/DocumentAnnotationLayer.tsx index 98b67f70bef..4c2bc7febee 100644 --- a/ui/src/components/DocumentAnnotationLayer.tsx +++ b/ui/src/components/DocumentAnnotationLayer.tsx @@ -67,6 +67,56 @@ interface ToolbarPosition { left: number; } +function elementFromNode(node: Node | null | undefined): HTMLElement | null { + if (!node) return null; + if (node instanceof HTMLElement) return node; + const parent = node.parentElement; + return parent instanceof HTMLElement ? parent : null; +} + +function intersectRects(a: DOMRect, b: DOMRect): DOMRect | null { + const left = Math.max(a.left, b.left); + const top = Math.max(a.top, b.top); + const right = Math.min(a.right, b.right); + const bottom = Math.min(a.bottom, b.bottom); + if (right <= left || bottom <= top) return null; + return { + x: left, + y: top, + left, + top, + right, + bottom, + width: right - left, + height: bottom - top, + toJSON: () => ({}), + } as DOMRect; +} + +function clipsOverflow(element: HTMLElement) { + if (element.classList.contains("fold-curtain__content")) return true; + if (typeof window === "undefined" || typeof window.getComputedStyle !== "function") return false; + const style = window.getComputedStyle(element); + return [style.overflow, style.overflowX, style.overflowY].some((value) => + value === "hidden" || value === "clip" || value === "auto" || value === "scroll", + ); +} + +function visibleClipRectForRange(range: Range, container: HTMLElement): DOMRect | null { + let clipRect = container.getBoundingClientRect(); + let element = elementFromNode(range.commonAncestorContainer); + while (element) { + if (clipsOverflow(element)) { + const nextClipRect = intersectRects(clipRect, element.getBoundingClientRect()); + if (!nextClipRect) return null; + clipRect = nextClipRect; + } + if (element === container) break; + element = element.parentElement; + } + return clipRect; +} + export function DocumentAnnotationLayer({ containerRef, markdown, @@ -84,7 +134,6 @@ export function DocumentAnnotationLayer({ const [highlightRects, setHighlightRects] = useState([]); const [toolbarPosition, setToolbarPosition] = useState(null); const overlayRef = useRef(null); - const layerContainerRef = useRef(null); const lastCaptureSelectionRequestIdRef = useRef(0); const visibleThreads = useMemo(() => { @@ -109,16 +158,20 @@ export function DocumentAnnotationLayer({ }); const startIndex = next.length; for (const range of ranges) { + const visibleClipRect = visibleClipRectForRange(range, container); + if (!visibleClipRect) continue; for (const rect of Array.from(range.getClientRects())) { if (rect.width === 0 || rect.height === 0) continue; + const visibleRect = intersectRects(rect, visibleClipRect); + if (!visibleRect) continue; next.push({ threadId: thread.id, status: thread.status, anchorState: thread.anchorState, - top: rect.top - overlayRect.top, - left: rect.left - overlayRect.left, - width: rect.width, - height: rect.height, + top: visibleRect.top - overlayRect.top, + left: visibleRect.left - overlayRect.left, + width: visibleRect.width, + height: visibleRect.height, isTail: false, }); } @@ -161,10 +214,10 @@ export function DocumentAnnotationLayer({ const mutationObserver = typeof window.MutationObserver === "function" && container ? new window.MutationObserver((mutations) => { - const layer = layerContainerRef.current; - const onlyLayerMutations = layer - ? mutations.every((mutation) => layer.contains(mutation.target)) - : false; + const onlyLayerMutations = mutations.every((mutation) => { + const target = elementFromNode(mutation.target); + return !!target?.closest(".paperclip-doc-annotation-layer, .paperclip-doc-annotation-visual-layer"); + }); if (!onlyLayerMutations) schedule(); }) : null; @@ -245,16 +298,49 @@ export function DocumentAnnotationLayer({ }; return ( - + ); } diff --git a/ui/src/components/IssueDocumentAnnotations.tsx b/ui/src/components/IssueDocumentAnnotations.tsx index a1932d6b23d..57ab9f13119 100644 --- a/ui/src/components/IssueDocumentAnnotations.tsx +++ b/ui/src/components/IssueDocumentAnnotations.tsx @@ -287,7 +287,9 @@ export function IssueDocumentAnnotations({ className="relative min-w-0" data-testid={`document-annotation-body-${doc.key}`} > - {children} +
+ {children} +
{!historicalPreview && doc.latestRevisionId ? (