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.

Sign up for GitHub

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

Jump to bottom

Binder.setAsRequiredEnabled instantly validates the field #17515

Closed
felix-office opened this issue Aug 29, 2023 · 1 comment · Fixed by #18535
Closed

Binder.setAsRequiredEnabled instantly validates the field #17515

felix-office opened this issue Aug 29, 2023 · 1 comment · Fixed by #18535
Labels
binder bug Impact: High Released with Vaadin 24.3.4 Severity: Major UX

Comments

@felix-office
Copy link

Description of the bug

Using setAsRequiredEnabled instantly validates the bound field. As this leads to a pretty bad UX/UI this is a bug for me.

I have a checkbox. When the checkbox is checked, a text field is visible, which in this case is required. When the checkbox is unchecked, the text field is no longer required, so the asRequired state must be changed. Otherwise the form will be invalid on a hidden text field. I'm using setAsRequiredEnabled, which means that when the checkbox is checked, the text field is immediately invalid and red.

As the user hasn't even had a chance to enter anything, this is bad UX. I don't see why the validation has to be immediate.

Workaround:
Adding and removing the whole binder.

Expected behavior

Validation is not performed immediately.

Minimal reproducible example

I won't provide an reproducible example here as you can see it directly in the source code:

flow/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java

Lines 1530 to 1541 in 49d68c7

@Override
public void setAsRequiredEnabled(boolean asRequiredEnabled) {
if (!asRequiredSet) {
throw new IllegalStateException(
"Unable to toggle asRequired validation since "
+ "asRequired has not been set.");
}
if (asRequiredEnabled != isAsRequiredEnabled()) {
field.setRequiredIndicatorVisible(asRequiredEnabled);
validate();
}
}

Versions

Vaadin: 24.1.2
Flow: 24.1.3
Java: Amazon.com Inc. 17.0.8.1
OS: amd64 Linux 6.2.0-26-generic
Browser: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/115.0.0.0 Safari/537.36
Live reload: Java active (Spring Boot Devtools): Front end active
@caalador caalador added the UX label Aug 31, 2023
@caalador caalador moved this to 🔖 Normal Priority (P2) in Vaadin Flow bugs & maintenance (Vaadin 10+) Sep 20, 2023
@mshabarov mshabarov moved this to 🅿️Parking lot - under consideration in Vaadin Flow ongoing work (Vaadin 10+) Sep 28, 2023
This was referenced Jan 24, 2024
Closed
Closed
Merged
@github-project-automation github-project-automation bot moved this from 🔖 Normal Priority (P2) to ✅ Closed in Vaadin Flow bugs & maintenance (Vaadin 10+) Jan 26, 2024
vaadin-bot pushed a commit that referenced this issue Jan 26, 2024
@tepi @vaadin-bot
fix: Binder validation fixes (#18535)
4efe9df 
Changes:

- Once Binder.handleFieldValueChange runs for a binding when readBean was used, the whole binder will be silently validated also. BinderValidationStatusHandler is called like before (only contains status from changed binding), but StatusChangeEvent is now fired considering all bindings and if possible bean validators as well.
- Once Binder.handleFieldValueChange runs for a binding when setBean was used, doWriteIfValid will validate all bindings, not only the changed ones. This prevents writing an invalid bean in cases where one or more of the initial values are in invalid state (but not marked as such since setBean resets validation status), but they have not been changed from their initial value(s).
 Calling setAsRequiredEnabled with a changed value no longer triggers validation, since that validation is now handled elsewhere when needed as stated above.

Minor Javadoc fixes, trying to align Javadocs with code.

Fixes #18163
Fixes #4988
Fixes #17515
vaadin-bot pushed a commit that referenced this issue Jan 26, 2024
@tepi @vaadin-bot
fix: Binder validation fixes (#18535)
e35a3e9 
Changes:

- Once Binder.handleFieldValueChange runs for a binding when readBean was used, the whole binder will be silently validated also. BinderValidationStatusHandler is called like before (only contains status from changed binding), but StatusChangeEvent is now fired considering all bindings and if possible bean validators as well.
- Once Binder.handleFieldValueChange runs for a binding when setBean was used, doWriteIfValid will validate all bindings, not only the changed ones. This prevents writing an invalid bean in cases where one or more of the initial values are in invalid state (but not marked as such since setBean resets validation status), but they have not been changed from their initial value(s).
 Calling setAsRequiredEnabled with a changed value no longer triggers validation, since that validation is now handled elsewhere when needed as stated above.

Minor Javadoc fixes, trying to align Javadocs with code.

Fixes #18163
Fixes #4988
Fixes #17515
vaadin-bot added a commit that referenced this issue Jan 26, 2024
@vaadin-bot @tepi
fix: Binder validation fixes (#18535) (#18544)
cddc27a 
Changes:

- Once Binder.handleFieldValueChange runs for a binding when readBean was used, the whole binder will be silently validated also. BinderValidationStatusHandler is called like before (only contains status from changed binding), but StatusChangeEvent is now fired considering all bindings and if possible bean validators as well.
- Once Binder.handleFieldValueChange runs for a binding when setBean was used, doWriteIfValid will validate all bindings, not only the changed ones. This prevents writing an invalid bean in cases where one or more of the initial values are in invalid state (but not marked as such since setBean resets validation status), but they have not been changed from their initial value(s).
 Calling setAsRequiredEnabled with a changed value no longer triggers validation, since that validation is now handled elsewhere when needed as stated above.

Minor Javadoc fixes, trying to align Javadocs with code.

Fixes #18163
Fixes #4988
Fixes #17515

Co-authored-by: Teppo Kurki <[email protected]>
vaadin-bot added a commit that referenced this issue Jan 26, 2024
@vaadin-bot @tepi
fix: Binder validation fixes (#18535) (#18543)
d8d9ca7 
Changes:

- Once Binder.handleFieldValueChange runs for a binding when readBean was used, the whole binder will be silently validated also. BinderValidationStatusHandler is called like before (only contains status from changed binding), but StatusChangeEvent is now fired considering all bindings and if possible bean validators as well.
- Once Binder.handleFieldValueChange runs for a binding when setBean was used, doWriteIfValid will validate all bindings, not only the changed ones. This prevents writing an invalid bean in cases where one or more of the initial values are in invalid state (but not marked as such since setBean resets validation status), but they have not been changed from their initial value(s).
 Calling setAsRequiredEnabled with a changed value no longer triggers validation, since that validation is now handled elsewhere when needed as stated above.

Minor Javadoc fixes, trying to align Javadocs with code.

Fixes #18163
Fixes #4988
Fixes #17515

Co-authored-by: Teppo Kurki <[email protected]>
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.3.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
binder bug Impact: High Released with Vaadin 24.3.4 Severity: Major UX
Projects
OLD Vaadin Flow bugs & maintenance (Vaadin 10+)
Status: Closed
Vaadin Flow bugs & maintenance (Vaadin 10+)
Status: ✅ Closed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Validation fixes vaadin/flow
Validation fixes vaadin/flow
Validation fixes vaadin/flow
4 participants
@caalador @vaadin-bot @mshabarov @felix-office