From 0d84b9654ba553453482fea03e8edd3c18ecb75d Mon Sep 17 00:00:00 2001 From: Joel Brock Date: Sat, 9 May 2026 22:49:25 -0700 Subject: [PATCH] Polish pass: removed lastTouched dead code, selective useWatch perf, token sweep, required-field messages, scroll-to-first-error, file prefill display, matrix sticky shadow --- components/EngagementForm.tsx | 108 +++++++++++++++++++++++----- components/MatrixGroup.tsx | 73 +++++++++++-------- components/StageSection.tsx | 32 ++++----- components/fields/FieldRenderer.tsx | 56 +++++++++------ 4 files changed, 180 insertions(+), 89 deletions(-) diff --git a/components/EngagementForm.tsx b/components/EngagementForm.tsx index 49396e8..d5bbcf9 100644 --- a/components/EngagementForm.tsx +++ b/components/EngagementForm.tsx @@ -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, + 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 | 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); + 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 ; if (load.kind === "error") return ; @@ -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) => { + 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 ( -
+
-

+

{group.label}

{group.intro && ( -

{group.intro}

+

{group.intro}

)}
@@ -44,7 +48,7 @@ export function MatrixGroup({ group, register }: MatrixGroupProps) { Metric @@ -52,7 +56,7 @@ export function MatrixGroup({ group, register }: MatrixGroupProps) { {c.label} @@ -60,29 +64,38 @@ export function MatrixGroup({ group, register }: MatrixGroupProps) { - {group.rows.map((row, rowIdx) => ( - - - {decoratedRowLabel(row.label, row.type)} - - {row.fields.map((fieldName, colIdx) => ( - { + const zebra = rowIdx % 2 === 1; + return ( + + - - - ))} - - ))} + {decoratedRowLabel(row.label, row.type)} + + {row.fields.map((fieldName, colIdx) => ( + + + + ))} + + ); + })} @@ -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"; diff --git a/components/StageSection.tsx b/components/StageSection.tsx index c356e04..24ac703 100644 --- a/components/StageSection.tsx +++ b/components/StageSection.tsx @@ -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; - /** 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(); for (const m of section.matrixGroups ?? []) { @@ -104,12 +112,6 @@ export function StageSection({ {fieldCount} {fieldCount === 1 ? "field" : "fields"} - {lastTouched && ( - <> - · - Last touched {formatRelative(lastTouched)} - - )} @@ -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`; -} diff --git a/components/fields/FieldRenderer.tsx b/components/fields/FieldRenderer.tsx index 128b869..3b3b9a1 100644 --- a/components/fields/FieldRenderer.tsx +++ b/components/fields/FieldRenderer.tsx @@ -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} @@ -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" />
-