-
Notifications
You must be signed in to change notification settings - Fork 148
fix: reset form in stepper on each step #3722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Merge activity
|
### TL;DR Added form reset functionality when navigating between steps and added debugging for form state. ### What changed? - Added `form.reset()` with options to keep values but clear errors when moving to the next step in the stepper form - Added a console log to display the form state in the `StepPanel` component ### How to test? 1. Navigate through a stepper form and verify that errors are cleared when moving to the next step 2. Confirm that form values are preserved when navigating between steps 3. Check the browser console to see the form state being logged ### Why make this change? This change improves the user experience by clearing validation errors when users move to the next step while preserving their input values. The console log helps with debugging form state during development.
Graphite Automations"Test" took an action on this PR • (01/01/26)1 assignee was added to this PR based on Kacper Wojciechowski's automation. |
Code ReviewThank you for this PR! I've reviewed the changes and have some feedback on the implementation. Issues Found1. Critical: Console.log left in production codeLocation: frontend/src/app/forms/stepper-form.tsx:183 This debug log should be removed before merging. Console logs in production code can leak sensitive form data to browser consoles, create unnecessary performance overhead, and clutter the console for end users. Fix: Remove the console.log(form.formState) line entirely. 2. Logic Issue: Form reset behavior may not work as expectedLocation: frontend/src/app/forms/stepper-form.tsx:115-118 According to React Hook Form documentation, calling form.reset(undefined, { keepValues: true }) creates a logical contradiction. When you pass undefined as the first argument, you're telling the form to reset to its defaultValues. The keepValues: true option then says keep the current values. Expected behavior: This will likely reset the form to defaultValues and ignore keepValues: true. Suggested fix: Use form.clearErrors() instead to just clear errors while keeping values intact. 3. Consistency: Same pattern already exists in Previous buttonLocation: frontend/src/app/forms/stepper-form.tsx:195-198 The Previous button already uses the same reset pattern. This suggests the pattern should be reconsidered for both locations. Testing Recommendations
Summary
Please address these issues and I'll be happy to review again! |
f7b2779 to
ccfd423
Compare
PR ReviewSummaryThis PR adds form reset functionality when navigating between steps in the stepper form. The change clears validation errors while preserving form values, improving the user experience. Code Quality & Best Practices✅ Positive aspects:
Issues & Concerns
Recommended fix: const handleSubmit = (values: z.infer<JoinStepSchemas<Steps>>) => {
ref.current = { ...ref.current, ...values };
if (stepper.isLast) {
return onSubmit?.({ values: ref.current, form, stepper });
}
stepper.next();
form.reset(ref.current, {
keepErrors: false,
keepValues: true,
});
};This ensures the form is reset with the accumulated values from all previous steps, maintaining consistency.
Testing RecommendationsThe PR description provides basic testing steps, but consider adding:
Minor Style IssueThe extra blank line added at line 182 in VerdictThe core idea of this PR is sound and improves UX. However, the implementation should pass Recommendation: Request changes to fix the form reset argument. |
### TL;DR Added form reset functionality when navigating between steps and added debugging for form state. ### What changed? - Added `form.reset()` with options to keep values but clear errors when moving to the next step in the stepper form ### How to test? 1. Navigate through a stepper form and verify that errors are cleared when moving to the next step 2. Confirm that form values are preserved when navigating between steps 3. Check the browser console to see the form state being logged ### Why make this change? This change improves the user experience by clearing validation errors when users move to the next step while preserving their input values. The console log helps with debugging form state during development.
More templates
@rivetkit/cloudflare-workers
@rivetkit/db
@rivetkit/framework-base
@rivetkit/next-js
@rivetkit/react
rivetkit
@rivetkit/sql-loader
@rivetkit/engine-runner
@rivetkit/engine-runner-protocol
commit: |

TL;DR
Added form reset functionality when navigating between steps and added debugging for form state.
What changed?
form.reset()with options to keep values but clear errors when moving to the next step in the stepper formHow to test?
Why make this change?
This change improves the user experience by clearing validation errors when users move to the next step while preserving their input values. The console log helps with debugging form state during development.