diff --git a/.changeset/lucky-pandas-attend.md b/.changeset/lucky-pandas-attend.md new file mode 100644 index 00000000..156465fd --- /dev/null +++ b/.changeset/lucky-pandas-attend.md @@ -0,0 +1,21 @@ +--- +"@tailor-platform/app-shell": minor +--- + +Let forms be driven natively: `Form` now accepts `id`, and `Select`, `Combobox`, and `Autocomplete` accept `name`, `form`, `required`, and `inputRef` (plus `itemToStringValue` on `Select` and `Combobox`). These props were already supported by the underlying Base UI roots but were filtered out by the wrapper `Pick<>` types, so there was no way to reach them. + +**`Form` `id`** — a submit button rendered outside the form can now target it with the native `form` attribute, which is what the common "Save in the page header, fields in the body" layout needs: + +```tsx +Save]} +/> +
…
+``` + +**`name` on the dropdowns** — Base UI renders a hidden input under that name, so the selected value is now visible to native submission: `new FormData(form)`, an uncontrolled `
`, and server actions. Previously these controls contributed nothing to the DOM payload, so a native form silently submitted without them. + +For non-string items, `itemToStringValue` controls serialisation (items shaped `{ value, label }` use `value` automatically). It is not available on `Combobox`'s creatable variants, which derive it internally so the pending-item sentinel serialises correctly. + +Note this is a _separate_ mechanism from `Form`'s `onFormSubmit`, which collects values from registered `Field.Root`s keyed by the field's `name` — that path already worked without `name` on the control and is unchanged. Consequently, **inside a `Field.Root` the field's `name` wins and the control's own `name` is ignored**; set it only when the control is used outside a `Field.Root`. diff --git a/.changeset/tidy-pianos-explain.md b/.changeset/tidy-pianos-explain.md new file mode 100644 index 00000000..6afbda1e --- /dev/null +++ b/.changeset/tidy-pianos-explain.md @@ -0,0 +1,11 @@ +--- +"@tailor-platform/app-shell": patch +--- + +Fix the bundled `app-shell-patterns` skill, whose form guidance contradicted the package. `components.md` described `Form` as "wired to react-hook-form" and `Field` as binding "to react-hook-form via `name`" — neither is true. `Form`/`Field`/`Fieldset` wrap Base UI and own accessibility wiring and visual state only; `react-hook-form` stopped being a runtime dependency in 1.4.0. Meanwhile every `form/*` reference implementation ignored `Form` entirely and hand-rolled `` + `new FormData(...)`, which skips validation and server-error routing — while the skill's own rules say to use AppShell components over raw HTML. + +The four `form/*` patterns now use `Form` with `onFormSubmit`, and document the model they implement: `onFormSubmit` collects values from registered `Field.Root`s rather than reading `FormData`, so **every** control — `Select`, `Combobox` and `Autocomplete` included — participates simply by being wrapped in a `Field.Root name="…"`. No `name` on the control, no `useState`, no merging in the submit handler. + +Documents two things that were previously undiscoverable: an object-valued dropdown submits as a JSON string unless `itemToStringValue` is supplied (items shaped `{ value, label }` use `value` automatically), and a page-header Save reaches a body form by matching `Form`'s `id` with a detached `]} +/> +…
+``` + +**Submit:** use `onFormSubmit(values)` — it receives parsed form values and is the default. Use the +low-level `onSubmit` **only** when handing the native event to React Hook Form's `handleSubmit`. +**Never** hand-roll `
new FormData(e.currentTarget)}>` — that skips validation +and error routing. +**Server errors:** pass `errors={{ fieldName: "message" }}`; they route to the matching +`Field.Error` and clear when the user edits that field. **Example:** see `form/single-page.md`. **Used in patterns:** all `form/*`. @@ -626,9 +654,71 @@ const table = useDataTable({ **Import:** `import { Field } from '@tailor-platform/app-shell'` **Purpose:** Single form field with label + control + error message wiring. -**API:** Compound. Wraps any input control (`Input`, `Select`, `Combobox`, etc.) and binds it to react-hook-form via `name`. +**API:** Compound — `Field.Root`, `Field.Label`, `Field.Control`, `Field.Description`, `Field.Error`, +`Field.Validity`. `Field.Root` establishes a context boundary; every Base UI-backed AppShell control +placed inside it inherits label association, `aria-describedby`, `disabled`, and invalid state +automatically. +**`Field.Control` is already a styled input** — write ``, not +`} />`. `Input` and `Field.Control` share the same base classes, so +the `render` form just double-wraps. Reach for `Input` directly when you need a control outside a +`Field.Root`, or when the value is React-controlled. +**RHF interop (optional):** `Field.Root` accepts `isTouched`, `isDirty`, `invalid`, and `error`, +matching RHF's `fieldState` shape — so `` works with no adapter. **Used in patterns:** all `form/*`. +### How values reach your submit handler + +There are three separate mechanisms. Picking the wrong mental model is the most common +form mistake, so be explicit about which one a screen uses. + +**1. `Form` + `Field.Root` → `onFormSubmit` (the default).** `onFormSubmit` does **not** read +`FormData`; it collects values from the `Field.Root`s registered inside the `Form`, keyed by each +field's `name`. Every AppShell control works this way once wrapped in a `Field.Root name="…"` — +including `Select`, `Combobox`, and `Autocomplete`. They need **no `name` of their own and no React +state**: + +```tsx + save(values)}> + + Category + - - - + + + + + + Price + + Price is required. + + + Currency + + + + + + - - Inventory -
- - Initial quantity - } /> - - - Reorder point - } /> - -
- - + + + + + + Initial quantity + + Initial quantity is required. + + + Reorder point + + + Leave blank to disable replenishment alerts. + + + {/* + * Object items would otherwise reach `onFormSubmit` as JSON. + * `itemToStringValue` picks the field to submit; `mapItem` + * stays responsible for what the user sees. + */} + + Default warehouse + ({ label: w.name, key: String(w.id) })} + itemToStringValue={(w) => String(w.id)} + placeholder="Select warehouse" + /> + + + + + ); diff --git a/catalogue/src/pattern/form/single-page/PATTERN.md b/catalogue/src/pattern/form/single-page/PATTERN.md index 48982e70..a58dbf89 100644 --- a/catalogue/src/pattern/form/single-page/PATTERN.md +++ b/catalogue/src/pattern/form/single-page/PATTERN.md @@ -4,7 +4,7 @@ name: Single Page Form category: pattern subcategory: form description: Routed full-page form for moderate field count (6-15) without natural sectioning -requiredImports: [Layout, Form, Field, Fieldset, Input, Select, Button] +requiredImports: [Layout, Form, Field, Select, Button] tags: [form, page, create, edit, routed] do: - A routed Create or Edit page that the design has explicitly called out (e.g. /orders/create) @@ -31,6 +31,20 @@ dont: - Single column full width below 1024px; single column max-w constrained at 1024–1280px - Without an explicit routed-page requirement, the answer is `form/modal` - A `/create` or `/edit` route in the screen spec does NOT require a full-page replacement +- Save/Cancel belong in `Layout.Header`, wired with `, - , ]} /> -
{ - e.preventDefault(); - const formData = new FormData(e.currentTarget); - const entries: Record = {}; - formData.forEach((value, key) => { - entries[key] = value as string; - }); - onSave(entries); - }} - className="space-y-4 max-w-2xl" + + id="product-form" + noValidate + className="max-w-2xl space-y-4" + onFormSubmit={(values) => onSave(values)} > Name - } /> + + Name is required. SKU - } /> + + Format: ABC-1234 + Use the format ABC-1234. + SKU is required. + {/* + * Dropdowns need no `name` and no React state: `Field.Root` registers + * the control, so its value arrives in `onFormSubmit` under the + * field's name like any other input. + */} Category - + Price - } /> + + Price cannot be negative. + Price is required. Description - } /> + - +
); diff --git a/catalogue/src/pattern/form/wizard/PATTERN.md b/catalogue/src/pattern/form/wizard/PATTERN.md index 77b26ce4..16b34b81 100644 --- a/catalogue/src/pattern/form/wizard/PATTERN.md +++ b/catalogue/src/pattern/form/wizard/PATTERN.md @@ -4,15 +4,16 @@ name: Wizard Form category: pattern subcategory: form description: Multi-stage create flow with 3-7 steps and per-step validation gates -requiredImports: [Layout, Card, Form, Fieldset, Field, Input, Select, Badge, Button] -tags: [form, wizard, multi-step, import, stepper] +requiredImports: [Layout, Card, Badge, Form, Fieldset, Field, Select, Button] +tags: [form, wizard, multi-step, stepper, onboarding] do: - Multi-stage Create with 3-7 steps - - Import flows (upload → map → validate → confirm) + - Onboarding and request flows where users should focus on one step at a time - Per-step validation gates progression dont: - Single screen of fields — use form/modal or form/single-page - More than 7 steps — split into separate routed pages or reduce scope + - CSV/spreadsheet import — use the CsvImporter component, not a hand-built wizard --- # pattern/form/wizard @@ -20,9 +21,12 @@ dont: ## When to Use - Multi-stage Create with 3–7 steps -- Import flows (upload → map → validate → confirm) +- Onboarding and request flows where users should focus on one step at a time - Per-step validation gates progression +For CSV/spreadsheet import specifically, use the `CsvImporter` component — it already implements +the upload → map → validate → confirm flow. Don't rebuild it here. + ## Page Implementation @@ -33,9 +37,32 @@ dont: - Back-navigation must preserve prior step's input - Validation must be per-step — don't defer until final submit - Step indicator collapses to "Step 2 of 4" label below 1024px +- One `
` rendered per step, keyed by step index so it remounts cleanly +- Accumulated values live in a `draft` state object above the `Form`; each step's fields read + their initial value from it via `defaultValue` + +## Form state + +Rules that apply to every `form/*` pattern are in **`components.md`** → Forms, with a worked example +in **`form/modal`** → Form state: `Form` + `Field` is the default stack, submit via `onFormSubmit`, +dropdowns need only a wrapping `Field.Root` (no `name`, no state), server errors route through +`errors`, and React Hook Form is an optional escape hatch. + +The wizard's specific mechanic: **make "Next" a `type="submit"` button.** `onFormSubmit` fires only +after the current step's fields pass validation, so progression is gated natively — no manual +validity check, and no deferring errors to the final submit. The handler merges that step's values +into `draft` and advances; on the last step it calls the completion callback instead. + +Because each step's `Form` unmounts on navigation, values must be lifted into `draft` — that is +what makes Back non-destructive. Every field, dropdowns included, is uncontrolled and re-reads its +prior value from `draft` via `defaultValue`; nothing needs an `onChange`. ## Anti-patterns - More than 7 steps — users lose context and abandon - No back-navigation preservation — pressing Back loses prior step's input - Validation deferred until final submit — failures force full re-traversal +- A plain `onClick` "Next" that advances without validating — use `type="submit"` and let + `onFormSubmit` gate it +- Wrapping all steps in one `` and hiding inactive ones — hidden required fields block submit +- Hand-building a CSV import wizard — use `CsvImporter` diff --git a/catalogue/src/pattern/form/wizard/wizard-form.tsx b/catalogue/src/pattern/form/wizard/wizard-form.tsx index bb01c4e6..5a3225ba 100644 --- a/catalogue/src/pattern/form/wizard/wizard-form.tsx +++ b/catalogue/src/pattern/form/wizard/wizard-form.tsx @@ -1,93 +1,154 @@ /* pattern: form/wizard */ import { useState } from "react"; -import { Button, Card, Layout, Badge, Input, Field } from "@tailor-platform/app-shell"; +import { + Badge, + Button, + Card, + Field, + Fieldset, + Form, + Layout, + Select, +} from "@tailor-platform/app-shell"; -const STEPS = ["Upload", "Map", "Review", "Done"] as const; +const OWNERS = ["Tanaka", "Sato", "Suzuki", "Yamada"]; +const STEPS = ["Basic info", "Assignment", "Schedule", "Review"] as const; + +type Draft = { + title: string; + description: string; + owner: string; + startDate: string; + estimate: string; +}; + +const INITIAL: Draft = { + title: "", + description: "", + owner: "", + startDate: "", + estimate: "", +}; type Props = { - onComplete: () => void; + onComplete: (draft: Draft) => void; + onCancel: () => void; }; -export default function WizardForm({ onComplete }: Props) { - const [currentStep, setCurrentStep] = useState(0); +export default function WizardForm({ onComplete, onCancel }: Props) { + const [step, setStep] = useState(0); + // Accumulated values. Each step's Form unmounts on navigation, so the draft + // is what makes Back non-destructive — fields re-read it via `defaultValue`. + const [draft, setDraft] = useState(INITIAL); - const handleNext = () => { - if (currentStep < STEPS.length - 1) { - setCurrentStep(currentStep + 1); - } else { - onComplete(); - } - }; + const isLastStep = step === STEPS.length - 1; - const handleBack = () => { - if (currentStep > 0) { - setCurrentStep(currentStep - 1); + /** + * Each step is its own `Form`, and "Next" is a `type="submit"` button. + * `onFormSubmit` fires only after that step's fields pass validation, so + * progression is gated natively — no manual per-step validity check, and + * no deferring every error to the final submit. + */ + const handleStepSubmit = (values: Record) => { + const merged = { ...draft, ...(values as Partial) }; + setDraft(merged); + if (isLastStep) { + onComplete(merged); + return; } + setStep(step + 1); }; return ( - +
- {STEPS.map((step, i) => ( + {STEPS.map((label, i) => ( - {i + 1}. {step} + {i + 1}. {label} ))}
- - - {currentStep === 0 && ( -
- - CSV file - } /> - -
- )} - {currentStep === 1 && ( -
-

Map CSV columns to product fields

- - Name column - } /> - - - SKU column - } /> + + + + + {step === 0 && ( + + + Title + + Title is required. + + + Description + + + + )} + + {step === 1 && ( + + Owner + {/* Uncontrolled like every other field — `defaultValue` + restores the prior choice when the user steps Back. */} + - - - - - Japan - United States - - + , + ); + const input = container.querySelector('input[name="direction"]') as HTMLInputElement; + expect(input).not.toBeNull(); + expect(input.value).toBe("Up"); + }); + + it("is picked up by native FormData", () => { + const { container } = render( + + ({ label: item.label, key: item.value })} + aria-label="Country" + /> + , + ); + const form = container.querySelector("form") as HTMLFormElement; + expect(new FormData(form).get("country")).toBe("jp"); + }); + + it("serialises arbitrary object items via itemToStringValue", () => { + const warehouses = [ + { id: 7, name: "Warehouse A" }, + { id: 9, name: "Warehouse B" }, + ]; + const { container } = render( +
+ +
, + ); + const input = container.querySelector('input[name="direction"]') as HTMLInputElement; + expect(input.getAttribute("form")).toBe("outer"); + }); + + it("exposes the hidden input through inputRef", () => { + const ref = { current: null } as React.RefObject; + render( + Direction is required. +
+ + , + ); + + await user.click(screen.getByRole("button", { name: "Save" })); + + expect(onFormSubmit).not.toHaveBeenCalled(); + expect(screen.getByText("Direction is required.")).not.toBeNull(); + }); + + it("reflects the value chosen by the user", async () => { + const user = userEvent.setup(); + const { container } = render( +
+ + + +
, + ); + + expect(container.querySelector('input[name="fieldName"]')).not.toBeNull(); + expect(container.querySelector('input[name="controlName"]')).toBeNull(); + + await user.click(screen.getByRole("button", { name: "Save" })); + expect(onFormSubmit.mock.calls[0][0]).toMatchObject({ fieldName: "Up" }); + expect(onFormSubmit.mock.calls[0][0]).not.toHaveProperty("controlName"); + }); + + it("keeps working with Form + Field.Root, which reads registered fields", async () => { + const user = userEvent.setup(); + const onFormSubmit = vi.fn(); + render( +
+ + Direction +