-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fix form reinitialization with formik #26292
Conversation
This reverts commit cb88b24.
Codecov ReportBase: 64.28% // Head: 64.30% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #26292 +/- ##
==========================================
+ Coverage 64.28% 64.30% +0.01%
==========================================
Files 3148 3148
Lines 92202 92204 +2
Branches 11699 11699
==========================================
+ Hits 59270 59289 +19
+ Misses 28242 28219 -23
- Partials 4690 4696 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I'd vote for adding some sort of a useForm
hook that'd let us access FormContext
|
||
const useFormErrorMessage = (): string | undefined => { | ||
const { values, errors } = useFormikContext(); | ||
const { status, message } = useFormState(); | ||
const { status, message } = useContext(FormContext); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we maybe add another hook like useForm
that'd basically return useContext(FormContext);
? So there's no need to carry over the context here and there all the time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed - added a hook called useFormContext
. Decided to use the same naming convention the formik does, i.e. useFormikContext
setState({ status: "pending" }); | ||
await onSubmit(data, helpers); | ||
helpers.setStatus({ status: "fulfilled" }); | ||
setState({ status: "fulfilled" }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Epic #26178
Currently the submit button doesn't show
Success
message whenenableReinitialize
is passed toFormik
. This happens becauseFormik
resets the internalstatus
field used for submit status tracking. This PR fixes the issue by moving the submit state to its own context.How to test:
Update
Success
and becomes disabled after submit