Skip to content

Commit c0d0bc8

Browse files
os-zhuangclaude
andauthored
fix(form): a wizard with allowSkip no longer submits past the fields you skipped (#3030)
`allowSkip` let the user jump to any step from the indicator, and handleStepClick did so without validating anything on the way. Since a wizard mounts ONE step at a time and react-hook-form only validates the fields currently MOUNTED, a required field on a step nobody opened was never registered, never validated, and simply absent from the payload. Measured against the unfixed component — 3 steps, required `owner` on step 2, allowSkip, click step 3's indicator, fill it, hit Create: createCalls: 1 payload: { subject: 'S1', notes: 'S3' } // `owner` missing entirely UI mentions "required": false // nothing said so An invalid create went out and the client said nothing about why — #2959's validation half, wearing a wizard's clothes. The final submit now checks the WHOLE declared field set: when something is outstanding it returns the user to the first step holding one, marks that step's indicator (data-error, destructive circle + icon), names the fields in a toast, and sends nothing. Conditional rules are honoured — the check runs on the canonical resolveFieldRuleState, the same engine the form renderer and the server's rule-validator use, so a field hidden by visibleWhen or not yet required by requiredWhen is not demanded. The sequential path is unaffected: a forward jump is refused without allowSkip, so Next already validated each step on the way. Also in WizardForm: - FormView.columns is now honoured (spec key, previously dropped) — grid width is the view's `columns`, else the step's own. No widest-section fallback, unlike the tabbed/split hosts: wizard steps never share a viewport, so each keeps its authored width. - the root gained `@container`. The step grid is sized with container queries, so without a container ancestor every @md:/@2XL: variant was inert and a step declaring 2 columns rendered single-column. Found by running it in a browser — the class was present all along, which is why asserting the class alone had missed it; the test now also asserts the container ancestor. Browser-verified end to end (zero-backend harness): skip -> submit refused, back on the offending step with only that indicator marked and the two field names in the toast, no request sent; fill it -> marker clears and the create carries all three steps' values. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 9969e9f commit c0d0bc8

5 files changed

Lines changed: 469 additions & 17 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@object-ui/plugin-form": patch
3+
---
4+
5+
fix(form): a wizard with `allowSkip` no longer submits past the required fields you skipped
6+
7+
`allowSkip` let the user jump to any step from the indicator, and
8+
`handleStepClick` did so without validating anything on the way. Since a wizard
9+
mounts ONE step at a time and react-hook-form only validates the fields currently
10+
**mounted**, a required field on a step nobody opened was never registered, never
11+
validated, and simply absent from the payload.
12+
13+
Measured against the unfixed component — 3 steps, required `owner` on step 2,
14+
`allowSkip: true`, click step 3's indicator, fill it, hit Create:
15+
16+
createCalls: 1
17+
payload: { subject: 'S1', notes: 'S3' } // `owner` missing entirely
18+
UI mentions "required": false // nothing said so
19+
20+
So an invalid create went out and the client said nothing about why — #2959's
21+
validation half, wearing a wizard's clothes.
22+
23+
The final submit now checks the WHOLE declared field set, and when something is
24+
outstanding it returns the user to the first step that has one, marks that step's
25+
indicator (`data-error="true"`, destructive circle + icon), names the fields in a
26+
toast, and sends nothing. Conditional rules are honoured: the check runs on the
27+
canonical `resolveFieldRuleState`, the same engine the form renderer and the
28+
server's rule-validator use, so a field hidden by `visibleWhen` or not yet
29+
required by `requiredWhen` is not demanded. The sequential path is unaffected —
30+
a forward jump is refused without `allowSkip`, so Next already validated each step.
31+
32+
Also in `WizardForm`:
33+
34+
- `FormView.columns` is now honoured (spec key, previously dropped): the grid
35+
width is the view's `columns`, else the step's own. Unlike the tabbed/split
36+
hosts there is no widest-section fallback — wizard steps never share a viewport,
37+
so each keeps its authored width.
38+
- the root gained `@container`. The step grid is sized with container queries, and
39+
without a container ancestor every `@md:`/`@2xl:` variant was inert — a step
40+
declaring 2 columns rendered single-column. Found by running it in a browser;
41+
the class was present all along, which is why asserting the class alone had
42+
missed it.

content/docs/plugins/plugin-form.mdx

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,11 @@ A sectioned form renders as ONE grid. The form view's `columns` (spec
6969
densely that section fills it. The view's value wins; without it the grid takes
7070
the widest section's density, and without either it is single-column.
7171

72-
`ObjectForm` (simple), `ModalForm`, `TabbedForm` and `SplitForm` all resolve it
73-
that way, so the same metadata lays out identically in every host. The grid is
74-
applied to the field container inside the form, never wrapped around the `<form>`.
72+
`ObjectForm` (simple), `ModalForm`, `TabbedForm`, `SplitForm` and `WizardForm` all
73+
resolve it that way, so the same metadata lays out identically in every host
74+
(`WizardForm` has no widest-section fallback — its steps never share a viewport,
75+
so each keeps its own authored width). The grid is applied to the field container
76+
inside the form, never wrapped around the `<form>`.
7577

7678
### Tabbed field layout (`fieldTabs`)
7779

@@ -102,6 +104,20 @@ inactive tab unmount along with its values).
102104

