Polish pass: removed lastTouched dead code, selective useWatch perf, token sweep, required-field messages, scroll-to-first-error, file prefill display, matrix sticky shadow

This commit is contained in:
Joel Brock
2026-05-09 22:49:25 -07:00
parent e452fbb15f
commit 0d84b9654b
4 changed files with 180 additions and 89 deletions

View File

@@ -1,8 +1,21 @@
"use client";
import { useEffect, useMemo, useRef, useState } from "react";
import { useForm } from "react-hook-form";
import type { FormConfig, FormDataPayload, SubmitPayload } from "@/types/form";
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { useForm, useWatch, type FieldErrors, type FieldValues } from "react-hook-form";
import type { FormConfig, FormDataPayload, SubmitPayload, StageSectionConfig } from "@/types/form";
function sectionForField(
sections: ReadonlyArray<StageSectionConfig>,
fieldName: string,
): StageSectionConfig | undefined {
for (const s of sections) {
if (s.fields.some((f) => f.name === fieldName)) return s;
for (const m of s.matrixGroups ?? []) {
for (const row of m.rows) if (row.fields.includes(fieldName)) return s;
}
}
return undefined;
}
import { evaluate } from "@/lib/conditional";
import { StageSection } from "./StageSection";
import { loadDraft, saveDraft, clearDraft } from "@/lib/draft";
@@ -43,11 +56,33 @@ export function EngagementForm({ config, cid, cs }: EngagementFormProps) {
register,
handleSubmit,
reset,
control,
watch,
setFocus,
formState: { errors, isDirty },
} = useForm({ mode: "onBlur" });
const formValues = watch();
// Subscribe ONLY to current_stage. That's the single field that affects
// section visibility, so re-rendering the whole form on every keystroke
// (which `watch()` with no args would do) is wasteful — particularly with
// 120 fields and a 39-cell matrix in Stage 5.
//
// The auto-save effect uses watch's subscription API for side-effect-only
// change tracking (no re-render).
const currentStageValue =
(useWatch({ control, name: "current_stage" }) as string | undefined) ?? "";
// State map passed to the conditional engine and to FieldRenderer for
// readonly display values. Only fields referenced by visibility rules or
// by readonly displays need to be here. Both readonly fields in this form
// (current_stage, stage_at_submission) display the org's current stage.
const evalState = useMemo(
() => ({
current_stage: currentStageValue,
stage_at_submission: currentStageValue,
}),
[currentStageValue],
);
// ── Initial load ───────────────────────────────────────────────────────
useEffect(() => {
@@ -103,34 +138,46 @@ export function EngagementForm({ config, cid, cs }: EngagementFormProps) {
}, [cid, cs, reset, config.stageField]);
// ── Auto-save draft on idle ────────────────────────────────────────────
// Debounce — save 1.5s after the user stops editing.
// Debounce — save 1.5s after the user stops editing. Uses watch's
// subscription API so we get notified on every change WITHOUT re-rendering
// the form on each keystroke.
const saveTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
useEffect(() => {
if (load.kind !== "ready") return;
if (!isDirty) return;
if (saveTimer.current) clearTimeout(saveTimer.current);
saveTimer.current = setTimeout(() => {
saveDraft(cid, formValues);
setDraftSavedAt(new Date().toISOString());
}, 1500);
const sub = watch((values) => {
if (saveTimer.current) clearTimeout(saveTimer.current);
saveTimer.current = setTimeout(() => {
saveDraft(cid, values as Record<string, unknown>);
setDraftSavedAt(new Date().toISOString());
}, 1500);
});
return () => {
sub.unsubscribe();
if (saveTimer.current) clearTimeout(saveTimer.current);
};
}, [formValues, isDirty, load.kind, cid]);
}, [load.kind, cid, watch]);
// ── Section ordering ───────────────────────────────────────────────────
const sectionsToRender = useMemo(
() =>
config.sections.map((s) => ({
section: s,
sectionVisible: evaluate(s.visibleWhen, formValues),
sectionVisible: evaluate(s.visibleWhen, evalState),
})),
[config.sections, formValues],
[config.sections, evalState],
);
const currentStageValue = formValues[config.stageField] as string | undefined;
const currentRank = currentStageValue ? STAGE_RANK[currentStageValue] ?? 0 : 0;
// Track which sections the user has manually opened/closed so we can
// programmatically re-open them when validation surfaces an error within.
// Used by the onInvalid handler below.
const expandSection = useCallback((sectionId: string) => {
document.dispatchEvent(
new CustomEvent("coop-checkin:expand-section", { detail: { sectionId } }),
);
}, []);
// ── Render ─────────────────────────────────────────────────────────────
if (load.kind === "loading") return <LoadingState />;
if (load.kind === "error") return <ErrorState message={load.message} />;
@@ -183,8 +230,35 @@ export function EngagementForm({ config, cid, cs }: EngagementFormProps) {
}
};
// ── Invalid-submit handling ────────────────────────────────────────────
// When required-field validation blocks submit, RHF calls onInvalid with
// the errors map. Find the first error's section, expand it, scroll into
// view, focus the field. Saves users from chasing a silent failure.
const onInvalid = (errs: FieldErrors<FieldValues>) => {
const firstErrorField = Object.keys(errs)[0];
if (!firstErrorField) return;
const section = sectionForField(config.sections, firstErrorField);
if (section) {
expandSection(section.id);
// Allow the section to expand before focusing.
requestAnimationFrame(() => {
try {
setFocus(firstErrorField);
} catch {
// setFocus throws if the field isn't registered; safe to ignore.
}
const el = document.getElementById(`field-${firstErrorField}`);
el?.scrollIntoView({ behavior: "smooth", block: "center" });
});
}
setSubmitState({
kind: "error",
message: "Please fill the required fields highlighted below before submitting.",
});
};
return (
<form onSubmit={handleSubmit(onSubmit)} className="space-y-5" noValidate>
<form onSubmit={handleSubmit(onSubmit, onInvalid)} className="space-y-5" noValidate>
<SubmissionContextHeader
orgName={load.data.orgName}
currentStage={load.data.currentStage}
@@ -210,7 +284,7 @@ export function EngagementForm({ config, cid, cs }: EngagementFormProps) {
section={section}
register={register}
errors={errors}
formValues={formValues}
formValues={evalState}
isCurrent={section.rank === currentRank}
defaultOpen={section.rank === currentRank || section.rank === 0}
options={load.data.options ?? {}}

View File

@@ -14,10 +14,11 @@ interface MatrixGroupProps {
* - Row labels live in the leftmost column (sticky on horizontal scroll).
* - Column headers carry the period labels (M1..M12, Q1..Q4, …).
* - Each row's `type` controls the cell input rendering: currency cells get
* a $ prefix on the row label rather than per-cell to save space; percent
* a $ suffix on the row label rather than per-cell to save space; percent
* cells append %.
* - On narrow viewports the table scrolls horizontally; the metric column
* stays pinned via position:sticky.
* stays pinned via position:sticky with a paper background and a hairline
* ink rule on its right edge so the boundary reads cleanly during scroll.
*
* Cells are still individual react-hook-form fields, registered by
* `field.fields[colIndex]`. The matrix only changes layout — submission and
@@ -27,14 +28,17 @@ export function MatrixGroup({ group, register }: MatrixGroupProps) {
return (
<section
aria-labelledby={`${group.id}-heading`}
className="rounded-xl border border-stone-200 bg-white p-4 shadow-sm sm:p-5"
className="rounded-lg border border-rule bg-paper p-4 shadow-sm sm:p-5"
>
<header className="mb-3">
<h3 id={`${group.id}-heading`} className="text-base font-semibold text-stone-900">
<h3
id={`${group.id}-heading`}
className="font-display text-lg font-medium leading-tight tracking-tight text-ink"
>
{group.label}
</h3>
{group.intro && (
<p className="mt-1 text-sm text-stone-600 leading-relaxed">{group.intro}</p>
<p className="mt-1 text-sm text-ink-soft leading-relaxed">{group.intro}</p>
)}
</header>
@@ -44,7 +48,7 @@ export function MatrixGroup({ group, register }: MatrixGroupProps) {
<tr>
<th
scope="col"
className="sticky left-0 z-10 bg-stone-50 border-b border-stone-200 px-3 py-2 text-left font-medium text-stone-700"
className="sticky left-0 z-10 bg-paper-2 border-b border-rule px-3 py-2 text-left text-xs font-medium uppercase tracking-wide text-ink-mute shadow-[1px_0_0_0_var(--color-rule)]"
>
Metric
</th>
@@ -52,7 +56,7 @@ export function MatrixGroup({ group, register }: MatrixGroupProps) {
<th
key={c.key}
scope="col"
className="border-b border-stone-200 bg-stone-50 px-2 py-2 text-center font-medium text-stone-700 min-w-[5.5rem]"
className="border-b border-rule bg-paper-2 px-2 py-2 text-center text-xs font-medium uppercase tracking-wide text-ink-mute min-w-[5.5rem] tabular-nums"
>
{c.label}
</th>
@@ -60,29 +64,38 @@ export function MatrixGroup({ group, register }: MatrixGroupProps) {
</tr>
</thead>
<tbody>
{group.rows.map((row, rowIdx) => (
<tr key={rowIdx} className={rowIdx % 2 === 0 ? "bg-white" : "bg-stone-50/40"}>
<th
scope="row"
className="sticky left-0 z-10 border-b border-stone-100 bg-inherit px-3 py-1.5 text-left font-medium text-stone-800 whitespace-nowrap"
>
{decoratedRowLabel(row.label, row.type)}
</th>
{row.fields.map((fieldName, colIdx) => (
<td
key={`${rowIdx}-${colIdx}`}
className="border-b border-stone-100 px-1 py-1"
{group.rows.map((row, rowIdx) => {
const zebra = rowIdx % 2 === 1;
return (
<tr key={rowIdx}>
<th
scope="row"
className={
"sticky left-0 z-10 border-b border-rule-soft px-3 py-1.5 text-left text-sm font-medium text-ink whitespace-nowrap shadow-[1px_0_0_0_var(--color-rule)] " +
(zebra ? "bg-paper-2/50" : "bg-paper")
}
>
<CellInput
fieldName={fieldName}
type={row.type}
label={`${row.label} ${group.cols[colIdx]?.label ?? ""}`.trim()}
register={register}
/>
</td>
))}
</tr>
))}
{decoratedRowLabel(row.label, row.type)}
</th>
{row.fields.map((fieldName, colIdx) => (
<td
key={`${rowIdx}-${colIdx}`}
className={
"border-b border-rule-soft px-1 py-1 " +
(zebra ? "bg-paper-2/50" : "bg-paper")
}
>
<CellInput
fieldName={fieldName}
type={row.type}
label={`${row.label} ${group.cols[colIdx]?.label ?? ""}`.trim()}
register={register}
/>
</td>
))}
</tr>
);
})}
</tbody>
</table>
</div>
@@ -111,7 +124,7 @@ interface CellInputProps {
function CellInput({ fieldName, type, label, register }: CellInputProps) {
const isNumeric = type === "currency" || type === "percent" || type === "number";
const baseClass =
"w-full rounded border border-stone-200 bg-white px-2 py-1 text-right text-stone-900 " +
"w-full rounded border border-rule-soft bg-paper px-2 py-1 text-right text-ink " +
"tabular-nums shadow-sm transition " +
"focus:border-leaf-500 focus:outline-none focus:ring-2 focus:ring-leaf-500/30";

View File

@@ -1,6 +1,6 @@
"use client";
import { useId, useMemo, useState } from "react";
import { useEffect, useId, useMemo, useState } from "react";
import type { StageSectionConfig, SelectOption } from "@/types/form";
import type { UseFormRegister, FieldValues, FieldErrors } from "react-hook-form";
import { FieldRenderer } from "./fields/FieldRenderer";
@@ -20,8 +20,6 @@ interface StageSectionProps {
defaultOpen: boolean;
/** Option groups fetched from CiviCRM, keyed by option_group_id. */
options: Record<number, SelectOption[]>;
/** Optional last-checked-in timestamp for this stage (ISO date). */
lastTouched?: string;
}
/**
@@ -43,12 +41,22 @@ export function StageSection({
isCurrent,
defaultOpen,
options,
lastTouched,
}: StageSectionProps) {
const [open, setOpen] = useState(defaultOpen);
const headingId = useId();
const panelId = useId();
// Listen for programmatic expand requests (fired by EngagementForm when a
// submit fails on a required field inside a collapsed section).
useEffect(() => {
const handler = (e: Event) => {
const detail = (e as CustomEvent<{ sectionId: string }>).detail;
if (detail?.sectionId === section.id) setOpen(true);
};
document.addEventListener("coop-checkin:expand-section", handler);
return () => document.removeEventListener("coop-checkin:expand-section", handler);
}, [section.id]);
const matrixFieldNames = useMemo(() => {
const names = new Set<string>();
for (const m of section.matrixGroups ?? []) {
@@ -104,12 +112,6 @@ export function StageSection({
<span className="tabular-nums">
{fieldCount} {fieldCount === 1 ? "field" : "fields"}
</span>
{lastTouched && (
<>
<span aria-hidden>·</span>
<span>Last touched {formatRelative(lastTouched)}</span>
</>
)}
</span>
</span>
<Chevron open={open} />
@@ -210,13 +212,3 @@ function Chevron({ open }: { open: boolean }) {
);
}
function formatRelative(iso: string): string {
const then = new Date(iso).getTime();
if (Number.isNaN(then)) return iso;
const days = Math.round((Date.now() - then) / (1000 * 60 * 60 * 24));
if (days <= 0) return "today";
if (days === 1) return "yesterday";
if (days < 30) return `${days} days ago`;
if (days < 365) return `${Math.round(days / 30)} months ago`;
return `${Math.round(days / 365)} years ago`;
}

View File

@@ -25,6 +25,10 @@ interface FieldRendererProps {
* Currency, percent, and number all use input type="number" with appropriate
* `step` and inputMode for mobile keyboards. We keep formatting light — the
* server is the source of truth for normalization.
*
* Required-field validation: when `field.required` is true, RHF's register
* receives a string error message so the inline ErrorText has something to
* display. Without that, validation would block submit silently.
*/
export function FieldRenderer({
field,
@@ -39,6 +43,7 @@ export function FieldRenderer({
const describedBy = [helpId, errorId].filter(Boolean).join(" ") || undefined;
const errorMsg = errors[field.name]?.message as string | undefined;
const effectiveOptions = resolvedOptions ?? field.options ?? [];
const requiredOpt = field.required ? `${field.label} is required.` : false;
const baseInputClass =
"w-full rounded-md border border-rule bg-paper px-3 py-2 text-ink " +
@@ -49,7 +54,6 @@ export function FieldRenderer({
// ── Readonly display field ──────────────────────────────────────────────
if (field.type === "readonly") {
// Look up the human label if this readonly references an option-group field.
const opt = effectiveOptions.find((o) => o.value === readonlyValue);
const display =
readonlyValue == null || readonlyValue === ""
@@ -62,7 +66,7 @@ export function FieldRenderer({
id={id}
role="textbox"
aria-readonly="true"
className="rounded-md border border-stone-200 bg-stone-50 px-3 py-2 text-stone-700"
className="rounded-md border border-rule bg-paper-2/40 px-3 py-2 text-ink-soft"
>
{display}
</div>
@@ -80,11 +84,11 @@ export function FieldRenderer({
type="checkbox"
aria-describedby={describedBy}
aria-invalid={errorMsg ? true : undefined}
{...register(field.name)}
className="mt-0.5 h-4 w-4 rounded border-stone-400 text-leaf-700 focus:ring-2 focus:ring-leaf-500/40"
{...register(field.name, { required: requiredOpt })}
className="mt-0.5 h-4 w-4 rounded border-rule text-leaf-700 focus:ring-2 focus:ring-leaf-500/40"
/>
<div className="flex-1">
<label htmlFor={id} className="font-medium text-stone-800 cursor-pointer">
<label htmlFor={id} className="font-medium text-ink cursor-pointer">
{field.label}
{field.required && <RequiredMark />}
</label>
@@ -108,7 +112,7 @@ export function FieldRenderer({
placeholder={field.placeholder}
maxLength={field.maxLength}
rows={4}
{...register(field.name, { required: field.required })}
{...register(field.name, { required: requiredOpt })}
className={baseInputClass + " min-h-[7rem] leading-6"}
/>
{field.help && <Help id={helpId!}>{field.help}</Help>}
@@ -127,8 +131,8 @@ export function FieldRenderer({
aria-describedby={describedBy}
aria-invalid={errorMsg ? true : undefined}
aria-required={field.required || undefined}
{...register(field.name, { required: field.required })}
className={baseInputClass + " bg-white"}
{...register(field.name, { required: requiredOpt })}
className={baseInputClass}
defaultValue=""
>
<option value="" disabled>
@@ -154,7 +158,7 @@ export function FieldRenderer({
aria-invalid={errorMsg ? true : undefined}
className="space-y-2"
>
<legend className="block text-sm font-medium text-stone-800">
<legend className="block text-sm font-medium text-ink">
{field.label}
{field.required && <RequiredMark />}
</legend>
@@ -165,16 +169,16 @@ export function FieldRenderer({
<label
key={o.value}
htmlFor={optId}
className="flex items-start gap-2 rounded border border-stone-200 px-3 py-2 hover:bg-stone-50 cursor-pointer"
className="flex items-start gap-2 rounded border border-rule bg-paper px-3 py-2 hover:bg-paper-2/60 cursor-pointer transition-colors"
>
<input
id={optId}
type="checkbox"
value={o.value}
{...register(field.name)}
className="mt-0.5 h-4 w-4 rounded border-stone-400 text-leaf-700 focus:ring-2 focus:ring-leaf-500/40"
className="mt-0.5 h-4 w-4 rounded border-rule text-leaf-700 focus:ring-2 focus:ring-leaf-500/40"
/>
<span className="text-sm text-stone-800">{o.label}</span>
<span className="text-sm text-ink-soft">{o.label}</span>
</label>
);
})}
@@ -187,17 +191,24 @@ export function FieldRenderer({
// ── File ────────────────────────────────────────────────────────────────
if (field.type === "file") {
const priorName = typeof readonlyValue === "string" ? readonlyValue : "";
const hasPrior = priorName.length > 0;
return (
<div className="space-y-1">
<Label id={id} field={field} />
{hasPrior && (
<p className="text-xs text-ink-soft" aria-live="polite">
Currently on file: <span className="font-medium text-ink">{priorName}</span>
</p>
)}
<input
id={id}
type="file"
aria-describedby={describedBy}
aria-invalid={errorMsg ? true : undefined}
aria-required={field.required || undefined}
{...register(field.name, { required: field.required })}
className="block w-full text-sm text-stone-700 file:mr-3 file:rounded-md file:border-0 file:bg-leaf-100 file:px-3 file:py-2 file:text-leaf-800 file:text-sm file:font-medium hover:file:bg-leaf-200 cursor-pointer"
{...register(field.name, { required: requiredOpt })}
className="block w-full text-sm text-ink-soft file:mr-3 file:rounded-md file:border-0 file:bg-leaf-100 file:px-3 file:py-2 file:text-leaf-800 file:text-sm file:font-medium hover:file:bg-leaf-200 cursor-pointer transition"
/>
{field.help && <Help id={helpId!}>{field.help}</Help>}
{errorMsg && <ErrorText id={errorId!}>{errorMsg}</ErrorText>}
@@ -216,7 +227,7 @@ export function FieldRenderer({
{prefix && (
<span
aria-hidden
className="pointer-events-none absolute left-3 top-1/2 -translate-y-1/2 text-stone-500"
className="pointer-events-none absolute left-3 top-1/2 -translate-y-1/2 text-ink-mute"
>
{prefix}
</span>
@@ -233,11 +244,12 @@ export function FieldRenderer({
min={field.min}
max={field.max}
{...register(field.name, {
required: field.required,
required: requiredOpt,
valueAsNumber: true,
})}
className={
baseInputClass +
" tabular-nums" +
(prefix ? " pl-7" : "") +
(suffix ? " pr-8" : "")
}
@@ -245,7 +257,7 @@ export function FieldRenderer({
{suffix && (
<span
aria-hidden
className="pointer-events-none absolute right-3 top-1/2 -translate-y-1/2 text-stone-500"
className="pointer-events-none absolute right-3 top-1/2 -translate-y-1/2 text-ink-mute"
>
{suffix}
</span>
@@ -280,7 +292,7 @@ export function FieldRenderer({
field.type === "phone" ? "tel" :
undefined
}
{...register(field.name, { required: field.required })}
{...register(field.name, { required: requiredOpt })}
className={baseInputClass}
/>
{field.help && <Help id={helpId!}>{field.help}</Help>}
@@ -291,7 +303,7 @@ export function FieldRenderer({
function Label({ id, field }: { id: string; field: FieldConfig }) {
return (
<label htmlFor={id} className="block text-sm font-medium text-stone-800">
<label htmlFor={id} className="block text-sm font-medium text-ink">
{field.label}
{field.required && <RequiredMark />}
</label>
@@ -300,7 +312,7 @@ function Label({ id, field }: { id: string; field: FieldConfig }) {
function RequiredMark() {
return (
<span aria-label="required" className="ml-1 text-red-600">
<span aria-label="required" className="ml-1 text-clay-700">
*
</span>
);
@@ -308,7 +320,7 @@ function RequiredMark() {
function Help({ id, children }: { id: string; children: React.ReactNode }) {
return (
<p id={id} className="text-xs text-stone-600">
<p id={id} className="text-xs text-ink-mute leading-relaxed">
{children}
</p>
);
@@ -316,7 +328,7 @@ function Help({ id, children }: { id: string; children: React.ReactNode }) {
function ErrorText({ id, children }: { id: string; children: React.ReactNode }) {
return (
<p id={id} role="alert" className="text-xs font-medium text-red-700">
<p id={id} role="alert" className="text-xs font-medium text-clay-700">
{children}
</p>
);