-
-
Notifications
You must be signed in to change notification settings - Fork 493
fix(form-core): canSubmitWhenInvalid not allowing form submission #1632
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
base: main
Are you sure you want to change the base?
Conversation
ffa25a4
to
a1f4416
Compare
This might be a confusion about what |
it('should submit, when the form is invalid, with canSubmitWhenInvalid', async () => { | ||
const onSubmitFake = vi.fn(); | ||
const form = new FormApi({ | ||
defaultValues: { | ||
firstName: '', | ||
}, | ||
canSubmitWhenInvalid: true, | ||
validators: { | ||
onMount: () => { | ||
return { | ||
form: 'something went wrong', | ||
fields: { | ||
firstName: 'first name is required', | ||
}, | ||
} | ||
}, | ||
}, | ||
onSubmit: onSubmitFake | ||
}) | ||
form.mount() | ||
expect(form.state.isValid).toBe(false) | ||
await form.handleSubmit() | ||
expect(onSubmitFake).toHaveBeenCalled(); | ||
}) | ||
|
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.
This is not intended behaviour. However, it may hint at the documentation for canSubmitWhenInvalid
needing more information. If you want to change the PR to be that instead, I'm happy to review that instead.
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.
I’m happy for you to do that
Ive rejigged our form so I don’t necessarily need this feature now
But I did notice there’s an onSubmitInvalid option
Could also be worth adjusting the new test to make sure that’s called irregardless of the canSubmitWhenInvalid (I could be wrong but I don’t think I saw any tests for that)
im happy to adjust that test on Monday if needed
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.
Sure, that sounds good!
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.
I’ve reverted the code now and added the test
it would only trigger the onSubmitInvalid callback when the canSubmitWhenInvalid is true so it makes sure that’s works
ill leave it in your court now to add more documentation if you want or any changes to the or you would like
… form submission" This reverts commit a1f4416.
…n canSubmitWhenInvalid is true
View your CI Pipeline Execution ↗ for commit 1fde4b6
☁️ Nx Cloud last updated this comment at |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1632 +/- ##
==========================================
+ Coverage 89.57% 90.37% +0.79%
==========================================
Files 34 36 +2
Lines 1497 1621 +124
Branches 371 385 +14
==========================================
+ Hits 1341 1465 +124
Misses 139 139
Partials 17 17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.