feat(form): scroll to first invalid field on validation error

[Confmc]

What

useForm now scrolls the first invalid field ([aria-invalid="true"]) into view after a 422
populates the error bag, so the user lands on the first thing to fix. Two optional knobs:
scrollToError: false turns it off, and scrollRoot scopes it to one form on a multi-form page.

Why

useForm surfaces validation errors but leaves the viewport wherever the user submitted — on a long
form the first error is often off-screen. Consumer form owners re-implement scroll-to-first-error by
hand today (watch errors -> nextTick -> scrollIntoView) at every form. Moving it into useForm
gives every consumer the behaviour once and lets those hand-rolled watchers be deleted.

Design

• Lives in useForm, not useValidationErrors. The primitive stays pure and DOM-free (unchanged).
  Scrolling is opinionated presentation behaviour, so it sits in the composite that already owns
  submitting. A zero-DOM consumer uses useValidationErrors directly, or passes scrollToError: false.
  The wiring helper is internal (not exported).
• Keys off aria-invalid, derives nothing. It targets [aria-invalid="true"], which the
  presentation layer already sets from the error bag. useForm reads that attribute — it computes no
  ids and marks no fields itself, so it stays agnostic about how fields render.
• flush: 'post' runs the scroll after the DOM update that paints the mark. The watcher is
  registered in setup() (via useForm), so it stops on unmount.
• Default-on, opt-out — the expected behaviour for a form helper (cf. react-hook-form's
  shouldFocusError); mirrors fs-dialog's closeOnBackdropClick (default true, opt out false).
• scrollRoot (optional) scopes the query to one form's subtree, so on a page with several forms a
  422 in one never scrolls to another's field. Omit it and the query is document-wide (back-compat, and
  right for a single form). A passed-but-null ref is a no-op — it never silently falls back to a
  document-wide search, so opted-in scoping is never re-widened.

API

UseFormOptions gains two optional fields; useValidationErrors / useFormSubmit are unchanged.

useForm(httpService, {
    scrollToError?: boolean,             // default true
    scrollRoot?: Ref<HTMLElement | null>, // optional; scopes the scroll to one form
})

Tests

Six cases in form.spec.ts, in the existing happy-dom + mock-service style: scrolls on a 422, opt-out
false, no re-scroll on clearErrors, no-op when nothing is marked invalid, scoped to scrollRoot,
ignores an invalid field outside scrollRoot, and no document fallback when scrollRoot is null.
tsc, oxlint, oxfmt clean; scroll-to-first-error.ts and form.ts are at 100% coverage AND 100%
mutation (package 94.83%, above the 90% threshold).

Version

Bumped fs-form 0.1.1 -> 0.2.0 (minor = new option, per docs/contributing.md; version bumps are
author-managed here). fs-form has no dependent packages, so no peer-range cascade.

Possible follow-ups (not in this PR)

• prefers-reduced-motion — the scroll is behavior: 'smooth'; a reduced-motion-aware behaviour is
  a reasonable a11y follow-up.