103105
`ModalForm` (`contentLayout: 'tabbed'`) and `TabbedForm` build on this.
104106

107+
### Wizard steps and `allowSkip`
108+
109+
`allowSkip` lets the user jump to any step from the indicator instead of walking
110+
through them in order. It is navigation freedom, not an exemption from the
111+
object's rules: the **final submit checks every step's required fields**, and if
112+
something is outstanding it returns the user to the first step that has one, marks
113+
that step's indicator (`data-error="true"`), and names the fields in a toast —
114+
nothing is sent. Conditional rules are respected (`visibleWhen` / `requiredWhen`
115+
are evaluated on the same canonical engine as the renderer and the server).
116+
117+
This matters because react-hook-form only validates the fields currently mounted,
118+
and a wizard mounts one step at a time — so a required field on a step nobody
119+
opened used to be absent from the payload with nothing on screen saying so.
120+
105121
### Split field layout (`fieldPanes`)
106122

107123
Side-by-side panels follow the same rule: the `<form>` wraps the whole panel

packages/plugin-form/README.md

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,10 @@ A sectioned form renders as ONE grid, and two keys decide its shape:
9494

9595
The view's `columns` wins; without it the grid takes the widest section's
9696
density, and without either it is single-column. Every sectioned host resolves it
97-
the same way — `ObjectForm` (simple), `ModalForm`, `TabbedForm`, `SplitForm`
98-
so one piece of metadata lays out identically wherever it is rendered.
97+
the same way — `ObjectForm` (simple), `ModalForm`, `TabbedForm`, `SplitForm`,
98+
`WizardForm` — so one piece of metadata lays out identically wherever it is
99+
rendered. (`WizardForm` has no widest-section fallback: its steps never share a
100+
viewport, so each step keeps its own authored width.)
99101

