mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-08 13:58:59 +00:00
c9f932be65
Closes the Phase 5 batch from cowork/frontend-design-audit.html: ships
the joint UX-H4 + FE-M1 lever (FormField primitive + react-hook-form +
zod schemas) and the FE-H3 fix (Headless UI Dialog focus trap on the 3
inline-managed modals), with an axe-core regression test + CI guard to
prevent UX-H4 regressions.
═════════════════════════ AUDIT VERIFICATION ═════════════════════════
Confirmed live against the repo before implementing:
• Q1 labels / htmlFor / input-id = 139 / 6 / 0
(audit said 138 / 6 / 0 — labels +1, otherwise accurate)
• Q2 no form library installed
(no react-hook-form, formik, @tanstack/react-form, final-form)
• Q3 3 inline-managed dialog sites confirmed:
SCEPAdminPage.tsx:272, AgentsPage.tsx:314, ESTAdminPage.tsx:281
• Q4 audit's top-6 list was OFF — actual top form-heaviest pages
by useState count are: OIDCProviderDetailPage 21, AgentGroupsPage
18, CertificatesPage 17, CertificateDetailPage 14, BreakglassPage
13, ProfilesPage 13 — NOT the audit-suggested OnboardingWizard 5
(now split in Phase 4) / OIDCProvidersPage 8 / IssuersPage 11 /
ProfilesPage 13 / TargetsPage 9 / ApprovalsPage 5. Audit's
intuition skipped the higher-useState pages.
• Q5 jest-dom imported in src/test/setup.ts — axe-core landed
cleanly
═════════════════════════════ CLOSURES ═══════════════════════════════
UX-H4 (label/input binding) — FormField primitive shipped
• web/src/components/FormField.tsx wraps a <label> + an input child
and auto-generates a stable id via React 18's useId(); cloneElement
threads that id onto BOTH the <label htmlFor> AND the child's id
prop so the WCAG 1.3.1 binding holds by construction. Supports
`required` (asterisk + aria-required), `description` (wires
aria-describedby), `error` (aria-invalid + role=alert + extends
aria-describedby). 7 tests pin the contract.
FE-M1 (no form library) — react-hook-form + @hookform/resolvers + zod
• Added react-hook-form 7.75, @hookform/resolvers 5.2, zod 4.4 as
runtime deps; @axe-core/react, jest-axe, @types/jest-axe as devDeps
• Representative migration of CreateTeamModalInline (inside
onboarding/CertificateStep — operator's first-run experience)
from 3-useState + manual handlers to useForm + zodResolver +
FormField. Schema at pages/onboarding/team.schema.ts.
• Per the audit's "top-6 only, primitive is the lever" rule, the
other 5 audit-suggested pages migrate organically as feature
work touches them — documented as Phase 5 follow-up. The
FormField primitive is the leverage point; per-page migrations
are mechanical applications.
FE-H3 (no focus trap on modal pages)
• New ModalDialog primitive at web/src/components/ModalDialog.tsx —
Headless UI Dialog wrapper for arbitrary-content modals
(complements ConfirmDialog which is confirm-only). Auto-emits
role=dialog + aria-modal + aria-labelledby + ESC-to-close +
backdrop-click-to-close + focus trap.
• All 3 inline-managed modal sites migrated:
• SCEPAdminPage ConfirmReloadModal
• ESTAdminPage ConfirmReloadModal (data-testid preserved)
• AgentsPage RetireAgentModal (3-mode: confirm / blocked / error
— title + footer change per mode; body slot stays the same)
• 37/37 existing modal-page tests stay green — no behavior change
visible to the test suite, only the focus-trap + ESC handling.
UX-H4 regression gate
• web/src/test/a11y.test.tsx runs axe-core (not jest-axe — its
`toHaveNoViolations` matcher uses jest's expect API which can't
plug into Vitest's expect.extend; fails with "expectAssertion.call
is not a function"). Direct axe.run + assert violations.length===0
gives the same gate with a readable failure message.
• Scope: primitives, not page sweeps. Primitives carry the risk
surface; pages compose them. 5 tests covering FormField (with +
without description/error), Skeleton (all 4 variants),
ModalDialog, Breadcrumbs. ~400ms total.
• Skeleton.table's empty <th> cells are decorative shimmers inside
a role=status + aria-busy=true tree — axe-core's
`empty-table-header` rule doesn't model aria-busy gating, so it
is suppressed for the Skeleton variant scan with a clear comment.
• scripts/ci-guards/no-unbound-label.sh — fails CI if a new <label>
without htmlFor lands. Baseline-driven (132 today) so the existing
backlog doesn't block CI; every migration to FormField drops the
baseline. `--strict` mode rejects any unbound label once the
backlog clears.
═══════════════════════════ VERIFICATION ═════════════════════════════
• npx tsc --noEmit — exits 0
• New tests: FormField 7/7, ModalDialog 6/6, a11y 5/5 = 18/18 new
• Component suite: 14 files / 150/150 green
• Page suite (representative subset run): 16 files in first run
(timeout truncated final summary) + 10 files / 48/48 in second
run — all green
• OnboardingWizard 4/4 (the migrated CreateTeamModalInline test
case is the second one — `+ New team opens the inline modal,
calls createTeam, invalidates the cache, and auto-selects the
new team`)
• SCEPAdminPage 20/20, ESTAdminPage 14/14, AgentsPage 3/3 — all
37 modal-page tests stay green after ModalDialog migration
• npm run build ✓ in 3.27s
• CI guard: bash scripts/ci-guards/no-unbound-label.sh — passes at
baseline 132 (current unbound count matches; failure mode is
only on increase). --strict path will fail until backlog clears.
═══════════════════════════ RESIDUAL RISK ════════════════════════════
• RHF migration risk: zod resolver's input/output type mismatch
bit me once during this work (description: z.string().optional()
gave Input: string|undefined vs Output: string after .default()).
Both sides typed as string + defaultValues providing empty string
fixes it; documented in team.schema.ts. Pattern applies to every
future Zod schema with optional-but-empty-string fields.
• The audit's "top-6" page list is stale (Phase 4 split
OnboardingWizard; useState ranks shifted). Future RHF migrations
should re-derive the priority list against live useState counts,
not the audit's stamped names.
• DataTable per-row React.memo (PERF-M1 follow-up from Phase 4)
remains deferred — orthogonal to Phase 5 scope.
119 lines
4.3 KiB
TypeScript
119 lines
4.3 KiB
TypeScript
// Copyright 2026 certctl LLC. All rights reserved.
|
|
// SPDX-License-Identifier: BUSL-1.1
|
|
//
|
|
// FormField — Phase 5 closure for UX-H4 + the foundation of FE-M1.
|
|
//
|
|
// Pre-Phase-5 state: 139 <label> elements in production tsx; 6 with
|
|
// htmlFor; 0 inputs with id. WCAG 1.3.1 (info-and-relationships) fails
|
|
// on ~99% of form fields — screen readers can't programmatically pair
|
|
// a label with its input, so "Email" reads as a floating string rather
|
|
// than as the accessible name of the adjacent input.
|
|
//
|
|
// FormField fixes this by generating a stable id with React 18's
|
|
// useId() and threading it to BOTH the <label htmlFor=...> AND the
|
|
// child input's id prop via cloneElement. Consumers write:
|
|
//
|
|
// <FormField label="Email" required>
|
|
// <input type="email" value={email} onChange={…} />
|
|
// </FormField>
|
|
//
|
|
// — no manual id wiring, no risk of id-mismatch drift, no chance a
|
|
// developer copies the JSX and forgets to update one of the two
|
|
// strings. The label-↔-input binding is correct by construction.
|
|
//
|
|
// Composition with react-hook-form is straight-forward — RHF's
|
|
// register('field') returns onChange/onBlur/ref/name which spread onto
|
|
// the input alongside FormField's auto-id. The Zod-resolver path picks
|
|
// up errors and FormField surfaces them via the `error` prop slot.
|
|
|
|
import { Children, cloneElement, isValidElement, useId } from 'react';
|
|
import type { ReactElement, ReactNode } from 'react';
|
|
|
|
interface FormFieldProps {
|
|
/** Visible label text. Required for a11y — never render an unbound input. */
|
|
label: string;
|
|
/** Render `*` next to the label when true (display-only; validation lives in Zod). */
|
|
required?: boolean;
|
|
/** Optional helper / description text below the input. */
|
|
description?: string;
|
|
/** Optional error message — when set, surfaces below the input + flags aria-invalid. */
|
|
error?: string;
|
|
/** Optional class override for the wrapping div. */
|
|
className?: string;
|
|
/**
|
|
* Exactly one input-shaped child (<input>, <select>, <textarea>, or any
|
|
* forwardRef'd component that accepts `id` + `aria-describedby` +
|
|
* `aria-invalid` as props). FormField clones it and injects the
|
|
* auto-generated id so the label-↔-input pairing is correct by
|
|
* construction.
|
|
*/
|
|
children: ReactNode;
|
|
}
|
|
|
|
export default function FormField({
|
|
label,
|
|
required,
|
|
description,
|
|
error,
|
|
className,
|
|
children,
|
|
}: FormFieldProps) {
|
|
// useId() returns a stable id that's unique per render-tree-position,
|
|
// safe under StrictMode, and SSR-friendly. Two siblings get different
|
|
// ids automatically.
|
|
const reactId = useId();
|
|
const inputId = `field-${reactId}`;
|
|
const descId = description ? `desc-${reactId}` : undefined;
|
|
const errorId = error ? `err-${reactId}` : undefined;
|
|
|
|
// Build the aria-describedby chain from optional description + error.
|
|
// Browsers concatenate space-separated ids, so screen readers announce
|
|
// "Email, [description], [error]".
|
|
const describedBy = [descId, errorId].filter(Boolean).join(' ') || undefined;
|
|
|
|
const onlyChild = Children.only(children);
|
|
if (!isValidElement(onlyChild)) {
|
|
// Surface a clear error in dev rather than render a broken control.
|
|
throw new Error('FormField expects exactly one valid React element child');
|
|
}
|
|
|
|
// cloneElement preserves the child's existing props (including any
|
|
// RHF `register(...)` spread) and overlays the FormField-managed
|
|
// a11y props on top. The child's `id` / `aria-*` are always set
|
|
// here, but `name`/`value`/`onChange` from the child are preserved.
|
|
const childWithA11y = cloneElement(
|
|
onlyChild as ReactElement<Record<string, unknown>>,
|
|
{
|
|
id: inputId,
|
|
'aria-describedby': describedBy,
|
|
'aria-invalid': error ? true : undefined,
|
|
'aria-required': required ? true : undefined,
|
|
},
|
|
);
|
|
|
|
return (
|
|
<div className={className ?? 'mb-4'}>
|
|
<label
|
|
htmlFor={inputId}
|
|
className="block text-sm font-medium text-ink mb-1.5"
|
|
>
|
|
{label}
|
|
{required && (
|
|
<span className="text-red-600 ml-0.5" aria-hidden="true">*</span>
|
|
)}
|
|
</label>
|
|
{childWithA11y}
|
|
{description && (
|
|
<p id={descId} className="mt-1 text-xs text-ink-muted">
|
|
{description}
|
|
</p>
|
|
)}
|
|
{error && (
|
|
<p id={errorId} role="alert" className="mt-1 text-xs text-red-700">
|
|
{error}
|
|
</p>
|
|
)}
|
|
</div>
|
|
);
|
|
}
|