• Focus — the scroll does not move focus (matches consumers' current behaviour); focusing the first
  invalid field is a further a11y step.

🤖 Generated with Claude Code

[jasperboerhof]

Crit review

2 issues · 0 nitpicks · head 4a667f77bb

Crit requests changes — 2 issues.

Issues

Shared HttpService responses populate every useForm error bag and trigger unrelated scrolls
packages/form/src/form.ts:32 — see inline

Document-wide scrolling selects another form's invalid field before the submitting form
packages/form/src/scroll-to-first-error.ts:20 — see inline

[Goosterhof]

General's review (war room, decorrelated from crit — crit holds the bus lock on 3081 as I write; I have not read its verdict).

The mechanism is sound: errors is only ever replaced in useValidationErrors, so the watch fires exactly on 422-populate and on clear, flush: 'post' sits after the child renders that paint the mark, and the scoped/null-root semantics are the right call. The tests discriminate (document order, scope, null-root). What I am asking to change is the claim the package makes about its consumers, and one a11y regression. Both are small.

I checked the five fleet consumers of fs-form against the precondition this PR relies on ("the presentation layer already sets aria-invalid"):

Consumer	useForm sites	Renders aria-invalid="true" from the bag?	Effect of 0.2.0
brick-inventory-orchestrator	9	yes — ui-inputs :invalid (56 sites)	works
isms	1	yes — ui-inputs :invalid	works
town-crier	3	yes — inline :aria-invalid	works
emmie	3 (incl. every createFormModal)	no — marks .form-error by id, and already scrolls in FieldError.vue (EMMIE-0525)	silent no-op; double-scroll the day it adopts aria-invalid
ublgenie	15	no — editorial Input/Field set no aria-invalid	silent no-op

So the PR's Why ("consumer form owners re-implement scroll-to-first-error by hand today … lets those hand-rolled watchers be deleted") is true of exactly one implementation in the fleet — emmie's, the use case this came from — and that one keys on a marker this PR does not look for. As written, the originating territory cannot delete its watcher by bumping. Details inline.

Blocking (2): (1) make the claim true — either a scrollTarget selector option (default [aria-invalid="true"]) so emmie passes .form-error today, or at minimum state the precondition in the docs and drop the "delete your watchers" line; (2) honour prefers-reduced-motion now, not as a follow-up — ui-inputs already zeroes its transitions under it, so 0.2.0 would be the first Armory surface to animate against the user's setting.

Non-blocking (3): focus-vs-scroll (the react-hook-form precedent cited focuses), the default-on cross-form hazard with a shared HttpService (dialog over page — emmie's exact shape), and two test-hygiene points.

Version: ^0.1.1 does not admit 0.2.0, so no consumer flips behaviour without an explicit bump — good; the fleet bump is a war-room wave after this lands.

[dmooibroek]

No findings above the severity gate — eyeball manually.

[Confmc]

Thanks both — addressed in the incoming commit. Summary of what changed and where we're holding the line.

Fixed

• Reduced motion (General, blocking): the scroll now falls back to behavior: 'auto' under prefers-reduced-motion: reduce. A JS scrollIntoView behavior isn't subject to the CSS media query, so it's checked explicitly rather than deferred.
• scrollTarget option (General, option 1): useForm(http, {scrollTarget: '.field-error'}) lets a consumer name its own error mark instead of the default [aria-invalid="true"]. Default unchanged. This covers consumers whose inputs mark with a class rather than aria-invalid.
• Docs / the "delete your watchers" claim: stated the aria-invalid precondition as a precondition; made scrollRoot required for a dialog over a page form on the same HttpService; corrected the line that said a co-mounted form scrolls to "its own" field (document order can pick another form's). Dropped the react-hook-form shouldFocusError precedent from the narrative — it focuses (which scrolls for free); this scrolls without focusing. Focus is a deliberate defer.
• Test across the boundary (General, test hygiene): added a case where a child paints aria-invalid from a prop, so the flush: 'post' claim is now actually proven across the component boundary (the fleet shape), not just within the useForm component.

Keeping the default [aria-invalid="true"] — one correction

The premise that "emmie renders no aria-invalid" is a grep artifact. emmie's converted forms mark via @script-development/ui-inputs' :invalid, and ui-inputs renders aria-invalid="true" inside its own components — so frontend/apps/** shows zero occurrences while the DOM has them (verified in the shipped ui-inputs dist: every control emits :aria-invalid="invalid || undefined"). So the default selector matches emmie's forms after migration. The FieldError double-scroll is real only until emmie deletes its own FieldError scroll on migration, which is the emmie-side change, not this package's concern.

Cross-form scroll (crit, both findings) — confirmed, scoped

Both are real and share one root: a 422 on a shared HttpService fills every co-mounted form's bag, and a document-wide query then scrolls to whichever field is first in document order. The bag-level bleed is the pre-existing one-scope-per-service contract in useValidationErrors (documented under Scoping & Backend Contract) — not introduced by this scroll change, and out of scope to redesign here. For the scroll itself, scrollRoot is the scope fix and is now documented as required for the dialog-over-page shape, with the misleading "own field" wording corrected.

Not done, on purpose

• No checkVisibility skip: it's a recent DOM API (Safari 17.4+/FF 125+) — non-defensive it can throw in an older browser of a care consumer, defensive it leaves a permanently-uncovered branch, and it doesn't fix the real cross-form case (page fields behind a backdrop aren't display:none). scrollRoot is the right tool there.
• No dev-only warning (noted as optional).

Gates on the new commit: typecheck, 100% coverage, lint, format, and Stryker mutation 100% on both changed source files (95%+ overall).

[dmooibroek]

No findings above the severity gate — eyeball manually.

[dmooibroek]

CI green at d37f8fb — approving.

[jasperboerhof]

Crit review

2 issues · 1 nitpick · head d37f8fb01f

Crit requests changes — 2 issues.

Issues

The scroll watcher throws when a DOM runtime lacks global matchMedia
packages/form/src/scroll-to-first-error.ts:36 — see inline

Clearing the error bag can scroll an unrelated invalid field
packages/form/src/scroll-to-first-error.ts:29 — see inline

1 nitpick

Malformed 422 field values can leave consumers with incorrect or stale messages
packages/form/src/validation-errors.ts:15 — toFieldErrorMap casts every error entry to string[] without validation. mapFieldErrors reads messages[0] before assigning the new bag. A string stores its first character. A null entry throws, leaving consumers with the previous validation message.

nitpick because pre-existing — this pull request did not write those lines

---

Settled, not re-filed: 1

[jasperboerhof]

Crit review

2 issues · 0 nitpicks · head 014f8eef1a

Crit requests changes — 2 issues.

Issues

Default scrolling throws when DOM-free useForm consumers populate validation errors
packages/form/src/scroll-to-first-error.ts:34 — see inline

Late-bound scrollRoot values never trigger scrolling for existing validation errors
packages/form/src/scroll-to-first-error.ts:34 — see inline

[dmooibroek]

No findings above the severity gate — eyeball manually.

[crit-ai]

Crit review

0 issues · 1 nitpick · head 014f8eef1a

Crit approves — nothing blocking at this head.

1 nitpick

Malformed 422 field values bypass validation and can hide actionable form errors
packages/form/src/validation-errors.ts:15 — mapFieldErrors indexes each field value without confirming a string array. A string value stores only its first character. A null value throws before errors.value updates. guarded swallows that throw while handleSubmit swallows the 422.

nitpick because pre-existing — this pull request did not write those lines

---

Settled, not re-filed: 1

[Confmc]

Merging with the shared-bag limitation documented, not fixed, and the feature made opt-in.

The two crits from the first round are correct, and this PR does not close them. useValidationErrors registers its middleware on the HttpService, not on a request, so every 422 on that service fills every mounted form's bag. A watcher on the bag cannot tell whose refusal it is reacting to, and with no root given the query is document-wide.

Closing them properly means moving the trigger off the bag and onto the submit, so the scroll runs only for the action handleSubmit actually ran. We prototyped that and it is a substantially larger change: a new hook on useFormSubmit, scope taken from the caller, and a rewritten test matrix. It is not what this PR should grow into.

What changed instead is the default. scrollToError is now false, so the feature does nothing unless a consumer asks for it, and the guidance to pass scrollRoot alongside it is now the design rather than a caveat. A consumer that shares one HttpService across co-mounted forms therefore cannot be surprised by this: it opts in per form and says where to look. scrollTarget remains for consumers that do not mark errors the way ui-inputs does, reduced motion is honoured, matchMedia is guarded, and clearing the bag no longer scrolls.

[crit-ai]

Crit review

1 issue · 1 nitpick · head 55fb70e070

Crit requests changes — 1 issue.

Issues

Documented scroll examples omit scrollToError, leaving the watcher unregistered.
docs/packages/form.md:79 — see inline

1 nitpick

Malformed 422 field values produce incorrect messages or preserve stale errors.
packages/form/src/validation-errors.ts:15 — mapFieldErrors indexes every messages value without confirming that it is a string array. A string value yields its first character, while null throws inside guarded. guarded retains the prior error bag after that throw. Users can see an incorrect or stale validation message after a malformed 422 response.

nitpick because pre-existing — this pull request did not write those lines

stale

[crit-ai]

Crit review

0 issues · 1 nitpick · head 34e9a90b08

Crit requests changes — 1 open thread.

1 nitpick

Unchecked 422 field values truncate messages or leave the validation error bag empty.
packages/form/src/validation-errors.ts:15 — toFieldErrorMap casts each errors value to string[] without checking it. mapFieldErrors reads messages[0], turning a string value into its first character. guarded catches null-value failures, while useFormSubmit swallows the original 422. Non-Laravel callers can lose validation errors after a rejected submission.

nitpick because pre-existing — this pull request did not write those lines

Still open

docs/packages/form.md — Scroll examples omit the required opt-in, so copied forms never register the watcher.

---

Settled, not re-filed: 1

[crit-ai]

Crit review

1 issue · 1 nitpick · head 4b9bd428fb

Crit requests changes — 1 issue.

Issues

The primitive docs offer useForm-only scroll options to useValidationErrors
docs/packages/form.md:158 — not on the diff, so not inline

The primitive documentation gives useValidationErrors the same options as useForm. UseValidationErrorsOptions defines only keyMapper. useForm alone installs useScrollToFirstError. Consumers passing object literals receive excess-property errors or no scrolling.

1 nitpick

Validate 422 field messages before indexing their first element
packages/form/src/validation-errors.ts:15 — The cast treats each errors value as string[] without runtime validation. mapFieldErrors indexes strings as message arrays. mapFieldErrors throws for null values. Users receive truncated messages or no field feedback.

nitpick because pre-existing — this pull request did not write those lines

---

Settled, not re-filed: 2

[crit-ai]

Crit review

1 issue · 0 nitpicks · head f841a5b420

Crit requests changes — 1 issue.

Issues

The 0.2.0 fs-form release lacks a changeset entry for its new public options.
packages/form/package.json:3 — see inline

---

Settled, not re-filed: 3

refuted