From 430c0e124cf0ed3e4bf39d13d0e028c9c145a106 Mon Sep 17 00:00:00 2001 From: tegwick Date: Tue, 26 May 2026 22:57:48 +0200 Subject: [PATCH] Refine evidence UX: sidebar capture form, inline edit, click highlight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Significant UX iteration: Visual palette - Debug text-layer overlay flips from yellow to light grey so it no longer collides with the evidence highlight colour. - New highlight-styles.css matches the sidebar's #fff8d6/#e0c050 palette so a passage marked in the document and its sidebar card speak the same visual language. - Active (focused) evidence: same fill, thick #b78b1c outline on both the highlight and the sidebar card. Library's red --scrolledTo box-shadow is suppressed. Activation model - Click an evidence card in the sidebar → activates that item + scrolls the viewer to the passage + thickens the borders (existing behaviour, now visually clearer). - Click a highlight in the document → activates the evidence that owns that annotation. New `findByAnnotationId()` on EvidenceService is the reverse lookup. Wired through a new `onHighlightClicked` prop on PdfSpikeViewer + `activeAnnotationId` prop that drives the data-ce-active attribute on the highlight wrapper. Inline edit - Each evidence card has a ✎ button that flips the card into an inline form with the citation (quote) and commentary fields. - Saving calls a new `AnnotationService.updateQuote()` + existing `EvidenceService.updateCommentary()`. The selectors are untouched, so the marked passage in the document stays put — the inline hint says so explicitly. - New `AnnotationUpdated` event added to the engine event vocabulary (SharedContracts.md §4 updated). Capture form placement - The yellow "New annotation" toolbar that lived above the viewer is gone. A new InlineCaptureForm component is now slotted into the sidebar between the cards that bracket the new selection in document flow (sorted by page + y of the first PdfRectSelector). If the new selection is before all existing evidence it appears at the top; if after all of them, at the bottom. - The legacy AnnotationToolbar.tsx is removed; the public surface re-exports `InlineCaptureForm` instead. Test updates - tests/integration/citation-card-export-e2e.dom.test.tsx: switched to the seed-session helper (matches the other E2Es) since the fixture-button click path is gone. Co-Authored-By: Claude Opus 4.7 --- src/anchor/debug-textlayer.css | 29 +- src/anchor/highlight-styles.css | 38 ++ src/anchor/pdf-viewer-adapter-spike.tsx | 72 ++- src/engine/events/types.ts | 7 + src/engine/services/annotations.ts | 27 + src/engine/services/evidence.ts | 15 + src/work/EvidenceSidebar.tsx | 546 +++++++++++++----- ...ationToolbar.tsx => InlineCaptureForm.tsx} | 45 +- src/work/ViewerShell.tsx | 57 +- src/work/index.ts | 2 +- .../citation-card-export-e2e.dom.test.tsx | 49 +- 11 files changed, 640 insertions(+), 247 deletions(-) create mode 100644 src/anchor/highlight-styles.css rename src/work/{AnnotationToolbar.tsx => InlineCaptureForm.tsx} (68%) diff --git a/src/anchor/debug-textlayer.css b/src/anchor/debug-textlayer.css index a36e578..9c251e7 100644 --- a/src/anchor/debug-textlayer.css +++ b/src/anchor/debug-textlayer.css @@ -2,39 +2,34 @@ * Debug overlay for PDF text layer alignment. * * The text layer is normally invisible (`opacity: 0`) and selectable. - * When `.ce-debug-textlayer` is on a parent, every text span becomes a - * yellow highlight so it's obvious where text is selectable and where it + * When `.ce-debug-textlayer` is on a parent, every text node becomes a + * light grey box so it's obvious where text is selectable and where it * isn't — useful for diagnosing OCR misalignment, scan-only PDFs, and * text-layer shift caused by font fallbacks. * + * Light grey was chosen so the debug overlay does not clash with the + * citation-yellow used for evidence highlights (see highlight-styles.css). + * * Toggle via the "Debug text layer" entry in SessionMenu. */ .ce-debug-textlayer .textLayer { - outline: 2px dashed rgba(255, 0, 0, 0.5); - background: rgba(255, 0, 0, 0.05); + outline: 2px dashed rgba(120, 120, 120, 0.55); + background: rgba(120, 120, 120, 0.06); } /* PDF.js 4.x wraps marked content in nested spans/divs — cover every descendant so the entire selectable area is visible regardless of how the renderer nested things. */ .ce-debug-textlayer .textLayer * { - background: rgba(255, 220, 0, 0.45) !important; - color: rgba(0, 0, 100, 0.85) !important; + background: rgba(170, 170, 170, 0.4) !important; + color: rgba(40, 40, 40, 0.85) !important; opacity: 1 !important; - outline: 1px solid rgba(0, 100, 255, 0.3); + outline: 1px solid rgba(100, 100, 100, 0.35); } -/* Make the canvas-rendered layer dim so the text-layer overlay stands +/* Dim the canvas-rendered layer slightly so the debug overlay stands out by contrast. */ .ce-debug-textlayer canvas { - opacity: 0.35; -} - -/* Make any existing TextHighlight rectangles obvious even in debug - mode (the highlighter's own yellow gets washed out by our debug - yellow). */ -.ce-debug-textlayer .TextHighlight__part { - background: rgba(0, 200, 0, 0.45) !important; - outline: 2px solid rgba(0, 120, 0, 0.7) !important; + opacity: 0.4; } diff --git a/src/anchor/highlight-styles.css b/src/anchor/highlight-styles.css new file mode 100644 index 0000000..a74120e --- /dev/null +++ b/src/anchor/highlight-styles.css @@ -0,0 +1,38 @@ +/* + * Evidence highlight styling — matches the sidebar's "evidence card" + * palette so the viewer and the sidebar speak the same visual language. + * + * .TextHighlight__part inactive highlight (light yellow fill, + * thin amber border) + * .TextHighlight--active … the currently-focused evidence — same + * fill, thicker border + * + * The "active" class is applied by the spike viewer when the parent + * wrapper is marked with `data-ce-active="true"` so a single + * `activeAnnotationId` prop drives the entire viewer's focus state + * without per-highlight component coupling. + * + * We override the library's red `--scrolledTo` box-shadow so an + * activation doesn't flash a red ring that doesn't match the palette. + */ + +.TextHighlight__part { + background: #fff8d6 !important; + outline: 1px solid #e0c050 !important; + outline-offset: 0; + cursor: pointer; + transition: outline 0.15s ease; +} + +[data-ce-active="true"] .TextHighlight__part { + outline: 3px solid #b78b1c !important; + background: #fff5b8 !important; +} + +/* The library applies `--scrolledTo` after a programmatic scroll. We + override its red box-shadow so the "you just landed on this" cue + sticks with the yellow palette. The thicker border from + `data-ce-active` already conveys focus. */ +.TextHighlight--scrolledTo .TextHighlight__part { + box-shadow: none !important; +} diff --git a/src/anchor/pdf-viewer-adapter-spike.tsx b/src/anchor/pdf-viewer-adapter-spike.tsx index 20e21da..b2c501d 100644 --- a/src/anchor/pdf-viewer-adapter-spike.tsx +++ b/src/anchor/pdf-viewer-adapter-spike.tsx @@ -28,6 +28,7 @@ import { } from "react-pdf-highlighter-plus"; import "react-pdf-highlighter-plus/style/style.css"; import "react-pdf-highlighter-plus/style/pdf_viewer.css"; +import "./highlight-styles.css"; import "./debug-textlayer.css"; import type { NormalizedRect, Selector } from "@shared/selector"; @@ -112,20 +113,34 @@ function captureFromPdfSelection(sel: PdfSelection): PdfSelectionCapture { * For the spike, no editing tooling — just visual proof of "did the saved * coordinates land on the right passage on the right page after reload?" */ -function SpikeHighlightContainer(): ReactNode { - const { highlight, isScrolledTo } = useHighlightContainerContext(); - // Wrap the highlight in a data-tagged container so the visual-guide - // overlay's HighlightRectBridge can locate it via DOM query. The - // wrapper uses display: contents so it doesn't affect layout — the - // bounding rect is gathered from the live TextHighlight children at - // query time. - return ( -
- - - -
- ); +interface HighlightContainerProps { + readonly activeAnnotationId: string | null | undefined; + readonly onHighlightClicked: ((annotationId: string) => void) | undefined; +} + +function makeSpikeHighlightContainer(props: HighlightContainerProps) { + return function SpikeHighlightContainer(): ReactNode { + const { highlight, isScrolledTo } = useHighlightContainerContext(); + const isActive = props.activeAnnotationId === highlight.id; + return ( +
{ + // Click-capture so the click registers before the library's + // built-in selection-clearing logic eats it. Stop propagation + // so the highlight click doesn't also start a new selection. + e.stopPropagation(); + props.onHighlightClicked?.(highlight.id); + }} + > + + + +
+ ); + }; } /** @@ -168,7 +183,18 @@ export interface PdfSpikeViewerProps { /** Annotation id to scroll to and highlight on mount, if any. */ readonly scrollToAnnotationId?: string; /** - * When true, paint the PDF text-layer spans in yellow so it's + * Annotation id currently focused. The matching highlight gets a + * thicker border (see highlight-styles.css). `null`/undefined means + * "no active highlight". + */ + readonly activeAnnotationId?: string | null; + /** + * Called when the user clicks an existing highlight in the page. + * The receiver typically activates the matching evidence item. + */ + onHighlightClicked?(annotationId: string): void; + /** + * When true, paint the PDF text-layer spans in light grey so it's * obvious which glyphs have a selectable text overlay and which are * image-only. Also logs every onSelection event to the console. */ @@ -188,7 +214,19 @@ export interface StoredAnnotation { * - scrolls to `scrollToAnnotationId` if its highlight can be reconstructed */ export function PdfSpikeViewer(props: PdfSpikeViewerProps) { - const { pdfUrl, storedAnnotations, onSelectionCaptured, scrollToAnnotationId, debugTextLayer } = props; + const { + pdfUrl, + storedAnnotations, + onSelectionCaptured, + scrollToAnnotationId, + activeAnnotationId, + onHighlightClicked, + debugTextLayer, + } = props; + const HighlightContainer = useMemo( + () => makeSpikeHighlightContainer({ activeAnnotationId, onHighlightClicked }), + [activeAnnotationId, onHighlightClicked], + ); const utilsRef = useRef(null); const [didScroll, setDidScroll] = useState(null); @@ -261,7 +299,7 @@ export function PdfSpikeViewer(props: PdfSpikeViewerProps) { onSelectionCaptured(capture, selectors); }} > - + )} diff --git a/src/engine/events/types.ts b/src/engine/events/types.ts index ae85307..e255b13 100644 --- a/src/engine/events/types.ts +++ b/src/engine/events/types.ts @@ -62,6 +62,12 @@ export interface AnnotationResolutionFailedEvent { readonly reason: string; } +export interface AnnotationUpdatedEvent { + readonly type: "AnnotationUpdated"; + readonly annotationId: AnnotationId; + readonly annotation: Annotation; +} + export interface EvidenceItemCreatedEvent { readonly type: "EvidenceItemCreated"; readonly evidenceItemId: EvidenceItemId; @@ -128,6 +134,7 @@ export type EngineEvent = | DocumentRepresentationGeneratedEvent | DocumentRemovedEvent | AnnotationCreatedEvent + | AnnotationUpdatedEvent | AnnotationResolvedEvent | AnnotationResolutionFailedEvent | EvidenceItemCreatedEvent diff --git a/src/engine/services/annotations.ts b/src/engine/services/annotations.ts index 6a25e27..5973f51 100644 --- a/src/engine/services/annotations.ts +++ b/src/engine/services/annotations.ts @@ -39,6 +39,12 @@ export interface AnnotationService { status: AnnotationResolutionStatus, opts: { readonly confidence: number; readonly reason?: string }, ): Annotation; + /** + * Edit the human-facing `quote` text on an annotation without touching + * the underlying selectors. Selectors stay the source of truth for + * locating the passage; the quote is the user's editable display copy. + */ + updateQuote(id: AnnotationId, quote: string): Annotation; } export function createAnnotationService( @@ -98,5 +104,26 @@ export function createAnnotationService( } return stored; }, + updateQuote(id, quote) { + const existing = annotations.get(id); + if (!existing) { + throw new Error(`AnnotationService.updateQuote: unknown id ${id}`); + } + const trimmed = quote.length === 0 ? undefined : quote; + const updated: Annotation = { + ...existing, + // exactOptionalPropertyTypes: drop `quote` when empty rather + // than setting it to undefined. + ...(trimmed !== undefined ? { quote: trimmed } : {}), + updatedAt: now(), + }; + if (trimmed === undefined && "quote" in updated) { + // Remove the field outright when clearing. + delete (updated as { quote?: string }).quote; + } + const stored = annotations.update(updated); + bus.emit({ type: "AnnotationUpdated", annotationId: stored.id, annotation: stored }); + return stored; + }, }; } diff --git a/src/engine/services/evidence.ts b/src/engine/services/evidence.ts index 49bf71c..b206f78 100644 --- a/src/engine/services/evidence.ts +++ b/src/engine/services/evidence.ts @@ -44,6 +44,15 @@ export interface EvidenceService { id: EvidenceItemId, source?: EvidenceItemActivatedEvent["source"], ): EvidenceItem; + /** + * Reverse lookup: find the evidence item that owns a given annotation. + * Used by the viewer's click-on-highlight handler so a click on the + * passage activates the right sidebar row. + */ + findByAnnotationId( + documentId: DocumentId, + annotationId: AnnotationId, + ): EvidenceItem | null; } export function createEvidenceService( @@ -123,5 +132,11 @@ export function createEvidenceService( }); return existing; }, + findByAnnotationId(documentId, annotationId) { + for (const item of items.listByDocument(documentId, annotationLookup)) { + if (item.annotationIds.includes(annotationId)) return item; + } + return null; + }, }; } diff --git a/src/work/EvidenceSidebar.tsx b/src/work/EvidenceSidebar.tsx index 96cccaa..2823b09 100644 --- a/src/work/EvidenceSidebar.tsx +++ b/src/work/EvidenceSidebar.tsx @@ -1,18 +1,27 @@ /** * EvidenceSidebar — the right pane. * - * Lists `EvidenceItem`s scoped to the currently-active document. Each row - * shows quote + commentary + status. Clicking a row emits - * `EvidenceItemActivated` via the engine, which T08 will translate into a - * scroll-to-passage in the viewer. + * Lists `EvidenceItem`s scoped to the active document, sorted by their + * position in the document (first PdfRectSelector's page + y). Each row: * - * CE-WP-0004-T04 added: a per-item Export popover (Copy as Markdown / - * Copy as HTML), a transient toast confirming the copy, and the - * Cmd/Ctrl+Shift+C keyboard shortcut that exports the currently-active - * evidence as Markdown. + * - Click → activates the evidence item (highlights its passage in + * the viewer + thickens its border). + * - Edit pencil → inline form to change the citation quote and + * commentary. The underlying selectors stay untouched, so the + * marked passage in the document doesn't move. + * - Export popover → copy as Markdown / HTML (CE-WP-0004). + * + * The "create new evidence from a fresh selection" form + * (`InlineCaptureForm`) is slotted into the list at the right + * document-flow position whenever there is a pending selection — so a + * new capture appears between the cards that bracket it, or at the + * top/bottom if it's the first or last passage in the document. + * + * Cmd/Ctrl+Shift+C exports the active evidence as Markdown. */ import { + Fragment, useCallback, useEffect, useMemo, @@ -20,14 +29,18 @@ import { useState, type CSSProperties, } from "react"; +import type { Annotation } from "@shared/annotation"; import type { EvidenceItem } from "@shared/evidence"; -import type { EvidenceItemId } from "@shared/ids"; +import type { AnnotationId, EvidenceItemId } from "@shared/ids"; +import type { PdfRectSelector, Selector } from "@shared/selector"; + import { useActiveDocument, useEngine, useEngineEventTick, useEngineRevision, useLastActivatedEvidence, + usePendingSelection, useScrollToAnnotation, } from "./EngineContext"; import { @@ -35,6 +48,7 @@ import { type ExportFormat, type ExportResult, } from "./useExportEvidence"; +import { InlineCaptureForm } from "./InlineCaptureForm"; const TOAST_TIMEOUT_MS = 2000; @@ -45,7 +59,6 @@ export interface EvidenceSidebarProps { interface ToastState { readonly message: string; readonly tone: "success" | "error"; - /** Bumps on every new toast so timers don't dismiss the *next* toast. */ readonly key: number; } @@ -67,23 +80,84 @@ function describeSuccess(format: ExportFormat): string { return format === "markdown" ? "Copied as Markdown" : "Copied as HTML"; } +/** + * A sortable scalar key for "where in the document is this passage". + * Page-first, then y-coordinate (0..1 within the page). Returns + * Infinity for items without a usable position so they sink to the + * bottom. The same scheme is used for `EvidenceItem`s (via their + * first annotation) and for the pending selection's capture. + */ +function docOrderKey(selectors: readonly Selector[]): number { + for (const s of selectors) { + if (s.type === "PdfRectSelector") { + const rect: PdfRectSelector = s; + const top = rect.rects[0]?.y ?? 0; + return rect.page * 1000 + top; + } + } + return Number.POSITIVE_INFINITY; +} + +function annotationOrderKey(annotation: Annotation | null): number { + if (!annotation) return Number.POSITIVE_INFINITY; + return docOrderKey(annotation.selectors); +} + export function EvidenceSidebar(props: EvidenceSidebarProps) { const engine = useEngine(); const { document } = useActiveDocument(); const { scrollTo } = useScrollToAnnotation(); const activeId = useLastActivatedEvidence(); const { exportItem } = useExportEvidence(); + const { pending } = usePendingSelection(); const createTick = useEngineEventTick("EvidenceItemCreated"); const updateTick = useEngineEventTick("EvidenceItemUpdated"); + const annotationUpdateTick = useEngineEventTick("AnnotationUpdated"); const revision = useEngineRevision(); - const items = useMemo(() => { - if (!document) return []; - return engine.evidence.listByDocument(document.id); - }, [document, engine, createTick, updateTick, revision]); + // Build the sorted view-model: each item gets its order key + the + // first annotation up-front so the render below doesn't have to + // re-resolve them inside the map. + const sortedItems = useMemo(() => { + if (!document) return [] as readonly { item: EvidenceItem; annotation: Annotation | null; order: number }[]; + const items = engine.evidence.listByDocument(document.id); + const out = items.map((item) => { + const firstAnnId = item.annotationIds[0]; + const annotation = firstAnnId ? engine.annotations.get(firstAnnId) : null; + return { item, annotation, order: annotationOrderKey(annotation) }; + }); + out.sort((a, b) => a.order - b.order); + return out; + }, [ + document, + engine, + createTick, + updateTick, + annotationUpdateTick, + revision, + ]); + + const pendingOrder = useMemo(() => { + if (!pending) return Number.POSITIVE_INFINITY; + const c = pending.capture; + return c.page * 1000 + (c.boundingRect?.y ?? 0); + }, [pending]); + + // Find the insert position for the pending capture form: first index + // whose order > pendingOrder, or sortedItems.length to append. + const pendingInsertIndex = useMemo(() => { + if (!pending) return -1; + for (let i = 0; i < sortedItems.length; i++) { + if (sortedItems[i]!.order > pendingOrder) return i; + } + return sortedItems.length; + }, [pending, pendingOrder, sortedItems]); const [openExportFor, setOpenExportFor] = useState(null); + const [editingId, setEditingId] = useState(null); + const [editQuote, setEditQuote] = useState(""); + const [editCommentary, setEditCommentary] = useState(""); const [toast, setToast] = useState(null); const toastKeyRef = useRef(0); @@ -127,6 +201,48 @@ export function EvidenceSidebar(props: EvidenceSidebarProps) { return () => window.removeEventListener("keydown", handler); }, [activeId, engine, runExport]); + const activateItem = useCallback( + (item: EvidenceItem, firstAnnotationId: AnnotationId | undefined) => { + engine.evidence.activate(item.id, "sidebar"); + if (firstAnnotationId) scrollTo(firstAnnotationId); + props.onActivate?.(item); + }, + [engine, scrollTo, props], + ); + + const beginEdit = useCallback( + (item: EvidenceItem, annotation: Annotation | null) => { + setEditingId(item.id); + setEditQuote(annotation?.quote ?? ""); + setEditCommentary(item.commentary ?? ""); + setOpenExportFor(null); + }, + [], + ); + + const cancelEdit = useCallback(() => { + setEditingId(null); + }, []); + + const saveEdit = useCallback( + (item: EvidenceItem, annotation: Annotation | null) => { + try { + if (annotation) { + engine.annotations.updateQuote(annotation.id, editQuote); + } + // updateCommentary expects a string — empty string clears it. + engine.evidence.updateCommentary(item.id, editCommentary); + setEditingId(null); + } catch (err) { + showToast( + err instanceof Error ? `Save failed: ${err.message}` : "Save failed", + "error", + ); + } + }, + [engine, editQuote, editCommentary, showToast], + ); + return (