generated from coulomb/repo-seed
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
This commit is contained in:
@@ -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<PdfHighlighterUtils | null>(null);
|
||||
const lastScrollKeyRef = useRef<string | null>(null);
|
||||
const scrollStateRef = useRef({ lastCompletedKey: null as string | null });
|
||||
|
||||
const highlights = useMemo<Highlight[]>(() => {
|
||||
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 (
|
||||
<div
|
||||
|
||||
73
src/anchor/scroll-job.test.ts
Normal file
73
src/anchor/scroll-job.test.ts
Normal file
@@ -0,0 +1,73 @@
|
||||
/**
|
||||
* CE-WP-0008-T02 — scroll job retries until utils and highlight exist.
|
||||
*/
|
||||
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { Highlight, PdfHighlighterUtils } from "react-pdf-highlighter-plus";
|
||||
|
||||
import { runScrollToHighlightJob } from "./scroll-job";
|
||||
|
||||
const TARGET = {
|
||||
id: "ann_test",
|
||||
type: "text",
|
||||
content: { text: "quote" },
|
||||
position: {
|
||||
boundingRect: {
|
||||
x1: 0,
|
||||
y1: 0,
|
||||
x2: 1,
|
||||
y2: 1,
|
||||
width: 1,
|
||||
height: 1,
|
||||
pageNumber: 2,
|
||||
},
|
||||
rects: [],
|
||||
},
|
||||
} as Highlight;
|
||||
|
||||
describe("runScrollToHighlightJob (CE-WP-0008-T02)", () => {
|
||||
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();
|
||||
});
|
||||
});
|
||||
73
src/anchor/scroll-job.ts
Normal file
73
src/anchor/scroll-job.ts
Normal file
@@ -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;
|
||||
};
|
||||
}
|
||||
@@ -64,9 +64,14 @@ export function FormsApp() {
|
||||
[schema],
|
||||
);
|
||||
|
||||
const [fieldValues, setFieldValues] = useState<Record<string, string>>({});
|
||||
const [showAddFieldForm, setShowAddFieldForm] = useState(false);
|
||||
const [editingFieldId, setEditingFieldId] = useState<string | null>(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() {
|
||||
<ViewerShell />
|
||||
<FormPane
|
||||
schema={schema}
|
||||
fieldValues={fieldValues}
|
||||
onFieldValueChange={handleFieldValueChange}
|
||||
showAddFieldForm={showAddFieldForm}
|
||||
editingFieldId={editingFieldId}
|
||||
onRequestAddField={() => {
|
||||
@@ -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<Record<string, string>>;
|
||||
onFieldValueChange: (fieldId: string, value: string) => void;
|
||||
showAddFieldForm: boolean;
|
||||
editingFieldId: string | null;
|
||||
onRequestAddField: () => void;
|
||||
@@ -253,6 +264,8 @@ function FormPane({
|
||||
{document ? (
|
||||
<FormRenderer
|
||||
schema={schema}
|
||||
values={fieldValues}
|
||||
onValueChange={onFieldValueChange}
|
||||
linkCounts={linkCounts}
|
||||
linkHints={linkHints}
|
||||
showAddFieldForm={showAddFieldForm}
|
||||
|
||||
82
tests/integration/forms-field-values.dom.test.tsx
Normal file
82
tests/integration/forms-field-values.dom.test.tsx
Normal file
@@ -0,0 +1,82 @@
|
||||
/**
|
||||
* CE-WP-0008-T01 — capture form field values persist across re-renders.
|
||||
*/
|
||||
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { cleanup, render, screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import type { Selector } from "@shared/selector";
|
||||
|
||||
import type { PdfSelectionCapture } from "@anchor/index";
|
||||
import manifest from "../../fixtures/pdfs/manifest.json" with { type: "json" };
|
||||
|
||||
interface ViewerProps {
|
||||
pdfUrl: string;
|
||||
storedAnnotations: readonly { id: string; text: string; selectors: readonly Selector[] }[];
|
||||
onSelectionCaptured(capture: PdfSelectionCapture, selectors: Selector[]): void;
|
||||
}
|
||||
|
||||
vi.mock("@anchor/index", async (importOriginal) => {
|
||||
const original = await importOriginal<typeof import("@anchor/index")>();
|
||||
const MockPdfSpikeViewer = (_props: ViewerProps) => (
|
||||
<div data-testid="mock-pdf-viewer" />
|
||||
);
|
||||
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(<App />);
|
||||
}
|
||||
|
||||
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");
|
||||
});
|
||||
});
|
||||
120
workplans/CE-WP-0008-capture-content-editing.md
Normal file
120
workplans/CE-WP-0008-capture-content-editing.md
Normal file
@@ -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<string, string>` 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.
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user