Skip to content

Commit c19ac11

Browse files
os-zhuangclaude
andauthored
fix(form): scroll+focus the first errored field on invalid submit (#2793) (#2813)
The missing-required-field toast (#2329) names the fields but leaves the user hunting — in a long form the offending field is off-screen and RHF's native focus-on-error silently no-ops for custom widgets (lookup / select / master-detail), the exact reported case. Disable RHF's unreliable shouldFocusError and, in onInvalid, scroll the first errored field (visual order) into view via its data-field wrapper and focus a focusable control inside it — works for every field type. Verified with an RTL DOM test that renders the real form renderer: submit with a missing required field toasts, scrolls, and focuses the first error; picks the first *errored* field when an earlier one is valid; stays quiet on a valid submit. Red-checked against the pre-fix code. Full form suite (15) and type-check pass. Browser verification was blocked by the shared checkout's mid-migration state; the DOM test covers the real render path. Closes #2793 Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent b5f4e0f commit c19ac11

3 files changed

Lines changed: 168 additions & 4 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@object-ui/components": patch
3+
---
4+
5+
fix(form): scroll and focus the first errored field on an invalid submit (#2793)
6+
7+
Submitting a form with a missing required field already toasts the offending field names (#2329), but in a long form the field itself stays off-screen, so the user still hunts for it. react-hook-form's native focus-on-error only reaches fields whose registered ref is a focusable native input — it silently no-ops for custom widgets (lookup / select / master-detail), which is exactly the reported case. The form renderer now disables RHF's unreliable `shouldFocusError` and, in its `onInvalid` handler, scrolls the first errored field (in visual/declared order) into view via its `data-field` wrapper and focuses a focusable control inside it — working for every field type.
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* #2793: submitting a form with a missing required field must give visible
11+
* feedback — a toast naming the fields (already shipped in #2329) AND scroll
12+
* the first errored field into view + focus it. Before this, react-hook-form's
13+
* native focus-on-error silently no-opped for custom widgets (lookup / select /
14+
* master-detail), so on a long form the user clicked 创建 and saw nothing.
15+
*/
16+
17+
import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest';
18+
import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react';
19+
import { ComponentRegistry } from '@object-ui/core';
20+
import { toast } from '../../../ui/sonner';
21+
22+
beforeAll(async () => {
23+
await import('../../../renderers');
24+
}, 30000);
25+
26+
let toastErrorSpy: ReturnType<typeof vi.spyOn>;
27+
28+
beforeEach(() => {
29+
toastErrorSpy = vi.spyOn(toast, 'error').mockImplementation(() => 'id' as any);
30+
// happy-dom doesn't implement scrollIntoView — install a no-op we can spy on.
31+
if (!(Element.prototype as any).scrollIntoView) {
32+
(Element.prototype as any).scrollIntoView = () => {};
33+
}
34+
});
35+
36+
afterEach(() => {
37+
cleanup();
38+
vi.restoreAllMocks();
39+
});
40+
41+
function renderForm(fields: any[], defaultValues?: Record<string, unknown>) {
42+
const Form = ComponentRegistry.get('form')!;
43+
return render(
44+
<Form
45+
schema={{
46+
type: 'form',
47+
mode: 'create',
48+
showSubmit: true,
49+
showCancel: false,
50+
submitLabel: 'Create',
51+
fields,
52+
...(defaultValues ? { defaultValues } : {}),
53+
}}
54+
/>,
55+
);
56+
}
57+
58+
const submit = () => fireEvent.click(screen.getByRole('button', { name: /create/i }));
59+
60+
describe('form renderer — invalid-submit feedback (#2793)', () => {
61+
it('toasts and scrolls+focuses the first errored field on a missing required field', async () => {
62+
const { container } = renderForm([
63+
{ name: 'title', label: 'Title', type: 'input', required: true },
64+
{ name: 'project', label: 'Project', type: 'input', required: true },
65+
]);
66+
67+
const titleWrap = container.querySelector('[data-field="title"]') as HTMLElement;
68+
const scrollSpy = vi.fn();
69+
titleWrap.scrollIntoView = scrollSpy;
70+
71+
submit();
72+
73+
await waitFor(() => expect(toastErrorSpy).toHaveBeenCalled());
74+
// Names the missing field(s).
75+
expect(String(toastErrorSpy.mock.calls[0][0])).toMatch(/Title/);
76+
// First errored field (visual order) scrolled into view + focused.
77+
expect(scrollSpy).toHaveBeenCalled();
78+
await waitFor(() =>
79+
expect(titleWrap.contains(document.activeElement)).toBe(true),
80+
);
81+
});
82+
83+
it('scrolls the first errored field even when an earlier field is valid', async () => {
84+
const { container } = renderForm(
85+
[
86+
{ name: 'title', label: 'Title', type: 'input', required: true },
87+
{ name: 'project', label: 'Project', type: 'input', required: true },
88+
],
89+
{ title: 'Filled' },
90+
);
91+
92+
const titleWrap = container.querySelector('[data-field="title"]') as HTMLElement;
93+
const projectWrap = container.querySelector('[data-field="project"]') as HTMLElement;
94+
const titleScroll = vi.fn();
95+
const projectScroll = vi.fn();
96+
titleWrap.scrollIntoView = titleScroll;
97+
projectWrap.scrollIntoView = projectScroll;
98+
99+
submit();
100+
101+
await waitFor(() => expect(toastErrorSpy).toHaveBeenCalled());
102+
// Only `project` is invalid → it, not the valid `title`, is scrolled to.
103+
expect(projectScroll).toHaveBeenCalled();
104+
expect(titleScroll).not.toHaveBeenCalled();
105+
});
106+
107+
it('does not toast or scroll when the form is valid', async () => {
108+
const { container } = renderForm(
109+
[{ name: 'title', label: 'Title', type: 'input', required: true }],
110+
{ title: 'Filled' },
111+
);
112+
const wrap = container.querySelector('[data-field="title"]') as HTMLElement;
113+
const scrollSpy = vi.fn();
114+
wrap.scrollIntoView = scrollSpy;
115+
116+
submit();
117+
118+
// Give any async validation a tick, then assert no error feedback fired.
119+
await new Promise((r) => setTimeout(r, 50));
120+
expect(toastErrorSpy).not.toHaveBeenCalled();
121+
expect(scrollSpy).not.toHaveBeenCalled();
122+
});
123+
});

packages/components/src/renderers/form/form.tsx

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -313,12 +313,21 @@ ComponentRegistry.register('form',
313313
disabled = false,
314314
} = schema;
315315

316-
// Initialize react-hook-form
316+
// Initialize react-hook-form. `shouldFocusError: false` because RHF's
317+
// native focus-on-error only works for fields whose registered ref is a
318+
// focusable native input — it silently no-ops for custom widgets
319+
// (lookup / select / master-detail), which is exactly the #2793 case. We
320+
// own the scroll+focus explicitly in the onInvalid handler below so it
321+
// works for every field type and follows visual order.
317322
const form = useForm({
318323
defaultValues,
319324
mode: validationMode,
325+
shouldFocusError: false,
320326
});
321327

328+
// Scoped to this form so the error-scroll query never reaches a sibling form.
329+
const formRef = React.useRef<HTMLFormElement>(null);
330+
322331
const [isSubmitting, setIsSubmitting] = React.useState(false);
323332
const [submitError, setSubmitError] = React.useState<string | null>(null);
324333

@@ -575,6 +584,30 @@ ComponentRegistry.register('form',
575584
const MAX = 3;
576585
const fieldsText = labels.slice(0, MAX).join('、') + (labels.length > MAX ? '…' : '');
577586
toast.error(t('validation.formInvalid', { fields: fieldsText }));
587+
588+
// #2793: the toast alone still leaves the user hunting — in a long form
589+
// the offending field is off-screen. Scroll the FIRST errored field into
590+
// view (in declared/visual order, not RHF's error-key order) and focus a
591+
// focusable control inside it. The field wrapper carries `data-field`
592+
// (FormItem); works for custom widgets that RHF's own focus can't reach.
593+
const errored = new Set(names);
594+
const firstName =
595+
(fields as FormFieldConfig[])
596+
.map((f) => f?.name)
597+
.find((n): n is string => Boolean(n) && errored.has(n as string)) ?? names[0];
598+
const root: ParentNode = formRef.current ?? document;
599+
const escaped =
600+
typeof CSS !== 'undefined' && typeof CSS.escape === 'function'
601+
? CSS.escape(firstName)
602+
: firstName.replace(/["\\]/g, '\\$&');
603+
const target = root.querySelector<HTMLElement>(`[data-field="${escaped}"]`);
604+
if (target) {
605+
target.scrollIntoView?.({ behavior: 'smooth', block: 'center' });
606+
const focusable = target.querySelector<HTMLElement>(
607+
'input:not([type="hidden"]), select, textarea, button, [tabindex]:not([tabindex="-1"]), [contenteditable="true"]',
608+
);
609+
focusable?.focus?.({ preventScroll: true });
610+
}
578611
});
579612

580613
// Handle cancel
@@ -645,9 +678,10 @@ ComponentRegistry.register('form',
645678

646679
return (
647680
<Form {...form}>
648-
<form
649-
onSubmit={handleSubmit}
650-
className={className}
681+
<form
682+
ref={formRef}
683+
onSubmit={handleSubmit}
684+
className={className}
651685
{...formProps}
652686
// Apply designer props
653687
data-obj-id={dataObjId}

0 commit comments

Comments
 (0)