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

Binder should always validate after field's internal validation #8242

Closed
pekam opened this issue May 5, 2020 · 4 comments · Fixed by #13940
Closed

Binder should always validate after field's internal validation #8242

pekam opened this issue May 5, 2020 · 4 comments · Fixed by #13940

Comments

@pekam
Copy link
Contributor

pekam commented May 5, 2020

Currently Binder validates fields on ValueChangeEvent, because the valid/invalid -state is based only on the field's server-side value.

However, some fields should be also validated when the field is blurred and only the presentation value has changed. For example, when entering "asdf" to an empty DatePicker, the server-side value remains null, but the validation-state should change from valid to invalid. Same applies to eg. non-parseable numbers entered in number fields. This kind of validation should be implemented by the components themselves, but Binder should also re-validate, so that all validators are effective.

For more details, see: https://github.com/vaadin/vaadin-date-picker-flow/issues/223#issuecomment-623386532

Possible solutions:

  • Make Binder validate also on blur, if field implements BlurNotifier (although academically incorrect)
  • Add an event fired by HasValidator fields to notify Binder that it should validate

This issue would be also good to get fixed: https://github.com/vaadin/flow/issues/8241

One more thing to consider is the inconsistency between Java and web component users:
The UX for web components has been designed so that "visiting" (focus and blur even without changing value) is considered "enough" to trigger validation (https://github.com/vaadin/vaadin-text-field-flow/issues/193#issuecomment-502631352).
Java forms and fields on the other hand validate only on value changes, which can mean many situations based on the ValueChangeMode, but it never means visiting a field without changing anything.
This inconsistency might become more disturbing with Vaadin 15+, where a single application may contain views written in Java, and others created with TypeScript.

@Legioth
Copy link
Member

Legioth commented May 11, 2020

To my understanding, this is only relevant in the case when a field component has a "default validator" which would always be used by Binder in addition to the user-configured validators.

Rather than reacting to every blur event, it might be more appropriate to introduce a new event that is fired by components with a default validator whenever their internal validation status changes.

@pekam pekam changed the title Binder: validate fields also on blur Binder should always validate after field's internal validation May 27, 2020
@pekam
Copy link
Contributor Author

pekam commented Jun 1, 2020

I've changed the ticket title and description from "Binder should validate on blur" to be more generic, focusing on the problem and leaving the solution to be decided.

@mvysny
Copy link
Member

mvysny commented Apr 7, 2022

Add an event fired by HasValidator fields to notify Binder that it should validate

👍 . Having HasValidator.addValidityChangedListener() would simplify the solution at vaadin/flow-components#2176 (comment) a lot - I would be able to remove the validationChangedListener (given that there's a good way to fire those listeners).

@mshabarov mshabarov moved this from Product backlog to Ready To Go in OLD Vaadin Flow ongoing work (Vaadin 10+) May 19, 2022
@taefi taefi self-assigned this Jun 8, 2022
@taefi taefi moved this from Ready To Go to In progress in OLD Vaadin Flow ongoing work (Vaadin 10+) Jun 8, 2022
@taefi taefi moved this from In progress to Iteration Reviews in OLD Vaadin Flow ongoing work (Vaadin 10+) Jun 14, 2022
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Iteration Reviews to Done - pending release Jun 16, 2022
mshabarov pushed a commit that referenced this issue Jun 17, 2022
#13940) (CP: 23.1) (#13993)

Adds the needed API to notify Binder about validation status changes that happen in field components.

Fixes #8242
Related to vaadin/flow-components#1158
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.1.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment