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

[jasperboerhof]

Shared HttpService responses populate every useForm error bag and trigger unrelated scrolls

Each useValidationErrors middleware assigns its own errors ref for every 422 response. Every default-enabled watcher then scrolls, including a co-mounted form that did not submit. A 422 from form A can scroll form B while the user is correcting form A.

crit · finding 3cfd0ad041a2

[dmooibroek]

Confirmed. registerResponseErrorMiddleware is a global axios interceptor on the shared httpService instance, not scoped per request — every mounted useForm() sharing that service gets its errors ref rewritten on ANY 422, including one triggered by a sibling form's submit.

[jasperboerhof]

Document-wide scrolling selects another form's invalid field before the submitting form

When scrollRoot is omitted, useScrollToFirstError queries document for the first invalid element. That query ignores which useForm instance received the 422 response. An earlier invalid control in form B can scroll the user away from form A after A's 422.

crit · finding bd0c2be21ef3

[dmooibroek]

Confirmed, same root cause as the sibling thread on form.ts:32 — document-wide querySelector has no notion of which useForm submitted, so an earlier invalid control from an unrelated mounted form wins the scroll. scrollRoot fixes it but is opt-in; nothing here forces a multi-form page to pass it.

[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.

[Goosterhof]

Major — the selector is the whole contract, and two of five fleet consumers do not satisfy it. emmie renders no aria-invalid anywhere (frontend/apps/** has zero occurrences); its FieldError.vue toggles a .form-error class on the control by id and scrolls itself (EMMIE-0525 — your own implementation, scoped to closest('form')). ublgenie's editorial Input/Field set no aria-invalid either, across 15 useForm sites. On both, this line is a silent no-op after the bump, and on emmie it becomes a double scroll the day aria-invalid is added while FieldError still scrolls.

Two ways to make the PR's Why true, pick one:

1. scrollTarget?: string (default '[aria-invalid="true"]') — emmie passes '.form-error' and deletes the FieldError scroll today; keeps "derives nothing", the consumer names the marker. One line here, one in types.ts, one test.
2. Minimum: keep the selector fixed, but say in docs/packages/form.md that the feature is inert unless the consumer renders aria-invalid="true" from the bag, and drop the "lets those hand-rolled watchers be deleted" line from the body — it is not true for the territory this came from.

I would also take a dev-only warning (errors non-empty, no match in scope) — ublgenie's FormField.vue does exactly that for the property-without-errors case, and a feature that silently does nothing is the failure mode ADR-0048 exists to name. That one is optional.

[dmooibroek]

Checked scroll-to-first-error.ts: the selector is hardcoded to [aria-invalid="true"] with no override, so this is confirmed as a silent no-op for any consumer whose field components don't set that attribute from the error bag. Agree scrollTarget with a documented default is the cleaner fix over a docs-only caveat.

[Goosterhof]

Update from the Commander (2026-09-07), narrowing this Major. Marcel is leading the emmie migration and emmie will add the aria-invalid mark itself, retiring FieldError.vue's hand-rolled scroll in the same change. So option 1 (scrollTarget) is not needed on emmie's account — keep the fixed selector.

What remains of this finding is option 2, docs-only: state the precondition as a precondition (the feature is inert unless the consumer renders aria-invalid="true" from the bag) and drop "lets those hand-rolled watchers be deleted" from the body — ublgenie (15 useForm sites, editorial inputs set no aria-invalid) still gets a silent no-op on bump and will decide mark-vs-scrollToError:false in its own PR. With that wording change this thread is settled from my side.

The prefers-reduced-motion thread below stands unchanged — that one belongs in the package, not in a consumer.

[Goosterhof]

Major — behavior: 'smooth' ignores prefers-reduced-motion. A JS-specified behavior is not overridden by the user's OS setting in any engine; only CSS scroll-behavior respects the media query, and this call bypasses CSS. ui-inputs already zeroes every ui-* transition under @media (prefers-reduced-motion: reduce) (docs/packages/ui-inputs.md), so this would be the first Armory surface that animates against the setting — and emmie and isms are the care/compliance consumers. It is listed as a follow-up in the body; follow-up lists on package PRs are where a11y goes to die, and the fix is one expression:

const behavior = matchMedia('(prefers-reduced-motion: reduce)').matches ? 'auto' : 'smooth';

Alternative that is even more in the PR's own spirit of "opinionated about nothing": pass no behavior at all and let the consumer's CSS scroll-behavior (which does honour the media query) decide. Either is fine; shipping smooth unconditionally is not. The existing test asserting {behavior: 'smooth', block: 'center'} will need a matchMedia stub — happy-dom provides one.

[dmooibroek]

Confirmed in code: scrollIntoView is called unconditionally with behavior: 'smooth', no matchMedia(prefers-reduced-motion) check anywhere in scroll-to-first-error.ts. Agree this is a real a11y regression, not a fair follow-up deferral.

[Goosterhof]

Minor — default-on + document-wide + a shared HttpService is the modal-over-page shape, and that is emmie's exact shape. createFormModal in emmie's services/dialog.ts calls useForm(userHttpService) per dialog; the page beneath may hold its own useForm on the same service. A 422 from the dialog fills both bags; if the page form shares a field key, the page's control is first in document order and this scrolls the page behind the backdrop. The docs name the shared-service case, but frame it as "scrolls to its own matching field" — the failure is that the other form's field wins the querySelector race. scrollRoot fixes it, but only if every consumer knows to pass it; emmie's hand-rolled version scoped by closest('form') automatically and would regress on migration.

Cheapest hardening: skip targets that are not rendered (el.checkVisibility?.() ?? true), and say in the docs that a dialog-hosted form must pass scrollRoot. A closest('form') fallback is not available to you here — useForm has no element — which is the real argument for making the scope explicit rather than "omit = document".

[dmooibroek]

Same root cause as the two crit threads above (shared httpService / document-wide query race) — agree scrollRoot needs to be documented as required for dialog-hosted forms, not left implicit.

[Goosterhof]

Minor — "the marker the presentation layer sets from the error bag" is a claim about consumers the package does not check. It holds for ui-inputs consumers (BIO, isms) and town-crier; it does not hold for emmie or ublgenie today. State the precondition as a precondition ("requires the consumer to render aria-invalid=\"true\" on the errored control") rather than as a description of the world. Also: the react-hook-form precedent cited in the PR body is shouldFocusError — it focuses, which scrolls for free and gives keyboard/AT users a landing point; this scrolls without focusing, which the body acknowledges. Fine to defer focus, but do not cite the precedent as if this matches it.

[Goosterhof]

Minor — test hygiene, two points. (1) Every case here attaches to document.body and unmounts at the end of the test, so a failing assertion leaks a mounted [aria-invalid="true"] into every later case and the document-wide query then finds a stale node — put wrapper.unmount() in an afterEach. (2) Every fixture renders the mark in the same component that calls useForm; the fleet shape is a child component (ui-inputs TextInput, emmie FieldError) receiving the bag as a prop. The flush: 'post' claim is correct across that boundary, but no test proves it — add one with a child that renders aria-invalid from a prop, since that is the case a future refactor to flush: 'sync' or a manual nextTick would break.

[dmooibroek]

Confirmed on point 1: form.spec.ts calls wrapper.unmount() at the end of each test body, not in afterEach, so a failing assertion mid-test skips it and leaks a marked node into later tests' document-wide query.

[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.