From 46570ea7363721febefa95fd0662dc8bb8cc2d4b Mon Sep 17 00:00:00 2001 From: ali <117142933+muhammad-ali-e@users.noreply.github.com> Date: Thu, 29 Oct 2026 08:07:25 +0000 Subject: [PATCH 1/7] UN-4506 [FEAT] Show text and images in the v1 DocumentViewer, with a switcher slot - DocumentViewer takes any DocumentSource: a PDF (URL or bytes), an image ({kind: "image", url}) or text ({kind: "text", text}). - TextView shows text line by line with line numbers, keeping the layout's spacing, and highlights whole lines (0-based, as LLMWhisperer numbers them) in default, active and low-confidence tones; highlighted lines report clicks and can be scrolled to. - ImageView shows an image as a one-page document: the toolbar's zoom and fits apply (zoom 1 is its natural size), page highlights draw over it, and a failed load offers Retry. - The toolbar gains a leading slot for the document switcher, shown in every state so a failed document can be switched away from; paging shows only for PDFs and zoom only for PDFs and images. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R7trgcAzu6jxDnit3eeozm --- .../document-viewer/DocumentViewer.test.tsx | 48 +++++ .../document-viewer/DocumentViewer.tsx | 149 ++++++++++++---- .../document-viewer/ImageView.test.tsx | 64 +++++++ .../patterns/document-viewer/ImageView.tsx | 166 ++++++++++++++++++ .../document-viewer/TextView.test.tsx | 86 +++++++++ .../patterns/document-viewer/TextView.tsx | 152 ++++++++++++++++ .../document-viewer/ViewerToolbar.tsx | 124 +++++++------ frontend/src/v1/lib/pdf/types.ts | 22 +++ 8 files changed, 726 insertions(+), 85 deletions(-) create mode 100644 frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx create mode 100644 frontend/src/v1/components/patterns/document-viewer/ImageView.tsx create mode 100644 frontend/src/v1/components/patterns/document-viewer/TextView.test.tsx create mode 100644 frontend/src/v1/components/patterns/document-viewer/TextView.tsx diff --git a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx index b9ea9e4701..0f0e5cb1a2 100644 --- a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx @@ -188,3 +188,51 @@ it("runs a scroll asked for while loading once the pages are laid out", async () } } }); + +it("shows text line by line, with the switcher but no paging or zoom", () => { + render( + invoice.pdf} + />, + ); + expect(screen.getByRole("region", { name: "Document text" })).toBeTruthy(); + expect(screen.getByRole("button", { name: "invoice.pdf" })).toBeTruthy(); + expect(screen.queryByRole("button", { name: "Zoom in" })).toBeNull(); + expect(screen.queryByLabelText("Page")).toBeNull(); +}); + +it("keeps the switcher while a PDF is still loading or has failed", async () => { + render( + invoice.pdf} + />, + ); + expect(screen.getByRole("button", { name: "invoice.pdf" })).toBeTruthy(); + await act(async () => opening.reject(new Error("Network error"))); + expect(screen.getByRole("button", { name: "invoice.pdf" })).toBeTruthy(); + expect(screen.queryByRole("button", { name: "Zoom in" })).toBeNull(); +}); + +it("zooms an image with the toolbar, fitted like a page, without paging", () => { + render(); + const image = screen.getByRole("img", { hidden: true }); + Object.defineProperties(image, { + naturalWidth: { value: 1632 }, + naturalHeight: { value: 2112 }, + }); + fireEvent.load(image); + act(() => resize?.(864, 600)); + // 1632 natural px into an 816 px fit: 50%. + expect(zoomLabel()).toBe("50%"); + expect(screen.queryByLabelText("Page")).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: "Zoom in" })); + expect(zoomLabel()).toBe("75%"); +}); + +it("is empty without a source", () => { + render(); + expect(screen.getByText("No document")).toBeTruthy(); +}); diff --git a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx index 980f626f85..0ba7e0b22c 100644 --- a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx @@ -3,7 +3,11 @@ import { usePdfDocument } from "@v1/hooks/usePdfDocument"; import { cn } from "@v1/lib/cn"; import type { Highlight } from "@v1/lib/pdf/highlights"; import { viewerKeyAction } from "@v1/lib/pdf/keyboard"; -import type { PageSize, PdfSource } from "@v1/lib/pdf/types"; +import { + type DocumentSource, + isPdfSource, + type PageSize, +} from "@v1/lib/pdf/types"; import { fitZoom, MAX_ZOOM, @@ -14,6 +18,7 @@ import { } from "@v1/lib/pdf/zoom"; import { type KeyboardEvent, + type ReactNode, type Ref, useCallback, useEffect, @@ -22,7 +27,9 @@ import { useRef, useState, } from "react"; +import { ImageView } from "./ImageView"; import { PdfPageList, type PdfPageListHandle } from "./PdfPageList"; +import { type TextLineHighlight, TextView } from "./TextView"; import { ViewerStatus } from "./ViewerStatus"; import { ViewerToolbar } from "./ViewerToolbar"; @@ -34,8 +41,15 @@ export interface DocumentViewerHandle { } export interface DocumentViewerProps { - source: PdfSource | null; + /** A PDF (URL or bytes), an image, or text. */ + source: DocumentSource | null; + /** Page highlights, for a PDF or an image. */ highlights?: readonly Highlight[]; + /** Line highlights, for text. */ + textHighlights?: readonly TextLineHighlight[]; + onTextHighlightClick?: (highlight: TextLineHighlight) => void; + /** Shown at the start of the toolbar, whatever state the document is in. */ + switcher?: ReactNode; /** Shown in the active tone and scrolled into view when it changes. */ activeHighlightId?: string | null; onHighlightClick?: (highlight: Highlight) => void; @@ -94,20 +108,31 @@ function initialZoomState( } /** - * A PDF with a toolbar: loads it, shows its states, and renders its pages with - * highlights. Keys while the pages have focus: PageUp/PageDown, Home/End, - * + and −. + * A document with a toolbar. A PDF is loaded and rendered page by page, an + * image as a single page, and text line by line; PDFs and images share zoom + * and highlights. Keys while the content has focus: PageUp/PageDown, + * Home/End, + and −. */ export function DocumentViewer({ source, highlights, + textHighlights, + onTextHighlightClick, + switcher, activeHighlightId, onHighlightClick, initialZoom, className, ref, }: DocumentViewerProps) { - const { state, retry, submitPassword } = usePdfDocument(source); + const { state, retry, submitPassword } = usePdfDocument( + isPdfSource(source) ? source : null, + ); + // An image's size is known once it loads; kept with its URL so a new image + // never borrows the last one's. + const [image, setImage] = useState<{ url: string; size: PageSize } | null>( + null, + ); const [zoomState, setZoomState] = useState(() => initialZoomState(initialZoom), ); @@ -116,9 +141,19 @@ export function DocumentViewer({ const listRef = useRef(null); const pendingRef = useRef(null); - const ready = state.status === "ready"; - const pageSizes = ready ? state.pageSizes : undefined; + const kind = + source === null ? null : isPdfSource(source) ? "pdf" : source.kind; + const imageSize = + source?.kind === "image" && image?.url === source.url ? image.size : null; + const pdfReady = kind === "pdf" && state.status === "ready"; + const pageSizes = pdfReady + ? state.pageSizes + : imageSize + ? [imageSize] + : undefined; const pageCount = pageSizes?.length ?? 0; + // When the content can take scroll requests. + const ready = pdfReady || kind === "text" || imageSize !== null; const zoom = useMemo(() => { if (zoomState.mode === "custom" || !pageSizes || !bodySize) { @@ -206,14 +241,82 @@ export function DocumentViewer({ goToPage(pageCount - 1); break; case "zoom-in": - zoomBy(zoomIn); + if (kind !== "text") { + zoomBy(zoomIn); + } break; case "zoom-out": - zoomBy(zoomOut); + if (kind !== "text") { + zoomBy(zoomOut); + } break; } }; + let content: ReactNode; + if (source === null || kind === null) { + content = ( + + ); + } else if (source.kind === "text") { + content = ( + + ); + } else if (source.kind === "image") { + const { url } = source; + content = ( + setImage({ url, size })} + onKeyDown={onKeyDown} + /> + ); + } else if (state.status === "ready") { + content = ( + + ); + } else { + content = ( + + ); + } + + const showPaging = pdfReady; + const showZoom = pdfReady || imageSize !== null; + return (
- {ready && ( + {(showPaging || showZoom || switcher) && ( )}
- {ready ? ( - - ) : ( - - )} + {content}
); diff --git a/frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx b/frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx new file mode 100644 index 0000000000..716e45e6f2 --- /dev/null +++ b/frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx @@ -0,0 +1,64 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { expect, it, vi } from "vitest"; +import { ImageView } from "./ImageView"; + +function loadImage(width: number, height: number) { + const image = screen.getByRole("img", { hidden: true }); + Object.defineProperties(image, { + naturalWidth: { value: width }, + naturalHeight: { value: height }, + }); + fireEvent.load(image); + return image; +} + +it("shows loading until the image arrives, then sizes it by the zoom", () => { + const onLoadSize = vi.fn(); + const { container, rerender } = render( + , + ); + expect(screen.getByRole("status")).toBeTruthy(); + loadImage(1200, 1600); + expect(screen.queryByRole("status")).toBeNull(); + // Reported in points, so the viewer fits it like a page. + expect(onLoadSize).toHaveBeenCalledWith({ width: 900, height: 1200 }); + + const frame = container.querySelector("img")?.parentElement; + expect(frame?.style.getPropertyValue("--page-width")).toBe("1200px"); + rerender(); + expect(frame?.style.getPropertyValue("--page-width")).toBe("600px"); + expect(frame?.style.getPropertyValue("--page-height")).toBe("800px"); +}); + +it("draws highlights over the image", () => { + const { container } = render( + , + ); + loadImage(1200, 1600); + expect(container.querySelector('[data-highlight-id="h"]')).not.toBeNull(); +}); + +it("offers Retry when the image fails, and loads it again", async () => { + render(); + fireEvent.error(screen.getByRole("img", { hidden: true })); + expect(screen.getByRole("alert").textContent).toContain("didn't load"); + await userEvent.click(screen.getByRole("button", { name: "Retry" })); + expect(screen.getByRole("status")).toBeTruthy(); + expect(screen.getByRole("img", { hidden: true })).toBeTruthy(); +}); diff --git a/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx new file mode 100644 index 0000000000..708d059c56 --- /dev/null +++ b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx @@ -0,0 +1,166 @@ +import type { Highlight } from "@v1/lib/pdf/highlights"; +import type { PageSize } from "@v1/lib/pdf/types"; +import { CSS_PIXELS_PER_POINT, pageCssSize } from "@v1/lib/pdf/windowing"; +import { + type KeyboardEvent, + type Ref, + type SyntheticEvent, + useImperativeHandle, + useLayoutEffect, + useRef, + useState, +} from "react"; +import { HighlightLayer } from "./HighlightLayer"; +import type { PdfPageListHandle } from "./PdfPageList"; +import { ViewerStatus } from "./ViewerStatus"; + +const NO_HIGHLIGHTS: readonly Highlight[] = []; + +export interface ImageViewProps { + url: string; + alt?: string; + /** 1 shows the image at its natural size. */ + zoom: number; + /** Highlights on page 0, in fractions of the image. */ + highlights?: readonly Highlight[]; + activeHighlightId?: string | null; + onHighlightClick?: (highlight: Highlight) => void; + /** + * The image's size once loaded, in PDF points, so the viewer can fit it the + * same way as a page. + */ + onLoadSize?: (size: PageSize) => void; + onKeyDown?: (event: KeyboardEvent) => void; + ref?: Ref; +} + +type LoadState = + | { status: "loading" } + | { status: "failed" } + | { status: "ready"; size: PageSize }; + +/** An image as a one-page document: the same zoom, highlights and scrolling. */ +export function ImageView({ + url, + alt = "Document image", + zoom, + highlights = NO_HIGHLIGHTS, + activeHighlightId, + onHighlightClick, + onLoadSize, + onKeyDown, + ref, +}: ImageViewProps) { + const [load, setLoad] = useState({ status: "loading" }); + const [attempt, setAttempt] = useState(0); + const rootRef = useRef(null); + const frameRef = useRef(null); + const css = load.status === "ready" ? pageCssSize(load.size, zoom) : null; + const width = css?.width; + const height = css?.height; + + useLayoutEffect(() => { + if (width !== undefined && height !== undefined) { + frameRef.current?.style.setProperty("--page-width", `${width}px`); + frameRef.current?.style.setProperty("--page-height", `${height}px`); + } + }, [width, height]); + + useImperativeHandle( + ref, + () => ({ + scrollToPage() { + if (rootRef.current) { + rootRef.current.scrollTop = 0; + } + }, + scrollToHighlight(id) { + const highlight = highlights.find((candidate) => candidate.id === id); + const root = rootRef.current; + const frame = frameRef.current; + if (!highlight || !root || !frame) { + return false; + } + root.scrollTop = + frame.offsetTop + + (highlight.y + highlight.h / 2) * frame.offsetHeight - + root.clientHeight / 2; + root.scrollLeft = + frame.offsetLeft + + (highlight.x + highlight.w / 2) * frame.offsetWidth - + root.clientWidth / 2; + return true; + }, + }), + [highlights], + ); + + const onLoad = (event: SyntheticEvent) => { + const { naturalWidth, naturalHeight } = event.currentTarget; + // Natural pixels as points, so zoom 1 shows the image pixel for pixel. + const size = { + width: naturalWidth / CSS_PIXELS_PER_POINT, + height: naturalHeight / CSS_PIXELS_PER_POINT, + }; + setLoad({ status: "ready", size }); + onLoadSize?.(size); + }; + + if (load.status === "failed") { + return ( + { + setLoad({ status: "loading" }); + setAttempt((count) => count + 1); + }} + onSubmitPassword={() => undefined} + /> + ); + } + + return ( + // biome-ignore lint/a11y/noNoninteractiveElementInteractions: the keys belong to the focusable scroll region +
+ {load.status === "loading" && ( + undefined} + onSubmitPassword={() => undefined} + /> + )} +
+
+ {/* biome-ignore lint/a11y/noNoninteractiveElementInteractions: load and error are not user interactions */} + {alt} setLoad({ status: "failed" })} + className="ui:block ui:size-full" + /> + +
+
+
+ ); +} diff --git a/frontend/src/v1/components/patterns/document-viewer/TextView.test.tsx b/frontend/src/v1/components/patterns/document-viewer/TextView.test.tsx new file mode 100644 index 0000000000..f2426dd796 --- /dev/null +++ b/frontend/src/v1/components/patterns/document-viewer/TextView.test.tsx @@ -0,0 +1,86 @@ +import { act, render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { createRef } from "react"; +import { expect, it, vi } from "vitest"; +import type { PdfPageListHandle } from "./PdfPageList"; +import { TextView } from "./TextView"; + +// Shaped like an LLMWhisperer layout result: columns kept with spaces. +const text = [ + "INVOICE No. 4471", + "", + "Bill to: Acme Corp Date: 2026-10-01", + "Total due: $1,240.00", +].join("\n"); + +function line(container: HTMLElement, index: number): HTMLElement { + const element = container.querySelector( + `[data-line="${index}"]`, + ); + if (!element) { + throw new Error(`no line ${index}`); + } + return element; +} + +it("shows every line, blank ones included, with its number", () => { + const { container } = render(); + expect(container.querySelectorAll("[data-line]")).toHaveLength(4); + expect(line(container, 3).textContent).toBe( + "4Total due: $1,240.00", + ); +}); + +it("highlights whole lines in their tone, and the active one as active", () => { + const { container } = render( + , + ); + expect(line(container, 3).getAttribute("data-tone")).toBe("active"); + expect(line(container, 2).getAttribute("data-tone")).toBe("low-confidence"); + expect(line(container, 0).getAttribute("data-tone")).toBeNull(); +}); + +it("reports clicks on highlighted lines only", async () => { + const onHighlightClick = vi.fn(); + render( + , + ); + const buttons = screen.getAllByRole("button"); + expect(buttons).toHaveLength(1); + await userEvent.click(buttons[0] as HTMLElement); + expect(onHighlightClick).toHaveBeenCalledWith({ id: "total", line: 3 }); +}); + +it("centres a highlighted line on request", () => { + const ref = createRef(); + const { container } = render( + , + ); + const root = screen.getByRole("region", { name: "Document text" }); + Object.defineProperty(root, "clientHeight", { value: 100 }); + Object.defineProperties(line(container, 3), { + offsetTop: { value: 400 }, + offsetHeight: { value: 20 }, + }); + act(() => { + expect(ref.current?.scrollToHighlight("total")).toBe(true); + }); + expect(root.scrollTop).toBe(400 + 10 - 50); + expect(ref.current?.scrollToHighlight("nope")).toBe(false); +}); + +it("shows the empty state for blank text", () => { + render(); + expect(screen.getByText("No document")).toBeTruthy(); +}); diff --git a/frontend/src/v1/components/patterns/document-viewer/TextView.tsx b/frontend/src/v1/components/patterns/document-viewer/TextView.tsx new file mode 100644 index 0000000000..4fe4978d3d --- /dev/null +++ b/frontend/src/v1/components/patterns/document-viewer/TextView.tsx @@ -0,0 +1,152 @@ +import { cn } from "@v1/lib/cn"; +import type { HighlightTone } from "@v1/lib/pdf/highlights"; +import { + type KeyboardEvent, + type Ref, + useImperativeHandle, + useMemo, + useRef, +} from "react"; +import type { PdfPageListHandle } from "./PdfPageList"; +import { ViewerStatus } from "./ViewerStatus"; + +/** A highlighted line of text. Lines are 0-based, as LLMWhisperer numbers them. */ +export interface TextLineHighlight { + id: string; + line: number; + tone?: HighlightTone; +} + +export interface TextViewProps { + text: string; + highlights?: readonly TextLineHighlight[]; + activeHighlightId?: string | null; + onHighlightClick?: (highlight: TextLineHighlight) => void; + onKeyDown?: (event: KeyboardEvent) => void; + ref?: Ref; +} + +const NO_HIGHLIGHTS: readonly TextLineHighlight[] = []; + +const TONES: Record = { + default: "ui:bg-primary/15", + active: "ui:bg-primary/30 ui:ring-1 ui:ring-primary", + "low-confidence": "ui:bg-destructive/15", +}; + +/** + * Text shown line by line with line numbers, as extracted (Raw Text, HITL + * text, an LLMWhisperer result), with whole lines highlighted. + */ +export function TextView({ + text, + highlights = NO_HIGHLIGHTS, + activeHighlightId, + onHighlightClick, + onKeyDown, + ref, +}: TextViewProps) { + const rootRef = useRef(null); + const lines = useMemo(() => text.split(/\r\n|\r|\n/), [text]); + // The last highlight on a line decides how it looks. + const byLine = useMemo(() => { + const map = new Map(); + for (const highlight of highlights) { + map.set(highlight.line, highlight); + } + return map; + }, [highlights]); + + useImperativeHandle( + ref, + () => ({ + scrollToPage() { + if (rootRef.current) { + rootRef.current.scrollTop = 0; + } + }, + scrollToHighlight(id) { + const highlight = highlights.find((candidate) => candidate.id === id); + const root = rootRef.current; + const line = + highlight && + root?.querySelector(`[data-line="${highlight.line}"]`); + if (!root || !line) { + return false; + } + root.scrollTop = + line.offsetTop + line.offsetHeight / 2 - root.clientHeight / 2; + return true; + }, + }), + [highlights], + ); + + if (text.trim() === "") { + return ( + undefined} + onSubmitPassword={() => undefined} + /> + ); + } + + return ( + // biome-ignore lint/a11y/noNoninteractiveElementInteractions: the keys belong to the focusable scroll region +
+
    + {lines.map((content, index) => { + const highlight = byLine.get(index); + const tone: HighlightTone | null = highlight + ? highlight.id === activeHighlightId + ? "active" + : (highlight.tone ?? "default") + : null; + const body = + highlight && onHighlightClick ? ( + + ) : ( + {content || " "} + ); + return ( +
  1. + + {body} +
    2. + ); + })} +
+
+ ); +} diff --git a/frontend/src/v1/components/patterns/document-viewer/ViewerToolbar.tsx b/frontend/src/v1/components/patterns/document-viewer/ViewerToolbar.tsx index da20056582..27e468de5f 100644 --- a/frontend/src/v1/components/patterns/document-viewer/ViewerToolbar.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/ViewerToolbar.tsx @@ -9,7 +9,7 @@ import { ZoomIn, ZoomOut, } from "lucide-react"; -import { type FormEvent, useId, useState } from "react"; +import { type FormEvent, type ReactNode, useId, useState } from "react"; export interface ViewerToolbarProps { /** 0-based. */ @@ -24,6 +24,12 @@ export interface ViewerToolbarProps { onZoomOut: () => void; onFitWidth: () => void; onFitPage: () => void; + /** Leading content, such as a document switcher (Figma C10). */ + leading?: ReactNode; + /** Paging controls; off for a single image. */ + showPaging?: boolean; + /** Zoom and fit controls; off for text. */ + showZoom?: boolean; className?: string; } @@ -110,6 +116,9 @@ export function ViewerToolbar({ onZoomOut, onFitWidth, onFitPage, + leading, + showPaging = true, + showZoom = true, className, }: ViewerToolbarProps) { return ( @@ -121,60 +130,67 @@ export function ViewerToolbar({ className, )} > -
- onPageChange(pageIndex - 1)} - /> - {/* Remounts when the page changes, so the box shows the new page. */} - - = pageCount - 1} - onClick={() => onPageChange(pageIndex + 1)} - /> -
-
- - - {formatZoom(zoom)} - - - - +
+ {leading} + {showPaging && ( +
+ onPageChange(pageIndex - 1)} + /> + {/* Remounts when the page changes, so the box shows the new page. */} + + = pageCount - 1} + onClick={() => onPageChange(pageIndex + 1)} + /> +
+ )}
+ {showZoom && ( +
+ + + {formatZoom(zoom)} + + + + +
+ )}
); } diff --git a/frontend/src/v1/lib/pdf/types.ts b/frontend/src/v1/lib/pdf/types.ts index 84cdd3af9c..fd6636a7e8 100644 --- a/frontend/src/v1/lib/pdf/types.ts +++ b/frontend/src/v1/lib/pdf/types.ts @@ -27,3 +27,25 @@ export type PdfLoadFailure = | { kind: "password"; incorrect: boolean } | { kind: "unsupported" } | { kind: "failed"; message: string }; + +/** An image shown as a single page. */ +export interface ImageSource { + kind: "image"; + url: string; + alt?: string; +} + +/** Extracted or raw text, shown line by line. */ +export interface TextSource { + kind: "text"; + text: string; +} + +/** Anything the document viewer shows. */ +export type DocumentSource = PdfSource | ImageSource | TextSource; + +export function isPdfSource( + source: DocumentSource | null, +): source is PdfSource { + return source?.kind === "url" || source?.kind === "data"; +} From 2c294bdcc10548de00d1fcefaade562abef1f511 Mon Sep 17 00:00:00 2001 From: ali <117142933+muhammad-ali-e@users.noreply.github.com> Date: Thu, 29 Oct 2026 08:46:11 +0000 Subject: [PATCH 2/7] UN-4506 [FEAT] Size the v1 image view through style custom properties Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R7trgcAzu6jxDnit3eeozm --- .../patterns/document-viewer/ImageView.tsx | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx index 708d059c56..59e96f6586 100644 --- a/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx @@ -6,7 +6,6 @@ import { type Ref, type SyntheticEvent, useImperativeHandle, - useLayoutEffect, useRef, useState, } from "react"; @@ -56,15 +55,6 @@ export function ImageView({ const rootRef = useRef(null); const frameRef = useRef(null); const css = load.status === "ready" ? pageCssSize(load.size, zoom) : null; - const width = css?.width; - const height = css?.height; - - useLayoutEffect(() => { - if (width !== undefined && height !== undefined) { - frameRef.current?.style.setProperty("--page-width", `${width}px`); - frameRef.current?.style.setProperty("--page-height", `${height}px`); - } - }, [width, height]); useImperativeHandle( ref, @@ -144,6 +134,11 @@ export function ImageView({ ? "ui:relative ui:h-(--page-height) ui:w-(--page-width) ui:shrink-0 ui:bg-card ui:shadow-sm" : "ui:invisible ui:absolute" } + // The image's size is runtime geometry: custom properties the classes read. + style={{ + "--page-width": css ? `${css.width}px` : undefined, + "--page-height": css ? `${css.height}px` : undefined, + }} > {/* biome-ignore lint/a11y/noNoninteractiveElementInteractions: load and error are not user interactions */} Date: Mon, 2 Nov 2026 06:31:12 +0000 Subject: [PATCH 3/7] UN-4506 [FEAT] Fix keys, scroll timing and highlights for text and images - Paging keys on text and images keep the browser's own scrolling; they used to be swallowed and jump to the top. Text ignores zoom keys. - Scroll requests wait until an image has loaded, not just until its view has mounted, so an early highlight scroll lands once the image is sized. - Returning to an image shown earlier waits for its reload instead of reusing the old size. - The image view draws only page 0's highlights. - Text rows are memoised, so moving the active highlight re-renders only the lines involved. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R7trgcAzu6jxDnit3eeozm --- .../document-viewer/DocumentViewer.test.tsx | 107 ++++++++++++++++++ .../document-viewer/DocumentViewer.tsx | 31 +++-- .../patterns/document-viewer/ImageView.tsx | 14 ++- .../patterns/document-viewer/TextView.tsx | 101 +++++++++++------ 4 files changed, 207 insertions(+), 46 deletions(-) diff --git a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx index 47922e227f..a6846f7aa1 100644 --- a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx @@ -255,3 +255,110 @@ it("fits every page in fit page, so a wider page later on sets the zoom", async // The landscape page is 1056 px wide at 100%: 816 / 1056 ≈ 77%. expect(zoomLabel()).toBe("77%"); }); + +function loadImage(width: number, height: number) { + const image = screen.getByRole("img", { hidden: true }); + Object.defineProperties(image, { + naturalWidth: { value: width }, + naturalHeight: { value: height }, + }); + fireEvent.load(image); +} + +it("leaves paging keys to the browser for text and images", () => { + const { rerender } = render( + , + ); + const text = screen.getByRole("region", { name: "Document text" }); + // fireEvent returns false when the handler called preventDefault. + expect(fireEvent.keyDown(text, { key: "PageDown" })).toBe(true); + expect(fireEvent.keyDown(text, { key: "End" })).toBe(true); + expect(fireEvent.keyDown(text, { key: "+" })).toBe(true); + + rerender(); + loadImage(1632, 2112); + const image = screen.getByRole("region", { name: "Document image" }); + expect(fireEvent.keyDown(image, { key: "PageDown" })).toBe(true); + expect(fireEvent.keyDown(image, { key: "+" })).toBe(false); +}); + +it("runs a highlight scroll asked for before an image loads once it has", () => { + const ref = createRef(); + render( + , + ); + const region = screen.getByRole("region", { name: "Document image" }); + act(() => ref.current?.scrollToHighlight("h")); + expect(region.scrollTop).toBe(0); + const frame = region.querySelector("img")?.parentElement; + if (!frame) { + throw new Error("no frame"); + } + Object.defineProperty(frame, "offsetHeight", { value: 1000 }); + Object.defineProperty(region, "clientHeight", { value: 400 }); + loadImage(1632, 2112); + // Centre of the box at 0.925 of 1000 px, minus half the 400 px viewport. + expect(region.scrollTop).toBeCloseTo(725); +}); + +it("draws only page 0's highlights over an image", () => { + const { container } = render( + , + ); + loadImage(1632, 2112); + expect(container.querySelector('[data-highlight-id="a"]')).not.toBeNull(); + expect(container.querySelector('[data-highlight-id="b"]')).toBeNull(); +}); + +it("waits for an image to load again after another source was shown", async () => { + const image = { kind: "image", url: "/scan.png" } as const; + const { rerender } = render(); + loadImage(1632, 2112); + act(() => resize?.(864, 600)); + expect(screen.queryByRole("button", { name: "Zoom in" })).not.toBeNull(); + + rerender(); + rerender(); + expect(screen.queryByRole("button", { name: "Zoom in" })).toBeNull(); + loadImage(1632, 2112); + expect(screen.queryByRole("button", { name: "Zoom in" })).not.toBeNull(); +}); diff --git a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx index 10da2395ca..336bf06636 100644 --- a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx @@ -143,6 +143,14 @@ export function DocumentViewer({ const kind = source === null ? null : isPdfSource(source) ? "pdf" : source.kind; + // Forget an image's size once another source is shown, so coming back to + // it waits for its reload instead of treating it as ready straight away. + const imageUrl = source?.kind === "image" ? source.url : null; + const [shownImageUrl, setShownImageUrl] = useState(imageUrl); + if (shownImageUrl !== imageUrl) { + setShownImageUrl(imageUrl); + setImage(null); + } const imageSize = source?.kind === "image" && image?.url === source.url ? image.size : null; const pdfReady = kind === "pdf" && state.status === "ready"; @@ -154,6 +162,8 @@ export function DocumentViewer({ const pageCount = pageSizes?.length ?? 0; // When the content can take scroll requests. const ready = pdfReady || kind === "text" || imageSize !== null; + const readyRef = useRef(ready); + readyRef.current = ready; const zoom = useMemo(() => { if (zoomState.mode === "custom" || !pageSizes || !bodySize) { @@ -180,7 +190,9 @@ export function DocumentViewer({ const run = useCallback((request: ScrollRequest) => { const list = listRef.current; - if (!list) { + // An image's view exists before the image has loaded, so wait for the + // content to be ready, not just mounted. + if (!list || !readyRef.current) { pendingRef.current = request; return; } @@ -227,7 +239,14 @@ export function DocumentViewer({ const onKeyDown = (event: KeyboardEvent) => { const action = viewerKeyAction(event); - if (action === null) { + // Text and images are one scrolling page: their paging keys keep the + // browser's own scrolling, and text has no zoom. + const paging = action !== null && !action.startsWith("zoom"); + if ( + action === null || + (paging && kind !== "pdf") || + (!paging && kind === "text") + ) { return; } event.preventDefault(); @@ -245,14 +264,10 @@ export function DocumentViewer({ goToPage(pageCount - 1); break; case "zoom-in": - if (kind !== "text") { - zoomBy(zoomIn); - } + zoomBy(zoomIn); break; case "zoom-out": - if (kind !== "text") { - zoomBy(zoomOut); - } + zoomBy(zoomOut); break; } }; diff --git a/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx index 59e96f6586..a9789eca8f 100644 --- a/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx @@ -6,6 +6,7 @@ import { type Ref, type SyntheticEvent, useImperativeHandle, + useMemo, useRef, useState, } from "react"; @@ -54,6 +55,11 @@ export function ImageView({ const [attempt, setAttempt] = useState(0); const rootRef = useRef(null); const frameRef = useRef(null); + // An image is one page: highlights for any other page are not drawn. + const pageHighlights = useMemo( + () => highlights.filter((highlight) => highlight.page === 0), + [highlights], + ); const css = load.status === "ready" ? pageCssSize(load.size, zoom) : null; useImperativeHandle( @@ -65,7 +71,9 @@ export function ImageView({ } }, scrollToHighlight(id) { - const highlight = highlights.find((candidate) => candidate.id === id); + const highlight = pageHighlights.find( + (candidate) => candidate.id === id, + ); const root = rootRef.current; const frame = frameRef.current; if (!highlight || !root || !frame) { @@ -82,7 +90,7 @@ export function ImageView({ return true; }, }), - [highlights], + [pageHighlights], ); const onLoad = (event: SyntheticEvent) => { @@ -150,7 +158,7 @@ export function ImageView({ className="ui:block ui:size-full" /> diff --git a/frontend/src/v1/components/patterns/document-viewer/TextView.tsx b/frontend/src/v1/components/patterns/document-viewer/TextView.tsx index 4fe4978d3d..1ca9a1e626 100644 --- a/frontend/src/v1/components/patterns/document-viewer/TextView.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/TextView.tsx @@ -2,6 +2,7 @@ import { cn } from "@v1/lib/cn"; import type { HighlightTone } from "@v1/lib/pdf/highlights"; import { type KeyboardEvent, + memo, type Ref, useImperativeHandle, useMemo, @@ -34,6 +35,62 @@ const TONES: Record = { "low-confidence": "ui:bg-destructive/15", }; +/** + * One line. Memoised so that moving the active highlight re-renders only the + * lines it leaves and reaches, not every line of a long result. + */ +const TextLine = memo(function TextLine({ + index, + content, + highlight, + active, + onHighlightClick, +}: { + index: number; + content: string; + highlight: TextLineHighlight | undefined; + active: boolean; + onHighlightClick?: (highlight: TextLineHighlight) => void; +}) { + const tone: HighlightTone | null = highlight + ? active + ? "active" + : (highlight.tone ?? "default") + : null; + const body = + highlight && onHighlightClick ? ( + + ) : ( + {content || " "} + ); + return ( +
  • + + {body} +
    • + ); +}); + /** * Text shown line by line with line numbers, as extracted (Raw Text, HITL * text, an LLMWhisperer result), with whole lines highlighted. @@ -105,45 +162,19 @@ export function TextView({
      {lines.map((content, index) => { const highlight = byLine.get(index); - const tone: HighlightTone | null = highlight - ? highlight.id === activeHighlightId - ? "active" - : (highlight.tone ?? "default") - : null; - const body = - highlight && onHighlightClick ? ( - - ) : ( - {content || " "} - ); return ( -
    1. - - {body} -
      2. + index={index} + content={content} + highlight={highlight} + active={ + highlight !== undefined && highlight.id === activeHighlightId + } + onHighlightClick={onHighlightClick} + /> ); })}
    From 02aa9cfe787cee6cbbd2834e7dae0d375d48c09b Mon Sep 17 00:00:00 2001 From: ali <117142933+muhammad-ali-e@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:08:47 +0000 Subject: [PATCH 4/7] UN-4506 Give every text row one line-number gutter Each row sized its own number column, so line 10 started further right than line 9 and the text lost its columns. The gutter is now as wide as the longest line number, shared through a custom property. Raised by Greptile on #2409. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R7trgcAzu6jxDnit3eeozm --- .../patterns/document-viewer/TextView.test.tsx | 10 ++++++++++ .../components/patterns/document-viewer/TextView.tsx | 9 +++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/frontend/src/v1/components/patterns/document-viewer/TextView.test.tsx b/frontend/src/v1/components/patterns/document-viewer/TextView.test.tsx index f2426dd796..da9a167f76 100644 --- a/frontend/src/v1/components/patterns/document-viewer/TextView.test.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/TextView.test.tsx @@ -84,3 +84,13 @@ it("shows the empty state for blank text", () => { render(); expect(screen.getByText("No document")).toBeTruthy(); }); + +it("gives every row one gutter, as wide as the longest line number", () => { + const text = Array.from({ length: 120 }, (_, n) => `line ${n}`).join("\n"); + const { container } = render(); + const list = container.querySelector("ol"); + expect(list?.style.getPropertyValue("--gutter")).toBe("3ch"); + const rows = container.querySelectorAll("li"); + expect(rows[8]?.className).toBe(rows[119]?.className); + expect(rows[8]?.className).toContain("grid-cols-[var(--gutter)_1fr]"); +}); diff --git a/frontend/src/v1/components/patterns/document-viewer/TextView.tsx b/frontend/src/v1/components/patterns/document-viewer/TextView.tsx index 1ca9a1e626..bb3cb3036e 100644 --- a/frontend/src/v1/components/patterns/document-viewer/TextView.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/TextView.tsx @@ -76,7 +76,7 @@ const TextLine = memo(function TextLine({ data-tone={tone ?? undefined} data-highlight-id={highlight?.id} className={cn( - "ui:grid ui:grid-cols-[auto_1fr] ui:gap-3 ui:px-3", + "ui:grid ui:grid-cols-[var(--gutter)_1fr] ui:gap-3 ui:px-3", tone && TONES[tone], )} > @@ -159,7 +159,12 @@ export function TextView({ onKeyDown={onKeyDown} className="ui:relative ui:h-full ui:overflow-auto ui:bg-card ui:py-2 ui:focus-visible:focus-ring" > -
      +
        {lines.map((content, index) => { const highlight = byLine.get(index); return ( From 68af05717417813723f61393441d9906408eaf19 Mon Sep 17 00:00:00 2001 From: ali <117142933+muhammad-ali-e@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:32:19 +0000 Subject: [PATCH 5/7] UN-4506 Keep the horizontal place when jumping to a line on an image Only a box highlight moves the image view across, as on a PDF page; a line spans the image and keeps the reader's horizontal scroll. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R7trgcAzu6jxDnit3eeozm --- .../document-viewer/ImageView.test.tsx | 49 ++++++++++++++++++- .../patterns/document-viewer/ImageView.tsx | 12 +++-- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx b/frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx index 716e45e6f2..5e5330631a 100644 --- a/frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx @@ -1,7 +1,9 @@ -import { fireEvent, render, screen } from "@testing-library/react"; +import { act, fireEvent, render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import { createRef } from "react"; import { expect, it, vi } from "vitest"; import { ImageView } from "./ImageView"; +import type { PdfPageListHandle } from "./PdfPageList"; function loadImage(width: number, height: number) { const image = screen.getByRole("img", { hidden: true }); @@ -62,3 +64,48 @@ it("offers Retry when the image fails, and loads it again", async () => { expect(screen.getByRole("status")).toBeTruthy(); expect(screen.getByRole("img", { hidden: true })).toBeTruthy(); }); + +it("centres a box across and down, and a line only down", () => { + const ref = createRef(); + const base = { page: 0, y: 0.25, h: 0.5, tone: "default" as const }; + const { container } = render( + , + ); + loadImage(1200, 1600); + const frame = container.querySelector("img")?.parentElement; + const root = container.querySelector("section"); + if (!frame || !root) { + throw new Error("no image frame"); + } + Object.defineProperties(frame, { + offsetTop: { value: 16 }, + offsetLeft: { value: 16 }, + offsetHeight: { value: 1600 }, + offsetWidth: { value: 1200 }, + }); + Object.defineProperties(root, { + clientHeight: { value: 400 }, + clientWidth: { value: 500 }, + }); + act(() => { + ref.current?.scrollToHighlight("box"); + }); + // 0.5 × 1600 = 800 down and 0.75 × 1200 = 900 across, from 16. + expect(root.scrollTop).toBe(16 + 800 - 200); + expect(root.scrollLeft).toBe(16 + 900 - 250); + + root.scrollLeft = 30; + act(() => { + ref.current?.scrollToHighlight("line"); + }); + expect(root.scrollTop).toBe(16 + 800 - 200); + expect(root.scrollLeft).toBe(30); +}); diff --git a/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx index a9789eca8f..666971535e 100644 --- a/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx @@ -83,10 +83,14 @@ export function ImageView({ frame.offsetTop + (highlight.y + highlight.h / 2) * frame.offsetHeight - root.clientHeight / 2; - root.scrollLeft = - frame.offsetLeft + - (highlight.x + highlight.w / 2) * frame.offsetWidth - - root.clientWidth / 2; + // A line spans the image, so only a box moves the view across; a + // line keeps the reader's horizontal place, as on a PDF page. + if (highlight.variant === "box") { + root.scrollLeft = + frame.offsetLeft + + (highlight.x + highlight.w / 2) * frame.offsetWidth - + root.clientWidth / 2; + } return true; }, }), From c51fbef06643ea756b96e23af13e6549092101bc Mon Sep 17 00:00:00 2001 From: ali <117142933+muhammad-ali-e@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:19:32 +0000 Subject: [PATCH 6/7] UN-4506 [FIX] Address review: text identity, image zoom and loading, one source key, empty text - A text source can carry an id; the view keeps its place while the id stays the same, so a live-edited text no longer remounts per keystroke. - One sourceKey drives every per-source reset (image size, page index) and keys the text and image views, replacing three separate mechanisms. - Zoom keys are ignored until an image has a size, so it still opens in its fit mode. - A scroll to a highlight on another page of an image is dropped instead of waiting for ever. - ImageView resets when its URL changes, reports a highlight scroll as not yet possible until it has loaded, and keeps the centre spot when zooming. - The image region is named Image viewer, so it no longer repeats the image's alt text. - Blank text says no text was found rather than no document. - Text lines use content-visibility, so the browser skips layout and paint for lines off screen while every line stays in the DOM. - One NO_HIGHLIGHTS constant and one page frame class are shared. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R7trgcAzu6jxDnit3eeozm --- .../document-viewer/DocumentViewer.test.tsx | 84 ++++++++++++++++++- .../document-viewer/DocumentViewer.tsx | 71 ++++++++-------- .../document-viewer/ImageView.test.tsx | 77 +++++++++++++++++ .../patterns/document-viewer/ImageView.tsx | 80 +++++++++++++++--- .../patterns/document-viewer/PdfPage.tsx | 5 +- .../patterns/document-viewer/PdfPageList.tsx | 8 +- .../document-viewer/TextView.test.tsx | 13 ++- .../patterns/document-viewer/TextView.tsx | 11 +-- .../patterns/document-viewer/ViewerStatus.tsx | 16 +++- .../patterns/document-viewer/pageFrame.ts | 7 ++ frontend/src/v1/lib/pdf/highlights.ts | 3 + frontend/src/v1/lib/pdf/types.ts | 6 ++ 12 files changed, 317 insertions(+), 64 deletions(-) create mode 100644 frontend/src/v1/components/patterns/document-viewer/pageFrame.ts diff --git a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx index d41c7ebaca..bdca017d31 100644 --- a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx @@ -317,7 +317,7 @@ it("leaves paging keys to the browser for text and images", () => { rerender(); loadImage(1632, 2112); - const image = screen.getByRole("region", { name: "Document image" }); + const image = screen.getByRole("region", { name: "Image viewer" }); expect(fireEvent.keyDown(image, { key: "PageDown" })).toBe(true); expect(fireEvent.keyDown(image, { key: "+" })).toBe(false); }); @@ -342,7 +342,7 @@ it("runs a highlight scroll asked for before an image loads once it has", () => ]} />, ); - const region = screen.getByRole("region", { name: "Document image" }); + const region = screen.getByRole("region", { name: "Image viewer" }); act(() => ref.current?.scrollToHighlight("h")); expect(region.scrollTop).toBe(0); const frame = region.querySelector("img")?.parentElement; @@ -669,3 +669,83 @@ it("starts a new document on its first page", async () => { // Not even one render showed page 3 of 1. expect(toolbarRenders.filter(([index, count]) => index >= count)).toEqual([]); }); + +it("keeps a live-edited text in place while its id stays the same", () => { + const { rerender } = render( + , + ); + const before = screen.getByRole("region", { name: "Document text" }); + before.scrollTop = 300; + rerender( + , + ); + const after = screen.getByRole("region", { name: "Document text" }); + expect(after).toBe(before); + expect(after.scrollTop).toBe(300); + expect(after.textContent).toContain("texts"); + + rerender( + , + ); + expect(screen.getByRole("region", { name: "Document text" })).not.toBe( + before, + ); +}); + +it("ignores zoom keys until an image has loaded, so it still opens fitted", () => { + render(); + const region = screen.getByRole("region", { name: "Image viewer" }); + // Not handled: fireEvent returns true when nothing called preventDefault. + expect(fireEvent.keyDown(region, { key: "+" })).toBe(true); + act(() => resize(864, 600)); + loadImage(1632, 2112); + expect( + screen + .getByRole("button", { name: "Fit width" }) + .getAttribute("aria-pressed"), + ).toBe("true"); +}); + +it("drops a scroll to a highlight on another page of an image instead of keeping it", () => { + const ref = createRef(); + const box = { + id: "h", + x: 0.5, + y: 0.9, + w: 0.1, + h: 0.05, + variant: "box" as const, + tone: "default" as const, + }; + const { rerender } = render( + , + ); + const region = screen.getByRole("region", { name: "Image viewer" }); + const frame = region.querySelector("img")?.parentElement; + if (!frame) { + throw new Error("no frame"); + } + Object.defineProperty(frame, "offsetHeight", { value: 1000 }); + Object.defineProperty(region, "clientHeight", { value: 400 }); + loadImage(1632, 2112); + act(() => ref.current?.scrollToHighlight("h")); + // Had the request waited, it would fire here, long after it was asked for. + rerender( + , + ); + expect(region.scrollTop).toBe(0); +}); diff --git a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx index 2f25967386..7cda8b21ae 100644 --- a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx @@ -146,38 +146,33 @@ export function DocumentViewer({ initialZoomState(initialZoom), ); const [pageIndex, setPageIndex] = useState(0); - // A new document starts on its first page: reset while rendering, so the - // toolbar never shows the last document's page against this one's count. - const shownDocument = state.status === "ready" ? state.document : null; - const [pageIndexFor, setPageIndexFor] = useState(shownDocument); - if (pageIndexFor !== shownDocument) { - setPageIndexFor(shownDocument); - setPageIndex(0); - } const [bodyRef, bodySize] = useElementSize(); const listRef = useRef(null); const pendingRef = useRef(null); const kind = source === null ? null : isPdfSource(source) ? "pdf" : source.kind; - // Forget an image's size once another source is shown, so coming back to - // it waits for its reload instead of treating it as ready straight away. - const imageUrl = source?.kind === "image" ? source.url : null; - const [shownImageUrl, setShownImageUrl] = useState(imageUrl); - if (shownImageUrl !== imageUrl) { - setShownImageUrl(imageUrl); + // The one identity of what is shown: the PDF's source key, the image's URL, + // or the text's id (the text itself when it has none). Everything kept per + // source hangs off it: the views are keyed on it, so a new source starts at + // the top, and the state below is reset when it changes, while rendering, + // so nothing from the last source is ever shown against this one. + const sourceKey = + source === null || isPdfSource(source) + ? pdfSourceKey(source) + : source.kind === "image" + ? `image:${source.url}` + : `text:${source.id ?? source.text}`; + const [shownSourceKey, setShownSourceKey] = useState(sourceKey); + if (shownSourceKey !== sourceKey) { + setShownSourceKey(sourceKey); + // Coming back to an image waits for its reload, not its old size. setImage(null); + // A new document starts on its first page. + setPageIndex(0); } const imageSize = source?.kind === "image" && image?.url === source.url ? image.size : null; - // A new text gets a fresh view, back at the top, like a new PDF or image. - const text = source?.kind === "text" ? source.text : null; - const [shownText, setShownText] = useState(text); - const [textKey, setTextKey] = useState(0); - if (shownText !== text) { - setShownText(text); - setTextKey((key) => key + 1); - } const pdfReady = kind === "pdf" && state.status === "ready"; const pageSizes = pdfReady ? state.pageSizes @@ -191,22 +186,26 @@ export function DocumentViewer({ const pdfShown = pdfReady && measured; // When the content can take scroll requests. const ready = pdfShown || kind === "text" || imageSize !== null; - // Which content a scroll request is for. A text is its own key. - const sourceKey = - source === null || isPdfSource(source) - ? pdfSourceKey(source) - : source.kind === "image" - ? `image:${source.url}` - : source.text; + // Zoom needs a size to zoom from: a key pressed before then would leave the + // fit mode and the content would never be fitted. + const zoomable = pdfReady || imageSize !== null; // Both kept in step at commit, before any parent's layout effect can make // a request, so a request is always judged against the source it was made // for, and a render React throws away cannot leave them stale. const readyRef = useRef(ready); const sourceKeyRef = useRef(sourceKey); + // An image is one page, so a highlight on any other page can never be + // scrolled to: the request is dropped instead of waiting for ever. + const offPageRef = useRef<(id: string) => boolean>(() => false); useLayoutEffect(() => { readyRef.current = ready; sourceKeyRef.current = sourceKey; - }, [ready, sourceKey]); + offPageRef.current = (id) => + kind === "image" && + (highlights ?? []).some( + (highlight) => highlight.id === id && highlight.page !== 0, + ); + }, [ready, sourceKey, kind, highlights]); const zoom = useMemo(() => { if (zoomState.mode === "custom" || !pageSizes || !bodySize) { @@ -245,7 +244,9 @@ export function DocumentViewer({ fromActive, }; }; - if (!list || !readyRef.current) { + if (request.kind === "highlight" && offPageRef.current(request.id)) { + pendingRef.current = null; + } else if (!list || !readyRef.current) { wait(); } else if (request.kind === "page") { pendingRef.current = null; @@ -312,7 +313,7 @@ export function DocumentViewer({ if ( action === null || (paging && kind !== "pdf") || - (!paging && kind === "text") + (!paging && !zoomable) ) { return; } @@ -351,7 +352,7 @@ export function DocumentViewer({ } else if (source.kind === "text") { content = ( { expect(root.scrollTop).toBe(16 + 800 - 200); expect(root.scrollLeft).toBe(30); }); + +it("will not scroll to a highlight before the image has its size", () => { + const ref = createRef(); + render( + , + ); + expect(ref.current?.scrollToHighlight("h")).toBe(false); + loadImage(1200, 1600); + expect(ref.current?.scrollToHighlight("h")).toBe(true); +}); + +it("starts over when the URL changes without a remount", () => { + const { container, rerender } = render( + , + ); + loadImage(1200, 1600); + rerender(); + expect(screen.getByRole("status")).toBeTruthy(); + const frame = container.querySelector("img")?.parentElement; + expect(frame?.style.getPropertyValue("--page-width")).toBe(""); +}); + +it("keeps the spot at the centre of the view when zooming", () => { + const { container, rerender } = render( + , + ); + loadImage(1200, 1600); + const frame = container.querySelector("img")?.parentElement; + const root = container.querySelector("section"); + if (!frame || !root) { + throw new Error("no image frame"); + } + // The frame's size follows its style, as the browser's layout would. + const px = (name: string) => + Number.parseFloat(frame.style.getPropertyValue(name)); + Object.defineProperties(frame, { + offsetTop: { value: 16 }, + offsetLeft: { value: 16 }, + offsetWidth: { get: () => px("--page-width") }, + offsetHeight: { get: () => px("--page-height") }, + }); + Object.defineProperties(root, { + clientWidth: { value: 600 }, + clientHeight: { value: 400 }, + }); + // The image's centre at the centre of the view. + root.scrollLeft = 16 + 600 - 300; + root.scrollTop = 16 + 800 - 200; + fireEvent.scroll(root); + + rerender(); + expect(root.scrollLeft).toBeCloseTo(16 + 1200 - 300); + expect(root.scrollTop).toBeCloseTo(16 + 1600 - 200); +}); + +it("names the viewer and the image differently, so neither is read twice", () => { + render(); + loadImage(1200, 1600); + expect(screen.getByRole("region", { name: "Image viewer" })).toBeTruthy(); + expect(screen.getByRole("img", { name: "Invoice scan" })).toBeTruthy(); +}); diff --git a/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx index 666971535e..e7d8305cb6 100644 --- a/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx @@ -1,21 +1,22 @@ -import type { Highlight } from "@v1/lib/pdf/highlights"; +import { type Highlight, NO_HIGHLIGHTS } from "@v1/lib/pdf/highlights"; import type { PageSize } from "@v1/lib/pdf/types"; import { CSS_PIXELS_PER_POINT, pageCssSize } from "@v1/lib/pdf/windowing"; import { type KeyboardEvent, type Ref, type SyntheticEvent, + useCallback, useImperativeHandle, + useLayoutEffect, useMemo, useRef, useState, } from "react"; import { HighlightLayer } from "./HighlightLayer"; import type { PdfPageListHandle } from "./PdfPageList"; +import { PAGE_FRAME_CLASS } from "./pageFrame"; import { ViewerStatus } from "./ViewerStatus"; -const NO_HIGHLIGHTS: readonly Highlight[] = []; - export interface ImageViewProps { url: string; alt?: string; @@ -53,8 +54,19 @@ export function ImageView({ }: ImageViewProps) { const [load, setLoad] = useState({ status: "loading" }); const [attempt, setAttempt] = useState(0); + // A new URL is a new image: forget the last one's size and state while + // rendering, so it is never laid out at the old size. + const [shownUrl, setShownUrl] = useState(url); + if (shownUrl !== url) { + setShownUrl(url); + setLoad({ status: "loading" }); + } const rootRef = useRef(null); const frameRef = useRef(null); + // The point of the image at the viewport's centre, as fractions of the + // image, so a zoom keeps the reader on the same spot. + const anchorRef = useRef({ x: 0.5, y: 0 }); + const ready = load.status === "ready"; // An image is one page: highlights for any other page are not drawn. const pageHighlights = useMemo( () => highlights.filter((highlight) => highlight.page === 0), @@ -62,12 +74,34 @@ export function ImageView({ ); const css = load.status === "ready" ? pageCssSize(load.size, zoom) : null; + const readAnchor = useCallback(() => { + const root = rootRef.current; + const frame = frameRef.current; + if ( + !root || + !frame || + frame.offsetWidth === 0 || + frame.offsetHeight === 0 + ) { + return; + } + anchorRef.current = { + x: + (root.scrollLeft + root.clientWidth / 2 - frame.offsetLeft) / + frame.offsetWidth, + y: + (root.scrollTop + root.clientHeight / 2 - frame.offsetTop) / + frame.offsetHeight, + }; + }, []); + useImperativeHandle( ref, () => ({ scrollToPage() { if (rootRef.current) { rootRef.current.scrollTop = 0; + readAnchor(); } }, scrollToHighlight(id) { @@ -76,7 +110,9 @@ export function ImageView({ ); const root = rootRef.current; const frame = frameRef.current; - if (!highlight || !root || !frame) { + // Until the image has its size the frame is a placeholder: say not + // yet, so the viewer tries again once it has loaded. + if (!highlight || !root || !frame || !ready) { return false; } root.scrollTop = @@ -91,12 +127,33 @@ export function ImageView({ (highlight.x + highlight.w / 2) * frame.offsetWidth - root.clientWidth / 2; } + readAnchor(); return true; }, }), - [pageHighlights], + [pageHighlights, ready, readAnchor], ); + // A zoom resizes the frame; put the same spot back at the centre. The frame + // took its new size from its style at commit. + const previousZoom = useRef(zoom); + useLayoutEffect(() => { + if (previousZoom.current === zoom) { + return; + } + previousZoom.current = zoom; + const root = rootRef.current; + const frame = frameRef.current; + if (!root || !frame || !ready) { + return; + } + const { x, y } = anchorRef.current; + root.scrollLeft = + frame.offsetLeft + x * frame.offsetWidth - root.clientWidth / 2; + root.scrollTop = + frame.offsetTop + y * frame.offsetHeight - root.clientHeight / 2; + }, [zoom, ready]); + const onLoad = (event: SyntheticEvent) => { const { naturalWidth, naturalHeight } = event.currentTarget; // Natural pixels as points, so zoom 1 shows the image pixel for pixel. @@ -127,8 +184,11 @@ export function ImageView({ ref={rootRef} // biome-ignore lint/a11y/noNoninteractiveTabindex: a scrollable region must be focusable tabIndex={0} - aria-label="Document image" + // The region names the viewer; the image's alt names the image, so a + // screen reader does not hear the same words twice. + aria-label="Image viewer" onKeyDown={onKeyDown} + onScroll={readAnchor} className="ui:relative ui:h-full ui:overflow-auto ui:bg-muted ui:focus-visible:focus-ring" > {load.status === "loading" && ( @@ -141,12 +201,8 @@ export function ImageView({
        ); + case "no-text": + return ( +
        +

        No text found

        +

        The document was read, but no text came out of it.

        +
        + ); case "unsupported": return (
        diff --git a/frontend/src/v1/components/patterns/document-viewer/pageFrame.ts b/frontend/src/v1/components/patterns/document-viewer/pageFrame.ts new file mode 100644 index 0000000000..bbedf37590 --- /dev/null +++ b/frontend/src/v1/components/patterns/document-viewer/pageFrame.ts @@ -0,0 +1,7 @@ +/** + * The box a page or an image sits in, shared by PdfPage and ImageView. It is + * sized by `--page-width` and `--page-height`, which each sets in its style + * attribute: the size is runtime geometry, not a token. + */ +export const PAGE_FRAME_CLASS = + "ui:relative ui:h-(--page-height) ui:w-(--page-width) ui:shrink-0 ui:bg-card ui:shadow-sm"; diff --git a/frontend/src/v1/lib/pdf/highlights.ts b/frontend/src/v1/lib/pdf/highlights.ts index 82ec494f53..49f748076b 100644 --- a/frontend/src/v1/lib/pdf/highlights.ts +++ b/frontend/src/v1/lib/pdf/highlights.ts @@ -62,6 +62,9 @@ export interface NormaliseOptions { export const LOW_CONFIDENCE_BELOW = 0.9; +/** One shared empty list, so a default never breaks a memo. Fits any list type. */ +export const NO_HIGHLIGHTS: readonly never[] = []; + function clamp01(value: number): number { return Math.min(1, Math.max(0, value)); } diff --git a/frontend/src/v1/lib/pdf/types.ts b/frontend/src/v1/lib/pdf/types.ts index fd6636a7e8..a408ae8a04 100644 --- a/frontend/src/v1/lib/pdf/types.ts +++ b/frontend/src/v1/lib/pdf/types.ts @@ -39,6 +39,12 @@ export interface ImageSource { export interface TextSource { kind: "text"; text: string; + /** + * Which text this is, across edits. Keep it the same while the text is + * edited live (a HITL correction) and the view keeps its place; a new id is + * a new text, back at the top. Without one, any change of text is a new text. + */ + id?: string; } /** Anything the document viewer shows. */ From 07a75d0d5705ebdfa39c80a5677fe881f6ce3713 Mon Sep 17 00:00:00 2001 From: ali <117142933+muhammad-ali-e@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:24:58 +0000 Subject: [PATCH 7/7] UN-4506 [FIX] Retry a waiting scroll when a text fills in, and anchor the first image zoom on the view - A text kept by its id changes in place, so a scroll waiting on it is retried when the text changes, not only when the highlights do. - ImageView reads its anchor from the view once the image is first laid out, instead of starting from the image's centre. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R7trgcAzu6jxDnit3eeozm --- .../document-viewer/DocumentViewer.test.tsx | 41 +++++++++++++++++++ .../document-viewer/DocumentViewer.tsx | 14 ++++++- .../document-viewer/ImageView.test.tsx | 28 +++++++++++++ .../patterns/document-viewer/ImageView.tsx | 15 +++++-- 4 files changed, 92 insertions(+), 6 deletions(-) diff --git a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx index 021d38ab7a..0d682f4a3b 100644 --- a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.test.tsx @@ -774,3 +774,44 @@ it("drops a scroll to a highlight on another page of an image instead of keeping ); expect(region.scrollTop).toBe(0); }); + +it("runs a scroll waiting on an empty text once that same text fills in", () => { + const original = Object.getOwnPropertyDescriptor( + HTMLElement.prototype, + "offsetTop", + ); + Object.defineProperty(HTMLElement.prototype, "offsetTop", { + configurable: true, + get(this: HTMLElement) { + const line = this.getAttribute("data-line"); + return line === null ? 0 : Number(line) * 20; + }, + }); + try { + const ref = createRef(); + const textHighlights = [{ id: "total", line: 5 }]; + const { rerender } = render( + , + ); + act(() => ref.current?.scrollToHighlight("total")); + // The same text, by id, fills in; the highlights are unchanged. + rerender( + , + ); + expect( + screen.getByRole("region", { name: "Document text" }).scrollTop, + ).toBe(100); + } finally { + if (original) { + Object.defineProperty(HTMLElement.prototype, "offsetTop", original); + } + } +}); diff --git a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx index 17d78a14fd..0c81f74702 100644 --- a/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/DocumentViewer.tsx @@ -173,6 +173,7 @@ export function DocumentViewer({ } const imageSize = source?.kind === "image" && image?.url === source.url ? image.size : null; + const text = source?.kind === "text" ? source.text : null; const pdfReady = kind === "pdf" && state.status === "ready"; const pageSizes = pdfReady ? state.pageSizes @@ -268,7 +269,8 @@ export function DocumentViewer({ ); // A waiting request runs once the pages are laid out, and is tried again - // when the highlights change, in case its highlight has arrived. One made + // when the highlights or the text change, in case its target has arrived + // (a text kept by its id changes in place, without a new source). One made // for another source is dropped rather than scrolling a document nobody // asked about, and so is one made for an active highlight that is no longer // active: this effect runs before the one below that would cancel it. @@ -287,7 +289,15 @@ export function DocumentViewer({ } else { pendingRef.current = null; } - }, [ready, sourceKey, highlights, textHighlights, activeHighlightId, run]); + }, [ + ready, + sourceKey, + highlights, + textHighlights, + text, + activeHighlightId, + run, + ]); // The active highlight is scrolled to when it changes, and again for each // new source, so one chosen before its document arrived is still found. diff --git a/frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx b/frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx index e37f8d71bb..3c082f9ec5 100644 --- a/frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/ImageView.test.tsx @@ -186,3 +186,31 @@ it("names the viewer and the image differently, so neither is read twice", () => expect(screen.getByRole("region", { name: "Image viewer" })).toBeTruthy(); expect(screen.getByRole("img", { name: "Invoice scan" })).toBeTruthy(); }); + +it("anchors the first zoom on where the view is, not on the image's centre", () => { + const { container, rerender } = render( + , + ); + const frame = container.querySelector("img")?.parentElement; + const root = container.querySelector("section"); + if (!frame || !root) { + throw new Error("no image frame"); + } + const px = (name: string) => + Number.parseFloat(frame.style.getPropertyValue(name)) || 0; + Object.defineProperties(frame, { + offsetTop: { value: 16 }, + offsetLeft: { value: 16 }, + offsetWidth: { get: () => px("--page-width") }, + offsetHeight: { get: () => px("--page-height") }, + }); + Object.defineProperties(root, { + clientWidth: { value: 600 }, + clientHeight: { value: 400 }, + }); + // Opened at the top left, wider than the view; never scrolled. + loadImage(1200, 1600); + rerender(); + // The spot at the view's centre (284 px into a 1200 px image) stays there. + expect(root.scrollLeft).toBeCloseTo(16 + (284 / 1200) * 2400 - 300); +}); diff --git a/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx index e7d8305cb6..8faaf125c6 100644 --- a/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx +++ b/frontend/src/v1/components/patterns/document-viewer/ImageView.tsx @@ -136,15 +136,22 @@ export function ImageView({ // A zoom resizes the frame; put the same spot back at the centre. The frame // took its new size from its style at commit. + // Once the image is first laid out, the anchor is wherever the view is, + // not a guess. const previousZoom = useRef(zoom); + const wasReady = useRef(ready); useLayoutEffect(() => { - if (previousZoom.current === zoom) { + const justLoaded = ready && !wasReady.current; + wasReady.current = ready; + const zoomed = previousZoom.current !== zoom; + previousZoom.current = zoom; + if (justLoaded) { + readAnchor(); return; } - previousZoom.current = zoom; const root = rootRef.current; const frame = frameRef.current; - if (!root || !frame || !ready) { + if (!zoomed || !root || !frame || !ready) { return; } const { x, y } = anchorRef.current; @@ -152,7 +159,7 @@ export function ImageView({ frame.offsetLeft + x * frame.offsetWidth - root.clientWidth / 2; root.scrollTop = frame.offsetTop + y * frame.offsetHeight - root.clientHeight / 2; - }, [zoom, ready]); + }, [zoom, ready, readAnchor]); const onLoad = (event: SyntheticEvent) => { const { naturalWidth, naturalHeight } = event.currentTarget;