From 305a20d1d13dac2b5e0ca2521072e4a4cf59baa4 Mon Sep 17 00:00:00 2001 From: tegwick Date: Mon, 8 Jun 2026 01:07:52 +0200 Subject: [PATCH] CE-WP-0008: fix capture field values and viewport scroll retry - Wire fieldValues state in FormsApp so controlled inputs persist typed data - Add runScrollToHighlightJob with rAF retries when utils/highlights not ready - Re-trigger scroll when highlights update after PDF load - Tests: scroll-job unit test, forms-field-values integration tests - Workplan CE-WP-0008 marked done --- src/anchor/pdf-viewer-adapter-spike.tsx | 34 ++--- src/anchor/scroll-job.test.ts | 73 +++++++++++ src/anchor/scroll-job.ts | 73 +++++++++++ src/app/forms/FormsApp.tsx | 13 ++ .../forms-field-values.dom.test.tsx | 82 ++++++++++++ .../CE-WP-0008-capture-content-editing.md | 120 ++++++++++++++++++ workplans/README.md | 6 +- 7 files changed, 384 insertions(+), 17 deletions(-) create mode 100644 src/anchor/scroll-job.test.ts create mode 100644 src/anchor/scroll-job.ts create mode 100644 tests/integration/forms-field-values.dom.test.tsx create mode 100644 workplans/CE-WP-0008-capture-content-editing.md diff --git a/src/anchor/pdf-viewer-adapter-spike.tsx b/src/anchor/pdf-viewer-adapter-spike.tsx index 28a5464..5f9b967 100644 --- a/src/anchor/pdf-viewer-adapter-spike.tsx +++ b/src/anchor/pdf-viewer-adapter-spike.tsx @@ -41,6 +41,7 @@ import "./debug-textlayer.css"; import type { NormalizedRect, Selector } from "@shared/selector"; import type { AnchorResolution, PdfSelectionCapture, ResolvedAnchorTarget } from "./types"; import { findPdfRectSelector, selectorsFromPdfCapture, unionRect } from "./pdf-selector-math"; +import { runScrollToHighlightJob } from "./scroll-job"; export { selectorsFromPdfCapture }; @@ -292,7 +293,7 @@ export function PdfSpikeViewer(props: PdfSpikeViewerProps) { .filter((c): c is string => c !== null) .join(" "); const utilsRef = useRef(null); - const lastScrollKeyRef = useRef(null); + const scrollStateRef = useRef({ lastCompletedKey: null as string | null }); const highlights = useMemo(() => { const out: Highlight[] = []; @@ -324,27 +325,30 @@ export function PdfSpikeViewer(props: PdfSpikeViewerProps) { useEffect(() => { const requestKey = scrollRequestKey ?? scrollToAnnotationId ?? null; if (!requestKey || !scrollToAnnotationId) return; - if (lastScrollKeyRef.current === requestKey) return; - const utils = utilsRef.current; - const target = highlightsRef.current.find((h) => h.id === scrollToAnnotationId); + if (scrollStateRef.current.lastCompletedKey === requestKey) return; + if (debugTextLayer) { console.log("[ce] scrollToAnnotation requested", { id: scrollToAnnotationId, requestKey, - utilsAvailable: !!utils, - targetFound: !!target, + utilsAvailable: !!utilsRef.current, + targetFound: !!highlightsRef.current.find((h) => h.id === scrollToAnnotationId), knownIds: highlightsRef.current.map((h) => h.id), }); } - if (!utils || !target) return; - utils.scrollToHighlight(target); - lastScrollKeyRef.current = requestKey; - // After the library scrolls the page into view, nudge so the highlight - // centre aligns with the scroll container centre (CE-WP-0006-T02). - requestAnimationFrame(() => { - centerHighlightInViewer(utils, target); - }); - }, [scrollToAnnotationId, scrollRequestKey, debugTextLayer]); + + return runScrollToHighlightJob( + { requestKey, annotationId: scrollToAnnotationId }, + { + getUtils: () => utilsRef.current, + findHighlight: (id) => highlightsRef.current.find((h) => h.id === id), + scrollToHighlight: (utils, target) => utils.scrollToHighlight(target), + centerHighlight: (utils, target) => centerHighlightInViewer(utils, target), + scheduleFrame: (fn) => requestAnimationFrame(fn), + }, + scrollStateRef.current, + ); + }, [scrollToAnnotationId, scrollRequestKey, highlights, debugTextLayer]); return (
{ + it("retries until utils and highlight are available", () => { + const frames: Array<() => void> = []; + const scrollToHighlight = vi.fn(); + const centerHighlight = vi.fn(); + let utils: PdfHighlighterUtils | null = null; + let highlight: Highlight | undefined; + + const state = { lastCompletedKey: null as string | null }; + + const cancel = runScrollToHighlightJob( + { requestKey: "ann_test:1", annotationId: "ann_test" }, + { + getUtils: () => utils, + findHighlight: (id) => (id === "ann_test" ? highlight : undefined), + scrollToHighlight: (_u, target) => scrollToHighlight(target), + centerHighlight, + scheduleFrame: (fn) => { + frames.push(fn); + return frames.length; + }, + maxAttempts: 5, + }, + state, + ); + + expect(scrollToHighlight).not.toHaveBeenCalled(); + + // First two frames: still missing utils / highlight. + frames.shift()?.(); + frames.shift()?.(); + expect(scrollToHighlight).not.toHaveBeenCalled(); + + utils = { scrollToHighlight: vi.fn() } as unknown as PdfHighlighterUtils; + highlight = TARGET; + frames.shift()?.(); + + expect(scrollToHighlight).toHaveBeenCalledWith(TARGET); + expect(state.lastCompletedKey).toBe("ann_test:1"); + + frames.shift()?.(); + expect(centerHighlight).toHaveBeenCalledWith(utils, TARGET); + + cancel(); + }); +}); \ No newline at end of file diff --git a/src/anchor/scroll-job.ts b/src/anchor/scroll-job.ts new file mode 100644 index 0000000..a9d3c2d --- /dev/null +++ b/src/anchor/scroll-job.ts @@ -0,0 +1,73 @@ +/** + * Retryable scroll-to-highlight job for PdfSpikeViewer. + * + * The PDF highlighter's utils ref and highlight DOM are not always ready on + * the first effect tick (especially for page-2+ passages). This helper retries + * via rAF until both are available or attempts are exhausted. + */ + +import type { Highlight, PdfHighlighterUtils } from "react-pdf-highlighter-plus"; + +export const DEFAULT_SCROLL_ATTEMPTS = 40; + +export interface ScrollToHighlightJob { + readonly requestKey: string; + readonly annotationId: string; +} + +export interface ScrollToHighlightDeps { + readonly getUtils: () => PdfHighlighterUtils | null; + readonly findHighlight: (annotationId: string) => Highlight | undefined; + readonly scrollToHighlight: ( + utils: PdfHighlighterUtils, + target: Highlight, + ) => void; + readonly centerHighlight: ( + utils: PdfHighlighterUtils, + target: Highlight, + ) => void; + readonly scheduleFrame: (fn: () => void) => number; + readonly maxAttempts?: number; +} + +export interface ScrollToHighlightState { + lastCompletedKey: string | null; +} + +/** + * Attempt scroll for `job`. Returns a cancel function. Sets + * `state.lastCompletedKey` only after a successful scroll. + */ +export function runScrollToHighlightJob( + job: ScrollToHighlightJob, + deps: ScrollToHighlightDeps, + state: ScrollToHighlightState, +): () => void { + let cancelled = false; + let attempt = 0; + const maxAttempts = deps.maxAttempts ?? DEFAULT_SCROLL_ATTEMPTS; + + const tick = () => { + if (cancelled) return; + if (state.lastCompletedKey === job.requestKey) return; + + const utils = deps.getUtils(); + const target = deps.findHighlight(job.annotationId); + if (!utils || !target) { + if (attempt < maxAttempts) { + attempt += 1; + deps.scheduleFrame(tick); + } + return; + } + + deps.scrollToHighlight(utils, target); + state.lastCompletedKey = job.requestKey; + deps.scheduleFrame(() => deps.centerHighlight(utils, target)); + }; + + tick(); + return () => { + cancelled = true; + }; +} \ No newline at end of file diff --git a/src/app/forms/FormsApp.tsx b/src/app/forms/FormsApp.tsx index 663db36..53be648 100644 --- a/src/app/forms/FormsApp.tsx +++ b/src/app/forms/FormsApp.tsx @@ -64,9 +64,14 @@ export function FormsApp() { [schema], ); + const [fieldValues, setFieldValues] = useState>({}); const [showAddFieldForm, setShowAddFieldForm] = useState(false); const [editingFieldId, setEditingFieldId] = useState(null); + const handleFieldValueChange = useCallback((fieldId: string, value: string) => { + setFieldValues((prev) => ({ ...prev, [fieldId]: value })); + }, []); + const nextFieldId = useCallback((fields: readonly FormFieldSchema[]): string => { let max = 0; for (const f of fields) { @@ -119,6 +124,8 @@ export function FormsApp() { { @@ -156,6 +163,8 @@ function ScrollBridge() { function FormPane({ schema, + fieldValues, + onFieldValueChange, showAddFieldForm, editingFieldId, onRequestAddField, @@ -166,6 +175,8 @@ function FormPane({ onCancelFieldEdit, }: { schema: FormSchema; + fieldValues: Readonly>; + onFieldValueChange: (fieldId: string, value: string) => void; showAddFieldForm: boolean; editingFieldId: string | null; onRequestAddField: () => void; @@ -253,6 +264,8 @@ function FormPane({ {document ? ( { + const original = await importOriginal(); + const MockPdfSpikeViewer = (_props: ViewerProps) => ( +
+ ); + return { ...original, PdfSpikeViewer: MockPdfSpikeViewer }; +}); + +const FIXTURE = manifest.fixtures.find((f) => f.id === "fristsetzung-bezifferung")!; +const SYNTHETIC_CANONICAL = ["Pre.", FIXTURE.known_good_quote, "Post."].join(" "); + +import { seedSessionWithDoc } from "./helpers/seed-session"; + +async function loadApp() { + const { App } = await import("@app/App"); + return render(); +} + +describe("Capture — field value persistence (CE-WP-0008-T01)", () => { + beforeEach(() => { + globalThis.localStorage?.clear(); + if (typeof window !== "undefined") { + history.replaceState(null, "", window.location.pathname); + } + seedSessionWithDoc({ + sessionName: "T01-field-values", + documentTitle: FIXTURE.filename, + canonicalText: SYNTHETIC_CANONICAL, + mode: "forms", + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + cleanup(); + }); + + it("keeps typed text after focusing another field", { timeout: 15000 }, async () => { + const user = userEvent.setup(); + await loadApp(); + + const summary = screen.getByLabelText("Summary of the matter"); + await user.type(summary, "Tenant owes arrears"); + await user.click(screen.getByLabelText("Disputed amount")); + await user.click(summary); + + expect((summary as HTMLTextAreaElement).value).toBe("Tenant owes arrears"); + }); + + it("keeps a selected date after blur", { timeout: 15000 }, async () => { + const user = userEvent.setup(); + await loadApp(); + + const deadline = screen.getByLabelText("Key deadline") as HTMLInputElement; + await user.clear(deadline); + await user.type(deadline, "2026-12-15"); + await user.click(screen.getByLabelText("Disputed amount")); + + expect(deadline.value).toBe("2026-12-15"); + }); +}); \ No newline at end of file diff --git a/workplans/CE-WP-0008-capture-content-editing.md b/workplans/CE-WP-0008-capture-content-editing.md new file mode 100644 index 0000000..116efa5 --- /dev/null +++ b/workplans/CE-WP-0008-capture-content-editing.md @@ -0,0 +1,120 @@ +--- +id: CE-WP-0008 +type: workplan +title: "Capture content editing & viewport scroll reliability" +domain: citation_evidence +repo: citation-evidence +repo_id: a677c189-b4e2-4f2a-9e48-faa482c277e6 +topic_slug: citation_evidence_mvp +topic_id: 96fa8e80-9f74-40f2-84cd-644e9747b9ec +status: done +owner: Bernd +created: 2026-06-08 +updated: 2026-06-08 +depends_on_workplan: CE-WP-0007 +planning_order: 8 +planning_priority: high +spec_refs: + - wiki/ProductRequirementsDocument.md + - workplans/CE-WP-0007-capture-view-polish.md +--- + +# CE-WP-0008 — Capture Content Editing & Viewport Scroll + +Follow-on fixes from manual Capture-mode demo use after CE-WP-0007. + +## User requirements (locked) + +1. **Field values persist while typing** — text, textarea, and date inputs must + keep user-entered content; re-renders must not reset controlled inputs. +2. **Date fields hold their value** — picking a date must not snap back to empty + or a stale display value on blur or parent re-render. +3. **Viewport scroll reaches off-page evidence** — selecting evidence whose + passage is not on page 1 must scroll the PDF viewer to that passage (not leave + the viewport at the document top). CE-WP-0007 T01 regressed or was incomplete + when `PdfHighlighter` utils or highlight DOM were not ready on the first + scroll request. + +## Dependency order + +``` +T01 (field value state) ── T02 (viewport scroll retry) + └─ T03 (integration tests) +``` + +--- + +## T01 — Wire form field value state in FormsApp + +```task +id: CE-WP-0008-T01 +priority: critical +status: done +``` + +**Problem:** `FormRenderer` renders controlled inputs (`value={… ?? ""}`) but +`FormsApp` never passes `values` / `onValueChange`. Every React re-render resets +typed content; date inputs snap back immediately. + +**Fix:** + +- Hold `fieldValues: Record` in `FormsApp`. +- Pass `values` and `onValueChange` through `FormPane` → `FormRenderer`. +- Preserve values when field label/type is edited (stable field `id`). + +**Acceptance:** type in Summary, switch focus to another field, return — text +remains. Set a date — value persists after blur. + +--- + +## T02 — Retry scroll until viewer utils and highlight are ready + +```task +id: CE-WP-0008-T02 +priority: critical +status: done +depends_on: [T01] +``` + +**Problem:** `PdfSpikeViewer`'s scroll `useEffect` bails when `utilsRef` or the +target highlight is missing, and never retries because effect deps do not change. +Evidence on page 2+ appears to leave the viewport at the document top. + +**Fix:** + +- Extract `runScrollToHighlightJob` with rAF retries (same pattern as + `centerHighlightInViewer`). +- Re-run pending scroll when `highlights` updates (annotations rendered after PDF + load). +- Keep `lastScrollKeyRef` guard so successful scrolls are not duplicated. + +**Acceptance:** click strip evidence tied to a page-2+ passage — viewer receives +`scrollToAnnotationId` and scroll completes once utils are available. + +--- + +## T03 — Integration tests + +```task +id: CE-WP-0008-T03 +priority: high +status: done +depends_on: [T01, T02] +``` + +Happy-dom tests: + +- Type text and date in capture fields; refocus; values persist. +- Unit test for scroll job retry when utils arrive late. + +**Acceptance:** `npm run test` green. + +--- + +## Acceptance for the workplan + +After CE-WP-0008: + +1. Capture form fields retain typed values across re-renders and focus changes. +2. Date inputs keep the selected date after blur. +3. Evidence selection scrolls to off-page passages instead of staying at the top. \ No newline at end of file diff --git a/workplans/README.md b/workplans/README.md index 6ccf3cc..c0e8489 100644 --- a/workplans/README.md +++ b/workplans/README.md @@ -3,6 +3,7 @@ MVP workplans for the citation-evidence umbrella repo. CE-WP-0001..0006 delivered the PRD §20 reference scenario and Forms/Review UX polish. CE-WP-0007 delivered Capture-view polish including field add/edit UX. +CE-WP-0008 fixes capture field value persistence and viewport scroll reliability. | Workplan | Title | Status | |----------|----------------------------------------|--------| @@ -13,14 +14,15 @@ CE-WP-0007 delivered Capture-view polish including field add/edit UX. | `CE-WP-0005` | Demo sessions — uploads, named sessions, ZIP export/import | done | | `CE-WP-0006` | Forms & review UX refinements — blob fix, scroll centre, linking | done | | `CE-WP-0007` | Capture view polish — scroll, linking, layout, rename, field UX | done | +| `CE-WP-0008` | Capture content editing & viewport scroll reliability | done | ## Order CE-WP-0001..0004 are strictly sequential. CE-WP-0005 depends on 0004. -CE-WP-0006 depends on 0005. CE-WP-0007 depends on 0006: +CE-WP-0006 depends on 0005. CE-WP-0007 depends on 0006. CE-WP-0008 depends on 0007: ``` -/ralph-workplan workplans/CE-WP-0007-capture-view-polish.md +/ralph-workplan workplans/CE-WP-0008-capture-content-editing.md ``` ## How to run a workplan