diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/01-desktop-open-focused.png b/design/PAP-9436-doc-annotation-ui/screenshots/01-desktop-open-focused.png deleted file mode 100644 index 08d81c06aa7..00000000000 Binary files a/design/PAP-9436-doc-annotation-ui/screenshots/01-desktop-open-focused.png and /dev/null differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/01-integrated-desktop-open.png b/design/PAP-9436-doc-annotation-ui/screenshots/01-integrated-desktop-open.png new file mode 100644 index 00000000000..cc44b18358e Binary files /dev/null and b/design/PAP-9436-doc-annotation-ui/screenshots/01-integrated-desktop-open.png differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/02-desktop-resolved-focused.png b/design/PAP-9436-doc-annotation-ui/screenshots/02-desktop-resolved-focused.png deleted file mode 100644 index 0fae21dbb85..00000000000 Binary files a/design/PAP-9436-doc-annotation-ui/screenshots/02-desktop-resolved-focused.png and /dev/null differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/02-integrated-desktop-zero-count.png b/design/PAP-9436-doc-annotation-ui/screenshots/02-integrated-desktop-zero-count.png new file mode 100644 index 00000000000..8482ccc3536 Binary files /dev/null and b/design/PAP-9436-doc-annotation-ui/screenshots/02-integrated-desktop-zero-count.png differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/03-desktop-stale-focused.png b/design/PAP-9436-doc-annotation-ui/screenshots/03-desktop-stale-focused.png deleted file mode 100644 index 31beb55195c..00000000000 Binary files a/design/PAP-9436-doc-annotation-ui/screenshots/03-desktop-stale-focused.png and /dev/null differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/03-integrated-desktop-edit-mode.png b/design/PAP-9436-doc-annotation-ui/screenshots/03-integrated-desktop-edit-mode.png new file mode 100644 index 00000000000..ee6e2b42bbf Binary files /dev/null and b/design/PAP-9436-doc-annotation-ui/screenshots/03-integrated-desktop-edit-mode.png differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/04-desktop-orphaned-focused.png b/design/PAP-9436-doc-annotation-ui/screenshots/04-desktop-orphaned-focused.png deleted file mode 100644 index c99d7b989d0..00000000000 Binary files a/design/PAP-9436-doc-annotation-ui/screenshots/04-desktop-orphaned-focused.png and /dev/null differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/04-integrated-desktop-dirty-draft.png b/design/PAP-9436-doc-annotation-ui/screenshots/04-integrated-desktop-dirty-draft.png new file mode 100644 index 00000000000..3e4b94ec0f4 Binary files /dev/null and b/design/PAP-9436-doc-annotation-ui/screenshots/04-integrated-desktop-dirty-draft.png differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/05-dirty-draft-disabled.png b/design/PAP-9436-doc-annotation-ui/screenshots/05-dirty-draft-disabled.png deleted file mode 100644 index 8f064ba5576..00000000000 Binary files a/design/PAP-9436-doc-annotation-ui/screenshots/05-dirty-draft-disabled.png and /dev/null differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/05-integrated-mobile-sheet.png b/design/PAP-9436-doc-annotation-ui/screenshots/05-integrated-mobile-sheet.png new file mode 100644 index 00000000000..b8326f054ab Binary files /dev/null and b/design/PAP-9436-doc-annotation-ui/screenshots/05-integrated-mobile-sheet.png differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/06-mobile-bottom-sheet.png b/design/PAP-9436-doc-annotation-ui/screenshots/06-mobile-bottom-sheet.png deleted file mode 100644 index b649fd0c9d3..00000000000 Binary files a/design/PAP-9436-doc-annotation-ui/screenshots/06-mobile-bottom-sheet.png and /dev/null differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/10-states-open-focused.png b/design/PAP-9436-doc-annotation-ui/screenshots/10-states-open-focused.png new file mode 100644 index 00000000000..291d720a8f4 Binary files /dev/null and b/design/PAP-9436-doc-annotation-ui/screenshots/10-states-open-focused.png differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/11-states-resolved-focused.png b/design/PAP-9436-doc-annotation-ui/screenshots/11-states-resolved-focused.png new file mode 100644 index 00000000000..bcd689d6369 Binary files /dev/null and b/design/PAP-9436-doc-annotation-ui/screenshots/11-states-resolved-focused.png differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/12-states-stale-focused.png b/design/PAP-9436-doc-annotation-ui/screenshots/12-states-stale-focused.png new file mode 100644 index 00000000000..5f8a1c2b334 Binary files /dev/null and b/design/PAP-9436-doc-annotation-ui/screenshots/12-states-stale-focused.png differ diff --git a/design/PAP-9436-doc-annotation-ui/screenshots/13-states-orphaned-focused.png b/design/PAP-9436-doc-annotation-ui/screenshots/13-states-orphaned-focused.png new file mode 100644 index 00000000000..1490c15f84f Binary files /dev/null and b/design/PAP-9436-doc-annotation-ui/screenshots/13-states-orphaned-focused.png differ diff --git a/scripts/screenshot-doc-annotations.mjs b/scripts/screenshot-doc-annotations.mjs index 81a4f4c883f..ba616d44a1e 100644 --- a/scripts/screenshot-doc-annotations.mjs +++ b/scripts/screenshot-doc-annotations.mjs @@ -54,29 +54,41 @@ async function main() { const browser = await chromium.launch(); try { const stories = [ - { id: "product-documents-annotations--desktop-open-focused", file: "01-desktop-open-focused.png", width: 1280, height: 900 }, - { id: "product-documents-annotations--desktop-resolved-focused", file: "02-desktop-resolved-focused.png", width: 1280, height: 900 }, - { id: "product-documents-annotations--desktop-stale-focused", file: "03-desktop-stale-focused.png", width: 1280, height: 900 }, - { id: "product-documents-annotations--desktop-orphaned-focused", file: "04-desktop-orphaned-focused.png", width: 1280, height: 900 }, - { id: "product-documents-annotations--dirty-draft-disables-new-comments", file: "05-dirty-draft-disabled.png", width: 720, height: 720 }, - { id: "product-documents-annotations--mobile-bottom-sheet-view", file: "06-mobile-bottom-sheet.png", width: 375, height: 812 }, + // INTEGRATED — these are the captures the UX gate requires. They render + // IssueDocumentsSection chrome (count chip in header row, body + side panel + // wired together, edit mode wrapping the layer, real shadcn Sheet on mobile). + { id: "product-documents-annotations--integrated-desktop-open", file: "01-integrated-desktop-open.png", width: 1440, height: 900 }, + { id: "product-documents-annotations--integrated-desktop-zero-comments", file: "02-integrated-desktop-zero-count.png", width: 1440, height: 900 }, + { id: "product-documents-annotations--integrated-desktop-edit-mode", file: "03-integrated-desktop-edit-mode.png", width: 1440, height: 900 }, + { id: "product-documents-annotations--integrated-desktop-dirty-draft", file: "04-integrated-desktop-dirty-draft.png", width: 1440, height: 900 }, + { id: "product-documents-annotations--integrated-mobile-bottom-sheet", file: "05-integrated-mobile-sheet.png", width: 390, height: 844 }, + // ISOLATED — kept around for visual debugging of state pieces. + { id: "product-documents-annotations--desktop-open-focused", file: "10-states-open-focused.png", width: 1280, height: 900 }, + { id: "product-documents-annotations--desktop-resolved-focused", file: "11-states-resolved-focused.png", width: 1280, height: 900 }, + { id: "product-documents-annotations--desktop-stale-focused", file: "12-states-stale-focused.png", width: 1280, height: 900 }, + { id: "product-documents-annotations--desktop-orphaned-focused", file: "13-states-orphaned-focused.png", width: 1280, height: 900 }, ]; + const themeArg = (process.env.SCREENSHOT_THEME || "dark").toLowerCase(); + const theme = themeArg === "light" ? "light" : "dark"; + for (const story of stories) { const ctx = await browser.newContext({ viewport: { width: story.width, height: story.height }, deviceScaleFactor: 2, - colorScheme: "light", + colorScheme: theme, }); const page = await ctx.newPage(); - const url = `${baseUrl}?id=${story.id}&viewMode=story`; + const url = `${baseUrl}?id=${story.id}&viewMode=story&globals=theme:${theme}`; await page.goto(url, { waitUntil: "networkidle" }); - await page.evaluate(() => { - document.documentElement.style.colorScheme = "light"; - }); - await page.waitForTimeout(700); + await page.evaluate((appliedTheme) => { + document.documentElement.classList.toggle("dark", appliedTheme === "dark"); + document.documentElement.style.colorScheme = appliedTheme; + }, theme); + // Allow async query prefill + the layer's interval-driven layout pass to settle. + await page.waitForTimeout(900); const out = path.join(outDir, story.file); - await page.screenshot({ path: out, fullPage: true }); + await page.screenshot({ path: out, fullPage: false }); console.log("wrote", out); await ctx.close(); } diff --git a/ui/src/components/DocumentAnnotationLayer.tsx b/ui/src/components/DocumentAnnotationLayer.tsx index 52e709e91e4..5c32a4cfac8 100644 --- a/ui/src/components/DocumentAnnotationLayer.tsx +++ b/ui/src/components/DocumentAnnotationLayer.tsx @@ -1,5 +1,5 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from "react"; -import { MessageSquarePlus } from "lucide-react"; +import { AlertTriangle, MessageSquarePlus } from "lucide-react"; import type { DocumentAnnotationAnchorState, DocumentAnnotationThreadStatus, @@ -43,6 +43,11 @@ export interface AnnotationLayerProps { hideResolved?: boolean; /** Test-only: override window object for layout calculations. */ testWindow?: { innerWidth: number; innerHeight: number }; + /** + * When this number changes, re-read the current document selection and emit a + * pending anchor (used by the panel's "+ New comment on selection" / ⌘⇧M CTA). + */ + captureSelectionRequestId?: number; } interface HighlightRect { @@ -53,6 +58,8 @@ interface HighlightRect { left: number; width: number; height: number; + /** True for the last rect of this thread (used to anchor a glyph at the run end). */ + isTail: boolean; } interface ToolbarPosition { @@ -74,6 +81,7 @@ export function DocumentAnnotationLayer({ newCommentDisabled = false, newCommentDisabledReason = null, hideResolved = true, + captureSelectionRequestId, }: AnnotationLayerProps) { const [highlightRects, setHighlightRects] = useState([]); const [toolbarPosition, setToolbarPosition] = useState(null); @@ -100,6 +108,7 @@ export function DocumentAnnotationLayer({ container, selectedText: thread.selectedText, }); + const startIndex = next.length; for (const range of ranges) { for (const rect of Array.from(range.getClientRects())) { if (rect.width === 0 || rect.height === 0) continue; @@ -111,9 +120,13 @@ export function DocumentAnnotationLayer({ left: rect.left - overlayRect.left, width: rect.width, height: rect.height, + isTail: false, }); } } + if (next.length > startIndex) { + next[next.length - 1].isTail = true; + } } setHighlightRects(next); }, [containerRef, visibleThreads]); @@ -141,53 +154,53 @@ export function DocumentAnnotationLayer({ }; }, [computeHighlightRects]); + const captureSelection = useCallback((): PendingAnchor | null => { + const container = containerRef.current; + const overlay = overlayRef.current; + if (!container || !overlay) return null; + const selection = window.getSelection(); + if (!selection || selection.rangeCount === 0 || selection.isCollapsed) return null; + const range = selection.getRangeAt(0); + if (!container.contains(range.commonAncestorContainer)) return null; + 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); + setToolbarPosition({ top, left }); + return { + selector: anchor.selector, + selectedText: containerOffset.selectedText, + }; + }, [containerRef, markdown]); + useEffect(() => { if (typeof document === "undefined") return; const handleSelectionChange = () => { - const container = containerRef.current; - const overlay = overlayRef.current; - if (!container || !overlay) return; - const selection = window.getSelection(); - if (!selection || selection.rangeCount === 0 || selection.isCollapsed) { - onPendingAnchorChange(null); - setToolbarPosition(null); - return; - } - const range = selection.getRangeAt(0); - if (!container.contains(range.commonAncestorContainer)) { - onPendingAnchorChange(null); - setToolbarPosition(null); - return; - } - const containerOffset = getContainerTextOffset(container, range); - if (!containerOffset) { - onPendingAnchorChange(null); - setToolbarPosition(null); - return; - } - const anchor = buildAnchorFromContainerSelection({ - markdown, - containerOffset, - }); + const anchor = captureSelection(); if (!anchor) { onPendingAnchorChange(null); setToolbarPosition(null); return; } - onPendingAnchorChange({ - selector: anchor.selector, - selectedText: containerOffset.selectedText, - }); - - 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); - setToolbarPosition({ top, left }); + onPendingAnchorChange(anchor); }; document.addEventListener("selectionchange", handleSelectionChange); return () => document.removeEventListener("selectionchange", handleSelectionChange); - }, [containerRef, markdown, onPendingAnchorChange]); + }, [captureSelection, onPendingAnchorChange]); + + useEffect(() => { + if (captureSelectionRequestId === undefined) return; + if (captureSelectionRequestId === 0) return; + const anchor = captureSelection(); + if (anchor) { + onPendingAnchorChange(anchor); + onRequestComment(anchor); + } + }, [captureSelectionRequestId, captureSelection, onPendingAnchorChange, onRequestComment]); const handleAddComment = () => { if (pendingAnchor) onRequestComment(pendingAnchor); @@ -202,6 +215,8 @@ export function DocumentAnnotationLayer({
{highlightRects.map((rect, index) => { const isFocused = rect.threadId === focusedThreadId; + const isStale = rect.anchorState === "stale"; + const isResolved = rect.status === "resolved"; return ( -
+
{FILTERS.map((entry) => { const count = counts[entry.id]; const isActive = filter === entry.id; return ( - + {entry.label} + + {count} + + ); })}
@@ -235,6 +301,8 @@ export function DocumentAnnotationPanel(props: AnnotationPanelProps) { onCopyLink={() => copyAnnotationLink(props.documentKey, thread.id)} pendingReply={addReply.isPending && addReply.variables?.threadId === thread.id} pendingStatus={updateStatus.isPending && updateStatus.variables?.threadId === thread.id} + agentMap={props.agentMap} + userProfileMap={props.userProfileMap} /> ))} @@ -290,8 +358,29 @@ export function DocumentAnnotationPanel(props: AnnotationPanelProps) {
- ) : null} - + ) : ( +
+ +
+ )} + ); } @@ -307,6 +396,8 @@ function ThreadCard(props: { onCopyLink: () => void; pendingReply: boolean; pendingStatus: boolean; + agentMap?: ReadonlyMap>; + userProfileMap?: ReadonlyMap; }) { const { thread } = props; const statusVariant: { variant: "default" | "outline" | "secondary"; label: string } = @@ -329,8 +420,8 @@ function ThreadCard(props: { data-focused={props.expanded || undefined} aria-labelledby={`thread-quote-${thread.id}`} className={cn( - "rounded-md border bg-card transition-colors", - props.expanded ? "border-primary/60" : "border-border", + "rounded-md border border-border bg-card transition-colors", + props.expanded && "ring-1 ring-ring/70", thread.status === "resolved" && "bg-muted/30", )} tabIndex={0} @@ -360,6 +451,8 @@ function ThreadCard(props: { key={comment.id} comment={comment} focused={props.focusedCommentId === comment.id} + agentMap={props.agentMap} + userProfileMap={props.userProfileMap} /> ))}