feat: focus visually first invalid field across composed forms#1682
feat: focus visually first invalid field across composed forms#1682serikjensen wants to merge 2 commits intomainfrom
Conversation
When composeSubmitHandler validates multiple forms and any are invalid, focus now lands on the field that's visually first in the DOM rather than the first form's first error in array order. This fixes the cross-form gap that surfaced when partners interleave fields from multiple form hooks on the same page. Implementation introduces a per-form FieldElementRegistry populated by useField on mount/unmount, read by composeSubmitHandler via _fieldElementRegistry to sort invalid fields by getBoundingClientRect() and focus the winner directly (bypassing RHF setFocus, which doesn't reliably resolve refs through react-aria-components). The registry is published via context through two complementary paths covering both field-connection modes documented in docs/hooks/hooks.md: - SDKFormProvider wraps with FieldElementRegistryProvider for the provider-based mode. - HookField components self-wrap via withFieldElementRegistry for the formHookResult-prop mode, where SDKFormProvider is absent. Each SDK form hook now constructs hookFormInternals via the new useHookFormInternals helper, which attaches the registry instance. Co-authored-by: Cursor <cursoragent@cursor.com>
serikjensen
requested review from
dmortal,
hukid,
jeffredodd and
krisxcrash
as code owners
Fresh Eyes ReviewFound 3 issues in this PR. Download findings.json — drag the file into Claude or use Generated by Fresh Eyes Reviewer | Share feedback in #ai-code-reviews |
Co-authored-by: Cursor <cursoragent@cursor.com>
![gusto-fresh-eyes[bot]](https://avatars.githubusercontent.com/in/3457483?s=60&v=4){kind=small}
| import { useController, useFormContext } from 'react-hook-form' | ||
| import React, { useMemo, type Ref } from 'react' | ||
| import React, { useCallback, useMemo, type Ref } from 'react' | ||
| import { useFieldElementRegistryContext } from '@/partner-hook-utils/form/fieldElementRegistry' |
There was a problem hiding this comment.
🟠 Major | module-boundaries
Reverse dependency: Common/Fields/hooks/useField.ts imports from @/partner-hook-utils/form/fieldElementRegistry. The established pattern is one-directional: partner-hook-utils depends on Common, never the reverse. This creates a bidirectional coupling between the two modules (partner-hook-utils/form/fields/*HookField → Common (via TextInputField etc.) → partner-hook-utils). The fieldElementRegistry context/hook could live in Common/Fields/hooks/ or a shared lower-level location instead, since it's consumed at the useField level and has no dependencies on partner-hook-utils internals.
| const fieldElementRegistry = useFieldElementRegistryContext() | ||
| const registryRef = useCallback( | ||
| (element: HTMLElement | null) => { | ||
| if (!fieldElementRegistry) return | ||
| fieldElementRegistry.register(name, element) | ||
| }, |
There was a problem hiding this comment.
🟡 Minor | test-coverage
The new registryRef callback in useField is not tested in useField.test.tsx. While it's exercised indirectly via the composeSubmitHandler.focus.test.tsx integration test, useField's own test file has no coverage for the registry registration/unregistration lifecycle.
| elements.set(name, element) | ||
| } else { | ||
| elements.delete(name) |
There was a problem hiding this comment.
🟡 Minor | test-coverage
The cleanup/unmount path (register called with null to delete an entry) has no direct test. The integration tests verify registration works but don't explicitly exercise field unregistration when a component unmounts.
1 participant
Summary
When
composeSubmitHandlervalidates multiple forms and any are invalid, focus now lands on the field that is visually first in the DOM rather than the first form's first error in array order. This fixes the cross-form gap that surfaces when partners interleave fields from multiple form hooks on the same page.How it works
A per-form
FieldElementRegistryis populated byuseFieldon mount/unmount (via a ref callback added to its existinguseForkRef) and read bycomposeSubmitHandlerto sort invalid fields bygetBoundingClientRect()and focus the winner directly.Direct
element.focus()bypasses RHF'ssetFocus, which doesn't reliably resolve refs throughreact-aria-components(_fields[name]._f.refends up as a synthetic placeholder rather than the DOM node).Two registry-publish paths
The registry is published via context through two complementary paths covering both field-connection modes documented in docs/hooks/hooks.md:
SDKFormProviderwraps withFieldElementRegistryProvider.withFieldElementRegistryusing the registry forwarded on theformHookResultprop, so the focus behavior works even whenSDKFormProvideris absent.Errors captured from
onInvalidReads errors from each form's
handleSubmit(onValid, onInvalid)callback rather thanformState.errors— the latter is a proxy that returns{}when accessed outside a component subscription.Hooks aligned
All eight SDK form hooks now construct
hookFormInternalsvia the newuseHookFormInternals(formMethods)helper, which attaches the registry instance:useEmployeeDetailsForm,useHomeAddressForm,useWorkAddressFormuseCompensationForm,usePayScheduleFormuseSignCompanyForm,useSignEmployeeFormuseFederalTaxesForm(latest hook from feat!: migrate Employee.FederalTaxes to hook architecture and add steady-state edit mode #1670)Wrapper hooks (
useCurrentHomeAddressForm,useCurrentWorkAddressForm) need no changes — they spread the underlying hook'shookFormInternals.Compatibility with raw
UseFormReturnslotsThe
ComposeSubmitInputunion (rawUseFormReturn<T>support added in a recent main commit) is preserved. Raw forms have no registry, so they don't participate in DOM-order sorting and gracefully fall back to RHF'ssetFocusfor their own first error.Test plan
composeSubmitHandler.focus.test.tsxcovers both paths:SDKFormProviderwith interleaved fields → focus lands on the visually first invalid field.formHookResultprop (noSDKFormProvider) → still focuses the visually first invalid field.composeSubmitHandler.test.tscover DOM-order sorting, ties broken byleft, fallback when no form publishes a registry, fallback when registry has no entries for the error fields.Made with Cursor