From bcf4ab202179473eb5cbb77abce0f6a38c3e0074 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:38:33 +0800 Subject: [PATCH] fix(form): a wizard with `allowSkip` no longer submits past the fields you skipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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: Claude Opus 5 --- .changeset/wizard-skip-required-fields.md | 42 +++ content/docs/plugins/plugin-form.mdx | 22 +- packages/plugin-form/README.md | 26 +- packages/plugin-form/src/WizardForm.tsx | 157 +++++++++++- .../src/wizardSkipValidation.test.tsx | 239 ++++++++++++++++++ 5 files changed, 469 insertions(+), 17 deletions(-) create mode 100644 .changeset/wizard-skip-required-fields.md create mode 100644 packages/plugin-form/src/wizardSkipValidation.test.tsx diff --git a/.changeset/wizard-skip-required-fields.md b/.changeset/wizard-skip-required-fields.md new file mode 100644 index 000000000..ec9e6e860 --- /dev/null +++ b/.changeset/wizard-skip-required-fields.md @@ -0,0 +1,42 @@ +--- +"@object-ui/plugin-form": patch +--- + +fix(form): a wizard with `allowSkip` no longer submits past the required fields you skipped + +`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: true`, 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 + +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, and when something is +outstanding it returns the user to the first step that has one, marks that step's +indicator (`data-error="true"`, 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. + +Also in `WizardForm`: + +- `FormView.columns` is now honoured (spec key, previously dropped): the grid + width is the view's `columns`, else the step's own. Unlike the tabbed/split + hosts there is no widest-section fallback — wizard steps never share a viewport, + so each keeps its authored width. +- the root gained `@container`. The step grid is sized with container queries, and + without a container ancestor every `@md:`/`@2xl:` variant was inert — 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. diff --git a/content/docs/plugins/plugin-form.mdx b/content/docs/plugins/plugin-form.mdx index 75beca9a2..38ed3a250 100644 --- a/content/docs/plugins/plugin-form.mdx +++ b/content/docs/plugins/plugin-form.mdx @@ -69,9 +69,11 @@ A sectioned form renders as ONE grid. The form view's `columns` (spec densely that section fills it. The view's value wins; without it the grid takes the widest section's density, and without either it is single-column. -`ObjectForm` (simple), `ModalForm`, `TabbedForm` and `SplitForm` all resolve it -that way, so the same metadata lays out identically in every host. The grid is -applied to the field container inside the form, never wrapped around the `
`. +`ObjectForm` (simple), `ModalForm`, `TabbedForm`, `SplitForm` and `WizardForm` all +resolve it that way, so the same metadata lays out identically in every host +(`WizardForm` has no widest-section fallback — its steps never share a viewport, +so each keeps its own authored width). The grid is applied to the field container +inside the form, never wrapped around the ``. ### Tabbed field layout (`fieldTabs`) @@ -102,6 +104,20 @@ inactive tab unmount along with its values). `ModalForm` (`contentLayout: 'tabbed'`) and `TabbedForm` build on this. +### Wizard steps and `allowSkip` + +`allowSkip` lets the user jump to any step from the indicator instead of walking +through them in order. It is navigation freedom, not an exemption from the +object's rules: the **final submit checks every step's required fields**, and if +something is outstanding it returns the user to the first step that has one, marks +that step's indicator (`data-error="true"`), and names the fields in a toast — +nothing is sent. Conditional rules are respected (`visibleWhen` / `requiredWhen` +are evaluated on the same canonical engine as the renderer and the server). + +This matters because react-hook-form only validates the fields currently mounted, +and a wizard mounts one step at a time — so a required field on a step nobody +opened used to be absent from the payload with nothing on screen saying so. + ### Split field layout (`fieldPanes`) Side-by-side panels follow the same rule: the `` wraps the whole panel diff --git a/packages/plugin-form/README.md b/packages/plugin-form/README.md index e92b5328a..b484fb4bf 100644 --- a/packages/plugin-form/README.md +++ b/packages/plugin-form/README.md @@ -94,8 +94,10 @@ A sectioned form renders as ONE grid, and two keys decide its shape: The view's `columns` wins; without it the grid takes the widest section's density, and without either it is single-column. Every sectioned host resolves it -the same way — `ObjectForm` (simple), `ModalForm`, `TabbedForm`, `SplitForm` — -so one piece of metadata lays out identically wherever it is rendered. +the same way — `ObjectForm` (simple), `ModalForm`, `TabbedForm`, `SplitForm`, +`WizardForm` — so one piece of metadata lays out identically wherever it is +rendered. (`WizardForm` has no widest-section fallback: its steps never share a +viewport, so each step keeps its own authored width.) The grid is applied to the field container **inside** the form, never wrapped around the `` (which would put the whole form in cell 1 and leave the other @@ -132,6 +134,26 @@ the renderer distribute the fields: `ModalForm` (`contentLayout: 'tabbed'`) and `TabbedForm` are built on this. +### Wizard steps and `allowSkip` + +`allowSkip` lets the user jump to any step from the indicator instead of walking +through them in order. It is **navigation** freedom, not an exemption from the +object's rules: + +- Next validates the step you are leaving, as always. +- The **final submit checks every step's required fields** — not just the last + one's — and if something is outstanding it sends the user to the first step + that has one, marks that step's indicator (`data-error="true"`), and names the + fields in a toast. Nothing is sent. +- Conditional rules are respected: a field whose `visibleWhen` is false, or whose + `requiredWhen` is false, is not demanded. The check runs on the same canonical + engine as the renderer and the server's rule-validator, so all three agree. + +This matters because react-hook-form only validates the fields currently +**mounted**, and a wizard mounts one step at a time — so a required field on a +step nobody opened used to be absent from the payload with nothing on screen +saying so (#2959's validation half, in a wizard). + ### Split field layout (`fieldPanes`) The same rule for side-by-side panels: the `` wraps the whole panel group diff --git a/packages/plugin-form/src/WizardForm.tsx b/packages/plugin-form/src/WizardForm.tsx index 54c0e674c..8a79b9985 100644 --- a/packages/plugin-form/src/WizardForm.tsx +++ b/packages/plugin-form/src/WizardForm.tsx @@ -16,14 +16,28 @@ import React, { useState, useCallback, useMemo } from 'react'; import type { FormField, DataSource } from '@object-ui/types'; import { Button, cn, toast } from '@object-ui/components'; -import { Check, ChevronLeft, ChevronRight, Loader2 } from 'lucide-react'; +import { AlertCircle, Check, ChevronLeft, ChevronRight, Loader2 } from 'lucide-react'; +import { resolveFieldRuleState } from '@object-ui/core'; +import { createSafeTranslation } from '@object-ui/i18n'; import { FormSection } from './FormSection'; import { SchemaRenderer, useSafeFieldLabel } from '@object-ui/react'; import { buildSectionFields as buildSectionFieldsShared } from './sectionFields'; -import { sectionFormLayout } from './autoLayout'; +import { applyAutoColSpan, containerGridColsFor } from './autoLayout'; import { resolveSuccessNavigate, isSameOriginUrl, type SubmitBehavior } from './successBehavior'; import type { FormSectionConfig } from './TabbedForm'; +// Falls back to English when no i18n provider is mounted. +const useWizardTranslation = createSafeTranslation( + { + 'wizard.missingRequired': 'Please complete the required fields: {{fields}}', + }, + 'wizard.missingRequired', +); + +/** Empty for the purposes of a required check: '' / null / undefined / []. */ +const isEmptyValue = (v: unknown): boolean => + v === undefined || v === null || v === '' || (Array.isArray(v) && v.length === 0); + export interface WizardFormSchema { type: 'object-form'; formType: 'wizard'; @@ -49,10 +63,23 @@ export interface WizardFormSchema { sections: FormSectionConfig[]; /** - * Allow navigation to any step (not just sequential) + * Allow navigation to any step (not just sequential). + * + * This is navigation freedom, NOT an exemption from the object's rules: the + * final submit still checks every step's required fields and sends the user + * back to the first step that has one outstanding (#2959's validation half — + * react-hook-form only validates the fields currently mounted, so a step + * nobody opened used to reach the server unvalidated). * @default false */ allowSkip?: boolean; + + /** + * Grid width for each step (1–4). Aligns with @objectstack/spec + * FormView.columns and OUTRANKS the per-step `columns`, which says how densely + * that step fills the grid. Omitted = the step's own `columns`, else 1. + */ + columns?: number; /** * Show step indicators @@ -181,6 +208,7 @@ export const WizardForm: React.FC = ({ className, }) => { const { fieldLabel } = useSafeFieldLabel(); + const { t } = useWizardTranslation(); const [objectSchema, setObjectSchema] = useState(null); const [formData, setFormData] = useState>({}); const [loading, setLoading] = useState(true); @@ -188,6 +216,10 @@ export const WizardForm: React.FC = ({ const [currentStep, setCurrentStep] = useState(0); const [completedSteps, setCompletedSteps] = useState>(new Set()); const [submitting, setSubmitting] = useState(false); + // Steps the final submit found an outstanding required field on, so their + // indicator can say so — a rejection on a step the user is not looking at is + // otherwise invisible and the submit just appears to do nothing. + const [invalidSteps, setInvalidSteps] = useState>(new Set()); // Terminal state for `submitBehavior: { kind: 'thank-you' | 'next-record' }`. // Without this the last step's form stayed mounted and fully filled after a // successful create, with nothing disabling "Create" — a second click fired @@ -284,15 +316,85 @@ export const WizardForm: React.FC = ({ return []; }, [currentStep, totalSteps, schema.sections, buildSectionFields]); + /** + * Required fields the record does not satisfy, grouped by the STEP that + * declares them. + * + * react-hook-form only validates the fields currently MOUNTED, and a wizard + * mounts one step at a time. Sequential navigation is safe (Next validates the + * step you are leaving), but `allowSkip` jumps straight to any step — so a + * required field on a step nobody opened was never registered, never + * validated, and simply absent from the create payload, with nothing on screen + * saying so. The final submit therefore re-checks the WHOLE declared field set + * here. + * + * Uses the canonical rule engine (`resolveFieldRuleState`), the same one the + * form renderer and the server's rule-validator use, so a conditionally + * required/hidden field gets the same verdict from all three rather than a + * second, divergent dialect. + */ + const missingRequiredByStep = useCallback( + (record: Record): Map => { + const out = new Map(); + schema.sections.forEach((section, index) => { + for (const field of buildSectionFields(section)) { + const name = field?.name; + if (!name || (field as any).hidden === true) continue; + const state = resolveFieldRuleState( + { + visibleWhen: (field as any).visibleWhen, + readonlyWhen: (field as any).readonlyWhen, + requiredWhen: (field as any).requiredWhen, + }, + record, + { required: !!field.required, readonly: (field as any).readonly === true }, + ); + // A hidden or read-only field is not the user's to fill in. + if (!state.visible || state.readonly || !state.required) continue; + if (!isEmptyValue(record[name])) continue; + out.set(index, [...(out.get(index) ?? []), (field as any).label || name]); + } + }); + return out; + }, + [schema.sections, buildSectionFields], + ); + // Handle step data collection (merge partial data into formData) const handleStepSubmit = useCallback(async (stepData: Record) => { const mergedData = { ...formData, ...stepData }; setFormData(mergedData); - + // Mark step as completed setCompletedSteps(prev => new Set(prev).add(currentStep)); + // This step just cleared its own validation, so drop any marker it carried. + setInvalidSteps(prev => { + if (!prev.has(currentStep)) return prev; + const next = new Set(prev); + next.delete(currentStep); + return next; + }); if (isLastStep) { + // Gate the submit on the FULL field set, not just this step's (see + // missingRequiredByStep) — then point the user at the first step that is + // short something, instead of letting the server answer with a 400 that + // names fields the user cannot even see. + const missing = missingRequiredByStep(mergedData); + if (missing.size > 0) { + setInvalidSteps(new Set(missing.keys())); + const labels = [...missing.values()].flat(); + const MAX = 3; + toast.error( + t('wizard.missingRequired', { + fields: labels.slice(0, MAX).join(', ') + (labels.length > MAX ? '…' : ''), + }), + ); + goToStep(Math.min(...missing.keys())); + return; + } + setInvalidSteps(new Set()); + // Final submission setSubmitting(true); try { @@ -371,7 +473,7 @@ export const WizardForm: React.FC = ({ // Move to next step goToStep(currentStep + 1); } - }, [formData, currentStep, isLastStep, schema, dataSource]); + }, [formData, currentStep, isLastStep, schema, dataSource, missingRequiredByStep, t]); // Navigation const goToStep = useCallback((step: number) => { @@ -431,9 +533,17 @@ export const WizardForm: React.FC = ({ } const currentSection = schema.sections[currentStep]; + const clampCol = (n: unknown): number | undefined => + typeof n === 'number' && n > 0 ? Math.min(Math.floor(n), 4) : undefined; + const stepGridColumns = clampCol(schema.columns) ?? clampCol(currentSection?.columns) ?? 1; + const stepFieldContainerClass = containerGridColsFor(stepGridColumns); return ( -
+ // `@container`: the step's field grid is sized with container queries + // (`@md:grid-cols-2` …), which need a container ancestor to resolve against — + // without one the classes are inert and a multi-column step silently stayed + // single-column. Same wrapper TabbedForm / SplitForm carry. +
{/* Step Indicator */} {schema.showStepIndicator !== false && (