From b01e29daf5a7c938ee91ab046b8d7abcbd951a3b Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Thu, 30 Jul 2026 15:24:37 +0800 Subject: [PATCH] fix(plugin-form): a wizard that ends on a field-less review step can finish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A wizard step with no inputs — `{ label: 'Review', fields: [] }`, the shape this component's own usage example ends on — rendered a bare hint div and NO `
`. The footer Next/Create buttons submit the step form natively (`type="submit" form={stepFormId}`), so on that step they pointed at a target that did not exist: clicking Create did nothing at all and the wizard could never be completed. Observed directly — `create` call count 0, no error, no feedback. This is fallout from the fix the regression suite guards: moving the buttons off `onClick={() => handleStepSubmit(formData)}` onto native submission is what made a missing form fatal. Render a real form on the field-less step; it contributes no values of its own, so the record is whatever the earlier steps accumulated. Also correct that suite's docblock, which overstated what it covers. Two redundant mechanisms carry values across steps (react-hook-form retains unmounted fields; the wizard merges `{...formData, ...stepData}`), and "submits the merged payload" passes when EITHER is broken alone — it catches a total failure to accumulate, not a broken merge. The new field-less-step case is what pins the merge down: with no react-hook-form instance on that step, `formData` is the only carrier left. Verified by mutation — breaking only the merge now fails with `expected {} to deeply equal {"name": "Alice"}`, and reverting the form fix fails with `expected null not to be null`. Co-Authored-By: Claude Opus 5 --- .../src/WizardForm.regression.test.tsx | 45 +++++++++++++++++++ packages/plugin-form/src/WizardForm.tsx | 23 ++++++++-- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/packages/plugin-form/src/WizardForm.regression.test.tsx b/packages/plugin-form/src/WizardForm.regression.test.tsx index 32459d2dcc..084db132b2 100644 --- a/packages/plugin-form/src/WizardForm.regression.test.tsx +++ b/packages/plugin-form/src/WizardForm.regression.test.tsx @@ -11,6 +11,18 @@ * 2. the create-mode seeding effect reset `formData` to `{}` on re-render. * The buttons now submit the inner form natively (`type="submit"` + `form={id}`) * and the create seed is idempotent. + * + * What each case here actually pins down — the first two overlap less than they + * look, so don't read either as covering the whole merge: + * - "submits the merged payload" is an END-TO-END outcome check. Two redundant + * mechanisms carry values across steps (react-hook-form keeps unmounted + * fields, since `shouldUnregister` is false; and the wizard merges + * `{...formData, ...stepData}`), and breaking EITHER ONE ALONE still passes. + * It catches a total failure to accumulate, not a broken merge. + * - "carries earlier steps through a field-less review step" is what pins the + * merge itself: a step with no inputs has no react-hook-form instance to + * retain anything, so `formData` is the only carrier left. + * - "wires the footer Next/Create buttons" pins the native-submit wiring. */ import { describe, it, expect, vi } from 'vitest'; import { render, screen, waitFor, fireEvent, act } from '@testing-library/react'; @@ -90,6 +102,39 @@ describe('WizardForm — multi-step create collects every step', () => { expect(payload).toEqual(expect.objectContaining({ name: 'Alice', note: 'hello' })); }); + it('carries earlier steps through a field-less review step (and can finish it)', async () => { + const ds = makeDS(); + // A final review/confirm step with no inputs — the shape this component's + // own usage example ends on. It renders no react-hook-form instance, so + // it is the one path where the wizard's own `formData` is the ONLY thing + // carrying the earlier steps. + const reviewSchema = { + ...schema, + sections: [{ label: 'Step 1', fields: ['name'] }, { label: 'Review', fields: [] }], + }; + const { container } = render(); + + fireEvent.change(await settledStepInput(container, 'name'), { target: { value: 'Alice' } }); + fireEvent.submit(container.querySelector('form') as HTMLFormElement); // → review step + + await waitFor(() => { + if (!screen.queryByText(/No fields configured/i)) throw new Error('not on the review step'); + }); + await act(async () => {}); + + // The review step must still expose a form for the footer button to submit; + // without one the button targets nothing and the wizard can never finish. + const createBtn = screen.getByRole('button', { name: /create/i }); + const reviewForm = container.querySelector('form'); + expect(reviewForm).not.toBeNull(); + expect(createBtn.getAttribute('form')).toBe(reviewForm!.getAttribute('id')); + + await act(async () => { fireEvent.click(createBtn); }); + await waitFor(() => expect(ds.create).toHaveBeenCalledTimes(1)); + // Step 1's value survives a step that contributed nothing of its own. + expect(ds.create.mock.calls[0][1]).toEqual(expect.objectContaining({ name: 'Alice' })); + }); + it('wires the footer Next/Create buttons to the step form (cannot bypass it)', async () => { const ds = makeDS(); const { container } = render(); diff --git a/packages/plugin-form/src/WizardForm.tsx b/packages/plugin-form/src/WizardForm.tsx index b2289929b4..54c0e674cf 100644 --- a/packages/plugin-form/src/WizardForm.tsx +++ b/packages/plugin-form/src/WizardForm.tsx @@ -537,9 +537,26 @@ export const WizardForm: React.FC = ({ }} /> ) : ( -
- No fields configured for this step -
+ // A step can legitimately have NO inputs — a final "Review"/confirm + // step is the canonical case (this component's own usage example + // ends on `{ label: 'Step 3: Review', fields: [] }`). The footer + // Next/Create buttons submit the step form *natively* by id, so + // that form has to exist even when there is nothing to fill in: + // without it the buttons point at a missing target, the click does + // nothing at all, and a wizard that ends on a review step can never + // be completed. Submitting contributes no new values — the record + // is whatever the earlier steps accumulated in `formData`. + { + e.preventDefault(); + void handleStepSubmit({}); + }} + > +
+ No fields configured for this step +
+ )} )}