mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-11 14:18:53 +00:00
feat(frontend): Phase 5 Accessibility + Forms — close FE-H3 + UX-H4 primitive + FE-M1 primitive + axe-core gate
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.
This commit is contained in:
@@ -0,0 +1,118 @@
|
||||
// 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>
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user