Skip to content
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

Show validation errors added by onSubmit #1572

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

misli
Copy link

@misli misli commented Jun 29, 2021

The onSubmit action may set additional validation errors
(typically errors received from a backend server).
This change makes these errors visible.

The `onSubmit` action may set additional validation errors
(typically errors received from a backend server).
This change makes these errors visible.
Copy link
Contributor

@jelhan jelhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot.

Are you sure that it isn't overruled by finally, which sets showAllValidations to undefined? Would be great if you could add tests. Not only to verify that one but also to ensure that we don't introduce a regression later.

I'm wondering if this is the correct behavior in all cases. Or if it should be controlled via an option like hideValidationsOnSubmit. A @onSubmit handler could fail for many cases. Violating validation rules only enforced by backend is only one of them.

Similar I'm wondering if it should be considered a breaking change. In that case we would need to control it by option. And the default must match current behavior. At least until release next major. @simonihmig any thoughts on that one?

@simonihmig
Copy link
Contributor

Would be great if you could add tests.

Yes, please!

Similar I'm wondering if it should be considered a breaking change

I agree with your concerns. When not adding validation errors in onSubmit, setting showAllValidations will introduce a side effect that likely is unwanted, as all fields will be marked as successful (green), even when hideValidationsOnSubmit has been set, or some (not required) field hasn't been filled at all.

So an option like showValidationsOnRejection seems preferable probably!? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants