Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
| 2026-08-20 | PR #2199 | 175c641321e58089d452ed76718972b8d7f705ca | PR #2199 on-demand document search — Codex P2 review-comment resolution | Fixed: the document-viewer visual-baseline target still declared .document-viewer-composer as a mask, but the composer is no longer rendered until Search document is pressed, and assertMaskSelectors fails loudly on a mask matching zero nodes — which classify-visual-baseline-outcome.mjs scores as a NON-drift failure, turning the advisory job hard red on every main push. Mask reduced to the sticky header, with the reasoning recorded in the target comment. Reviewed the rest of the diff: stale-response protection holds (closeDocumentSearch clears sourceSearch, whose effect cleanup aborts the in-flight controller before the reset run), focus restoration and Escape are covered by the Chromium journeys. | npm run verify:pr-local: check:runtime, check:installed-lock-parity, format:changed, lint, typecheck all passed; npm run test 692/693 files, 7592 passed, 4 skipped, 1 failed — tests/guard-push.test.ts 'inFlightCiGuard formats actionable blocked message', byte-identical to origin/main and failing only because gh is absent in this container so inFlightCiGuard fails open before the injected runFetcher is used; CI Unit coverage passed on this head. Browser proof delegated to CI Production UI: local Playwright pins chromium 1234, image ships 1194 (#255). |
103 changes: 62 additions & 41 deletions src/components/DocumentViewer.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,11 +8,11 @@ import {
Download,
Ellipsis,
ExternalLink,
FilePlus2,
FileText,
Loader2,
Search,
Sparkles,
X,
} from "lucide-react";
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { documentDisplayTitle } from "@/components/DocumentOrganizationBadges";
Expand DownExpand Up@@ -149,7 +149,10 @@ export function DocumentViewer({
// change / successful reload so only an unrecoverable URL exhausts the budget.
const signedUrlRefreshCountRef = useRef(0);
const sourceSearchInputRef = useRef<HTMLInputElement | null>(null);
const documentActionsTriggerRef = useRef<HTMLButtonElement | null>(null);
const documentSearchReturnFocusRef = useRef<HTMLElement | null>(null);
const viewerRootRef = useRef<HTMLElement | null>(null);
const [documentSearchOpen, setDocumentSearchOpen] = useState(false);
const [sourceSearch, setSourceSearch] = useState("");
const [documentSearchState, setDocumentSearchState] = useState<{
query: string;
Expand DownExpand Up@@ -212,6 +215,7 @@ export function DocumentViewer({
mobileActionsOpen || sectionSheetOpen,
composerChromeFocused,
);
const composerVisible = documentSearchOpen && !composerScrollHidden;
const activeScrollOwner = useActiveScrollOwner(shellScrollContainer, documentId);
// DocumentFrame owns every viewing control for the canvas PDF — there is one
// reader, so there is one toolbar. Reset viewing chrome when the document
Expand All@@ -228,6 +232,12 @@ export function DocumentViewer({
const [pdfPageCount, setPdfPageCount] = useState<number | null>(null);
if (pdfViewingDocumentId !== documentId) {
setPdfViewingDocumentId(documentId);
setDocumentSearchOpen(false);
setSourceSearch("");
setDocumentSearchState({ query: "", results: [] });
setSearchingDocument(false);
setDocumentSearchError(null);
setInspectRevealKey(null);
setPdfFitWidth(true);
setPdfZoom(VIEWER_DEFAULT_ZOOM);
setPdfViewingAid(false);
Expand DownExpand Up@@ -319,6 +329,7 @@ export function DocumentViewer({
setIndexHealth(null);
setSummary(null);
setSummaryError(null);
setDocumentSearchOpen(false);
setSourceSearch("");
setDocumentSearchState({ query: "", results: [] });
setDocumentSearchError(null);
Expand DownExpand Up@@ -899,9 +910,6 @@ export function DocumentViewer({
? "Document"
: "Source unavailable";
const documentHomeHref = "/?mode=documents";
const scopedDocumentHref = readyDocument
? `/?mode=documents&q=${encodeURIComponent(documentDisplayTitle(readyDocument))}&documentId=${encodeURIComponent(documentId)}`
: documentHomeHref;
const usefulPageHref = (page: number) => documentPageHref(documentId, page);
const canSummarizeDocument = viewerState === "ready" && !loadingSummary && canUsePrivateApis;
const summarizeTitle = !canUsePrivateApis
Expand DownExpand Up@@ -1051,6 +1059,28 @@ export function DocumentViewer({
behavior: resolveScrollBehavior(),
});
};
const openDocumentSearch = (returnFocusTarget?: HTMLElement | null) => {
documentSearchReturnFocusRef.current = returnFocusTarget ?? documentActionsTriggerRef.current;
setMobileActionsOpen(false);
setDocumentSearchOpen(true);
window.requestAnimationFrame(() => {
window.requestAnimationFrame(() => sourceSearchInputRef.current?.focus());
});
};
const closeDocumentSearch = () => {
const returnFocusTarget = documentSearchReturnFocusRef.current;
setDocumentSearchOpen(false);
setComposerChromeFocused(false);
setSourceSearch("");
setDocumentSearchState({ query: "", results: [] });
setSearchingDocument(false);
setDocumentSearchError(null);
setInspectRevealKey(null);
window.requestAnimationFrame(() => {
const focusTarget = returnFocusTarget?.isConnected ? returnFocusTarget : documentActionsTriggerRef.current;
focusTarget?.focus();
});
};
async function reviewTableFact(fact: TableFactRow, reviewClass: string) {
setReviewingTableFactId(fact.id);
try {
Expand DownExpand Up@@ -1147,6 +1177,7 @@ export function DocumentViewer({
<div className="ml-auto flex shrink-0 items-center">
<button
type="button"
ref={documentActionsTriggerRef}
onClick={() => setMobileActionsOpen(true)}
className="grid h-tap w-tap place-items-center rounded-xl border border-[color:var(--border-lux)] bg-[color:var(--surface-raised)] text-[color:var(--text-muted)] shadow-[var(--shadow-inset)] transition hover:border-[color:var(--border-strong)] hover:bg-[color:var(--surface-subtle)] hover:text-[color:var(--text)] focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]"
aria-label="Open document actions"
Expand DownExpand Up@@ -1201,18 +1232,15 @@ export function DocumentViewer({
<div className="grid grid-cols-1 gap-2 min-[420px]:grid-cols-2">
<button
type="button"
onClick={() => {
setMobileActionsOpen(false);
window.requestAnimationFrame(() => {
window.requestAnimationFrame(() => sourceSearchInputRef.current?.focus());
});
}}
onClick={() => openDocumentSearch(documentActionsTriggerRef.current)}
aria-expanded={documentSearchOpen}
aria-controls={documentSearchOpen ? "document-viewer-search" : undefined}
className={cn(secondaryButton, "min-h-14 justify-start gap-3 px-3 text-left text-sm")}
>
<span className="grid h-8 w-8 shrink-0 place-items-center rounded-lg bg-[color:var(--clinical-accent-soft)] text-[color:var(--clinical-accent)]">
<Search aria-hidden="true" className="h-4 w-4" />
</span>
Search in document
Search document
</button>
<button
type="button"
Expand DownExpand Up@@ -1276,19 +1304,6 @@ export function DocumentViewer({
</span>
{downloadingSource ? "Preparing PDF" : "Download PDF"}
</button>
<button
type="button"
onClick={() => {
setMobileActionsOpen(false);
router.push(scopedDocumentHref);
}}
className={cn(secondaryButton, "min-h-14 justify-start gap-3 px-3 text-left text-sm")}
>
<span className="grid h-8 w-8 shrink-0 place-items-center rounded-lg bg-[color:var(--surface-subtle)] text-[color:var(--text-muted)]">
<FilePlus2 aria-hidden="true" className="h-4 w-4" />
</span>
Add to scope
</button>
</div>
{canUseAdministrativeApis ? (
<details className="rounded-lg border border-[color:var(--border)] bg-[color:var(--surface-subtle)] p-3">
Expand All@@ -1310,14 +1325,14 @@ export function DocumentViewer({

<section
data-testid="document-viewer-content"
data-scroll-hidden={composerScrollHidden ? "true" : undefined}
data-reserve-transitioning={reserveTransitioning ? "true" : undefined}
data-scroll-hidden={documentSearchOpen && composerScrollHidden ? "true" : undefined}
data-reserve-transitioning={documentSearchOpen && reserveTransitioning ? "true" : undefined}
data-phone-scroll-owner={activeScrollOwner}
data-phone-footer-owner={readyDocument ? "document-viewer" : "none"}
data-phone-footer-owner={readyDocument && documentSearchOpen ? "document-viewer" : "none"}
data-phone-composer-reserve={
composerScrollHidden ? "0.75rem" : "calc(9rem + var(--safe-area-bottom) + var(--keyboard-height, 0px))"
composerVisible ? "calc(9rem + var(--safe-area-bottom) + var(--keyboard-height, 0px))" : "0.75rem"
}
data-phone-chrome-transition={reserveTransitioning ? "active" : "idle"}
data-phone-chrome-transition={documentSearchOpen && reserveTransitioning ? "active" : "idle"}
data-document-view={compactView ? "condensed" : "full"}
// Hidden state releases the composer's own 9rem clearance, but keeps a
// small resting gap (0.75rem — the same figure the floating pill itself
Expand All@@ -1334,14 +1349,14 @@ export function DocumentViewer({
// explicit track this is an implicit `auto` column sized by its items'
// min-content, so a single child that forgets `min-w-0` can widen the
// whole page past the viewport and get clipped by `overflow-x: clip`.
"mx-auto grid max-w-[1440px] grid-cols-1 gap-4 px-3 py-4 sm:gap-5 sm:px-4 sm:py-5 sm:pb-40 lg:grid-cols-[minmax(0,1fr)_480px] lg:items-start lg:px-8",
"mx-auto grid max-w-[1440px] grid-cols-1 gap-4 px-3 py-4 sm:gap-5 sm:px-4 sm:py-5 lg:grid-cols-[minmax(0,1fr)_480px] lg:items-start lg:px-8",
// The visible fixed composer needs endpoint clearance. Once hidden,
// release the composer-height clearance so Safari can paint document
// content beneath its translucent toolbar instead of showing a blank
// band — but keep a small 0.75rem resting pad (see comment above).
composerScrollHidden
? "max-sm:pb-3"
: "max-sm:pb-[calc(9rem+var(--safe-area-bottom)+var(--keyboard-height,0px))] max-sm:[--phone-focus-bottom-clearance:calc(9rem+var(--safe-area-bottom)+var(--keyboard-height,0px))]",
composerVisible
? "max-sm:pb-[calc(9rem+var(--safe-area-bottom)+var(--keyboard-height,0px))] max-sm:[--phone-focus-bottom-clearance:calc(9rem+var(--safe-area-bottom)+var(--keyboard-height,0px))] sm:pb-40"
: "max-sm:pb-3",
)}
>
{downloadError ? (
Expand DownExpand Up@@ -1398,7 +1413,8 @@ export function DocumentViewer({
signedUrl={signedUrl}
pages={pages}
onAskFromDocument={() => void summarize()}
onAddToScope={() => router.push(scopedDocumentHref)}
onSearchDocument={(event) => openDocumentSearch(event.currentTarget)}
searchOpen={documentSearchOpen}
onDownload={() => void openSourceDownload()}
downloading={downloadingSource}
canSummarizeDocument={canSummarizeDocument}
Expand DownExpand Up@@ -1582,13 +1598,20 @@ export function DocumentViewer({
onSelectPage={navigateToPage}
/>
</section>
{readyDocument ? (
{readyDocument && documentSearchOpen ? (
Comment thread
BigSimmo marked this conversation as resolved.
<PhoneFooterLayerPortal>
<form
id="document-viewer-search"
onSubmit={(event) => {
event.preventDefault();
submitSourceSearch();
}}
onKeyDown={(event) => {
if (event.key !== "Escape") return;
event.preventDefault();
event.stopPropagation();
closeDocumentSearch();
}}
data-scroll-hidden={composerScrollHidden ? "true" : undefined}
onFocusCapture={() => setComposerChromeFocused(true)}
onBlurCapture={(event) => {
Expand All@@ -1604,14 +1627,12 @@ export function DocumentViewer({
>
<button
type="button"
onClick={() => setMobileActionsOpen(true)}
onClick={closeDocumentSearch}
className="grid h-tap w-tap shrink-0 place-items-center rounded-full text-[color:var(--text-muted)] hover:bg-[color:var(--surface-subtle)] hover:text-[color:var(--text)]"
aria-label="Open document actions"
aria-haspopup="dialog"
aria-expanded={mobileActionsOpen}
title="Document actions"
aria-label="Close document search"
title="Close document search"
>
<Ellipsis aria-hidden="true" className="h-5 w-5" strokeWidth={2.25} />
<X aria-hidden="true" className="h-5 w-5" strokeWidth={2.25} />
</button>
<label className="relative flex min-w-0 flex-1 items-center overflow-hidden">
<span className="sr-only">Search within this document</span>
Expand Down
27 changes: 17 additions & 10 deletions src/components/document-viewer/document-overview-landing.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,7 +5,8 @@
// PDF on phones — most of the first viewport spent before any of the source.
// DocumentViewer now places it after the source, matching the order
// `buildDocumentSectionIndex` has always described.
import { Download, Loader2, MoreHorizontal, Sparkles, Target } from "lucide-react";
import { Download, Loader2, MoreHorizontal, Search, Sparkles } from "lucide-react";
import type { MouseEventHandler } from "react";
import { documentDisplayTitle, documentOrganizationProfile } from "@/components/DocumentOrganizationBadges";
import { formatDocumentLabelDisplay } from "@/lib/document-tags";
import {
Expand DownExpand Up@@ -48,7 +49,8 @@ export function DocumentOverviewLanding({
signedUrl,
pages,
onAskFromDocument,
onAddToScope,
onSearchDocument,
searchOpen,
onDownload,
downloading,
canSummarizeDocument,
Expand All@@ -57,7 +59,8 @@ export function DocumentOverviewLanding({
signedUrl: string | null;
pages: PageRow[];
onAskFromDocument: () => void;
onAddToScope: () => void;
onSearchDocument: MouseEventHandler<HTMLButtonElement>;
searchOpen: boolean;
onDownload: () => void;
downloading: boolean;
canSummarizeDocument: boolean;
Expand DownExpand Up@@ -87,7 +90,7 @@ export function DocumentOverviewLanding({
{/* Search relevance badges are rendered in document search results; the viewer has no ranking context. */}
</div>
</div>
{/* Phone: primary reading actions only. Download / scope stay behind More
{/* Phone: primary reading actions only. Download / search stay behind More
so the first viewport reaches the PDF and clinical summary faster. */}
<div className="mt-3 grid grid-cols-2 gap-2 sm:hidden">
{signedUrl ? (
Expand DownExpand Up@@ -136,11 +139,13 @@ export function DocumentOverviewLanding({
{downloading ? "Preparing" : "Download"}
</DocumentActionButton>
<DocumentActionButton
onClick={onAddToScope}
icon={Target}
onClick={onSearchDocument}
icon={Search}
aria-expanded={searchOpen}
aria-controls={searchOpen ? "document-viewer-search" : undefined}
className={cn(secondaryButton, "w-full min-h-12 px-2 text-xs")}
>
Add to scope
Search document
</DocumentActionButton>
</div>
</details>
Expand DownExpand Up@@ -171,11 +176,13 @@ export function DocumentOverviewLanding({
{downloading ? "Preparing" : "Download"}
</DocumentActionButton>
<DocumentActionButton
onClick={onAddToScope}
icon={Target}
onClick={onSearchDocument}
icon={Search}
aria-expanded={searchOpen}
aria-controls={searchOpen ? "document-viewer-search" : undefined}
className={cn(secondaryButton, "w-full min-h-12 px-2 text-sm")}
>
Add to scope
Search document
</DocumentActionButton>
<DocumentActionButton
onClick={onAskFromDocument}
Expand Down
Loading
Loading