100102
The grid is applied to the field container **inside** the form, never wrapped
101103
around the `<form>` (which would put the whole form in cell 1 and leave the other
@@ -132,6 +134,26 @@ the renderer distribute the fields:
132134

133135
`ModalForm` (`contentLayout: 'tabbed'`) and `TabbedForm` are built on this.
134136

137+
### Wizard steps and `allowSkip`
138+
139+
`allowSkip` lets the user jump to any step from the indicator instead of walking
140+
through them in order. It is **navigation** freedom, not an exemption from the
141+
object's rules:
142+
143+
- Next validates the step you are leaving, as always.
144+
- The **final submit checks every step's required fields** — not just the last
145+
one's — and if something is outstanding it sends the user to the first step
146+
that has one, marks that step's indicator (`data-error="true"`), and names the
147+
fields in a toast. Nothing is sent.
148+
- Conditional rules are respected: a field whose `visibleWhen` is false, or whose
149+
`requiredWhen` is false, is not demanded. The check runs on the same canonical
150+
engine as the renderer and the server's rule-validator, so all three agree.
151+
152+
This matters because react-hook-form only validates the fields currently
153+
**mounted**, and a wizard mounts one step at a time — so a required field on a
154+
step nobody opened used to be absent from the payload with nothing on screen
155+
saying so (#2959's validation half, in a wizard).
156+
135157
### Split field layout (`fieldPanes`)
136158

137159
The same rule for side-by-side panels: the `<form>` wraps the whole panel group

packages/plugin-form/src/WizardForm.tsx

Lines changed: 145 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,28 @@
1616
import React, { useState, useCallback, useMemo } from 'react';
1717
import type { FormField, DataSource } from '@object-ui/types';
1818
import { Button, cn, toast } from '@object-ui/components';
19-
import { Check, ChevronLeft, ChevronRight, Loader2 } from 'lucide-react';
19+
import { AlertCircle, Check, ChevronLeft, ChevronRight, Loader2 } from 'lucide-react';
20+
import { resolveFieldRuleState } from '@object-ui/core';
21+
import { createSafeTranslation } from '@object-ui/i18n';
2022
import { FormSection } from './FormSection';
2123
import { SchemaRenderer, useSafeFieldLabel } from '@object-ui/react';
2224
import { buildSectionFields as buildSectionFieldsShared } from './sectionFields';
23-
import { sectionFormLayout } from './autoLayout';
25+
import { applyAutoColSpan, containerGridColsFor } from './autoLayout';
2426
import { resolveSuccessNavigate, isSameOriginUrl, type SubmitBehavior } from './successBehavior';
2527
import type { FormSectionConfig } from './TabbedForm';
2628

29+
// Falls back to English when no i18n provider is mounted.
30+
const useWizardTranslation = createSafeTranslation(
31+
{
32+
'wizard.missingRequired': 'Please complete the required fields: {{fields}}',
33+
},
34+
'wizard.missingRequired',
35+
);
36+
37+
/** Empty for the purposes of a required check: '' / null / undefined / []. */
38+
const isEmptyValue = (v: unknown): boolean =>
39+
v === undefined || v === null || v === '' || (Array.isArray(v) && v.length === 0);
40+
2741
export interface WizardFormSchema {
2842
type: 'object-form';
2943
formType: 'wizard';
@@ -49,10 +63,23 @@ export interface WizardFormSchema {
4963
sections: FormSectionConfig[];
5064

5165
/**
52-
* Allow navigation to any step (not just sequential)
66+
* Allow navigation to any step (not just sequential).
67+
*
68+
* This is navigation freedom, NOT an exemption from the object's rules: the
69+
* final submit still checks every step's required fields and sends the user
70+
* back to the first step that has one outstanding (#2959's validation half —
71+
* react-hook-form only validates the fields currently mounted, so a step
72+
* nobody opened used to reach the server unvalidated).
5373
* @default false
5474
*/
5575
allowSkip?: boolean;
76+
77+
/**
78+
* Grid width for each step (1–4). Aligns with @objectstack/spec
79+
* FormView.columns and OUTRANKS the per-step `columns`, which says how densely
80+
* that step fills the grid. Omitted = the step's own `columns`, else 1.
81+
*/
82+
columns?: number;
5683

5784
/**
5885
* Show step indicators
@@ -181,13 +208,18 @@ export const WizardForm: React.FC<WizardFormProps> = ({
181208
className,
182209
}) => {
183210
const { fieldLabel } = useSafeFieldLabel();
211+
const { t } = useWizardTranslation();
184212
const [objectSchema, setObjectSchema] = useState<any>(null);
185213
const [formData, setFormData] = useState<Record<string, any>>({});
186214
const [loading, setLoading] = useState(true);
187215
const [error, setError] = useState<Error | null>(null);
188216
const [currentStep, setCurrentStep] = useState(0);
189217
const [completedSteps, setCompletedSteps] = useState<Set<number>>(new Set());
190218
const [submitting, setSubmitting] = useState(false);
219+
// Steps the final submit found an outstanding required field on, so their
220+
// indicator can say so — a rejection on a step the user is not looking at is
221+
// otherwise invisible and the submit just appears to do nothing.
222+
const [invalidSteps, setInvalidSteps] = useState<Set<number>>(new Set());
191223
// Terminal state for `submitBehavior: { kind: 'thank-you' | 'next-record' }`.
192224
// Without this the last step's form stayed mounted and fully filled after a
193225
// successful create, with nothing disabling "Create" — a second click fired
@@ -284,15 +316,85 @@ export const WizardForm: React.FC<WizardFormProps> = ({
284316
return [];
285317
}, [currentStep, totalSteps, schema.sections, buildSectionFields]);
286318

319+
/**
320+
* Required fields the record does not satisfy, grouped by the STEP that
321+
* declares them.
322+
*
323+
* react-hook-form only validates the fields currently MOUNTED, and a wizard
324+
* mounts one step at a time. Sequential navigation is safe (Next validates the
325+
* step you are leaving), but `allowSkip` jumps straight to any step — so a
326+
* required field on a step nobody opened was never registered, never
327+
* validated, and simply absent from the create payload, with nothing on screen
328+
* saying so. The final submit therefore re-checks the WHOLE declared field set
329+
* here.
330+
*
331+
* Uses the canonical rule engine (`resolveFieldRuleState`), the same one the
332+
* form renderer and the server's rule-validator use, so a conditionally
333+
* required/hidden field gets the same verdict from all three rather than a
334+
* second, divergent dialect.
335+
*/
336+
const missingRequiredByStep = useCallback(
337+
(record: Record<string, any>): Map<number, string[]> => {
338+
const out = new Map<number, string[]>();
339+
schema.sections.forEach((section, index) => {
340+
for (const field of buildSectionFields(section)) {
341+
const name = field?.name;
342+
if (!name || (field as any).hidden === true) continue;
343+
const state = resolveFieldRuleState(
344+
{
345+
visibleWhen: (field as any).visibleWhen,
346+
readonlyWhen: (field as any).readonlyWhen,
347+
requiredWhen: (field as any).requiredWhen,
348+
},
349+
record,
350+
{ required: !!field.required, readonly: (field as any).readonly === true },
351+
);
352+
// A hidden or read-only field is not the user's to fill in.
353+
if (!state.visible || state.readonly || !state.required) continue;
354+
if (!isEmptyValue(record[name])) continue;
355+
out.set(index, [...(out.get(index) ?? []), (field as any).label || name]);
356+
}
357+
});
358+
return out;
359+
},
360+
[schema.sections, buildSectionFields],
361+
);
362+
287363
// Handle step data collection (merge partial data into formData)
288364
const handleStepSubmit = useCallback(async (stepData: Record<string, any>) => {
289365
const mergedData = { ...formData, ...stepData };
290366
setFormData(mergedData);
291-
367+
292368
// Mark step as completed
293369
setCompletedSteps(prev => new Set(prev).add(currentStep));
370+
// This step just cleared its own validation, so drop any marker it carried.
371+
setInvalidSteps(prev => {
372+
if (!prev.has(currentStep)) return prev;
373+
const next = new Set(prev);
374+
next.delete(currentStep);
375+
return next;
376+
});
294377

295378
if (isLastStep) {
379+
// Gate the submit on the FULL field set, not just this step's (see
380+
// missingRequiredByStep) — then point the user at the first step that is
381+
// short something, instead of letting the server answer with a 400 that
382+
// names fields the user cannot even see.
383+
const missing = missingRequiredByStep(mergedData);
384+
if (missing.size > 0) {
385+
setInvalidSteps(new Set(missing.keys()));
386+
const labels = [...missing.values()].flat();
387+
const MAX = 3;
388+
toast.error(
389+
t('wizard.missingRequired', {
390+
fields: labels.slice(0, MAX).join(', ') + (labels.length > MAX ? '…' : ''),
391+
}),
392+
);
393+
goToStep(Math.min(...missing.keys()));
394+
return;
395+
}
396+
setInvalidSteps(new Set());
397+
296398
// Final submission
297399
setSubmitting(true);
298400
try {
@@ -371,7 +473,7 @@ export const WizardForm: React.FC<WizardFormProps> = ({
371473
// Move to next step
372474
goToStep(currentStep + 1);
373475
}
374-
}, [formData, currentStep, isLastStep, schema, dataSource]);
476+
}, [formData, currentStep, isLastStep, schema, dataSource, missingRequiredByStep, t]);
375477

376478
// Navigation
377479
const goToStep = useCallback((step: number) => {
@@ -431,9 +533,17 @@ export const WizardForm: React.FC<WizardFormProps> = ({
431533
}
432534

433535
const currentSection = schema.sections[currentStep];
536+
const clampCol = (n: unknown): number | undefined =>
537+
typeof n === 'number' && n > 0 ? Math.min(Math.floor(n), 4) : undefined;
538+
const stepGridColumns = clampCol(schema.columns) ?? clampCol(currentSection?.columns) ?? 1;
539+
const stepFieldContainerClass = containerGridColsFor(stepGridColumns);
434540

435541
return (
436-
<div className={cn('w-full', className, schema.className)}>
542+
// `@container`: the step's field grid is sized with container queries
543+
// (`@md:grid-cols-2` …), which need a container ancestor to resolve against —
544+
// without one the classes are inert and a multi-column step silently stayed
545+
// single-column. Same wrapper TabbedForm / SplitForm carry.
546+
<div className={cn('w-full @container', className, schema.className)}>
437547
{/* Step Indicator */}
438548
{schema.showStepIndicator !== false && (
439549
<nav aria-label="Progress" className="mb-8">
@@ -442,6 +552,8 @@ export const WizardForm: React.FC<WizardFormProps> = ({
442552
const isActive = index === currentStep;
443553
const isCompleted = completedSteps.has(index);
444554
const isClickable = schema.allowSkip || isCompleted || index <= currentStep;
555+
// The last submit found a required field outstanding on this step.
556+
const hasError = invalidSteps.has(index);
445557

446558
return (
447559
<li
@@ -474,17 +586,24 @@ export const WizardForm: React.FC<WizardFormProps> = ({
474586
)}
475587
onClick={() => handleStepClick(index)}
476588
disabled={!isClickable}
589+
data-testid={`wizard-step:${section.name || index}`}
590+
data-error={hasError || undefined}
477591
>
478592
{/* Step circle - smaller on mobile */}
479593
<span
480594
className={cn(
481595
'flex h-6 w-6 sm:h-8 sm:w-8 items-center justify-center rounded-full text-xs sm:text-sm font-medium transition-colors',
482-
isCompleted && 'bg-primary text-primary-foreground',
483-
isActive && !isCompleted && 'border-2 border-primary bg-background text-primary',
484-
!isActive && !isCompleted && 'border-2 border-muted bg-background text-muted-foreground'
596+
// An outstanding required field outranks the completed
597+
// tick: the step needs attention, whatever else it is.
598+
hasError && 'border-2 border-destructive bg-background text-destructive',
599+
!hasError && isCompleted && 'bg-primary text-primary-foreground',
600+
!hasError && isActive && !isCompleted && 'border-2 border-primary bg-background text-primary',
601+
!hasError && !isActive && !isCompleted && 'border-2 border-muted bg-background text-muted-foreground'
485602
)}
486603
>
487-
{isCompleted ? (
604+
{hasError ? (
605+
<AlertCircle className="h-3 w-3 sm:h-4 sm:w-4" aria-hidden />
606+
) : isCompleted ? (
488607
<Check className="h-3 w-3 sm:h-4 sm:w-4" />
489608
) : (
490609
index + 1
@@ -495,7 +614,9 @@ export const WizardForm: React.FC<WizardFormProps> = ({
495614
<span className="ml-2 sm:ml-3 text-xs sm:text-sm font-medium hidden sm:block">
496615
<span
497616
className={cn(
498-
isActive ? 'text-foreground' : 'text-muted-foreground'
617+
hasError
618+
? 'text-destructive'
619+
: isActive ? 'text-foreground' : 'text-muted-foreground'
499620
)}
500621
>
501622
{section.label || `Step ${index + 1}`}
@@ -528,7 +649,19 @@ export const WizardForm: React.FC<WizardFormProps> = ({
528649
id: stepFormId,
529650
// Multi-column on the field container inside the form, not a
530651
// grid around the whole form (which leaves columns empty).
531-
...sectionFormLayout(currentSectionFields, currentSection.columns || 1),
652+
//
653+
// Grid width: the form view's own `columns` first (spec
654+
// FormView.columns — it was being dropped, so a view that
655+
// declared 3 columns rendered single-column here while the same
656+
// metadata gave 3 in a modal), else this step's own `columns`.
657+
// Unlike the tabbed/split hosts there is no widest-section
658+
// fallback: wizard steps never share a viewport, so there is no
659+
// shared grid to size, and each step keeps its authored width.
660+
fields: stepGridColumns > 1
661+
? applyAutoColSpan(currentSectionFields, stepGridColumns, clampCol(currentSection.columns))
662+
: currentSectionFields,
663+
columns: stepGridColumns,
664+
...(stepFieldContainerClass ? { fieldContainerClass: stepFieldContainerClass } : {}),
532665
layout: 'vertical' as const,
533666
defaultValues: formData,
534667
showSubmit: false,

0 commit comments

Comments
 (0)