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

Validation refactor to fix issues related to the validated event #5537

Open
10 tasks done
vursen opened this issue Oct 3, 2023 · 4 comments
Open
10 tasks done

Validation refactor to fix issues related to the validated event #5537

vursen opened this issue Oct 3, 2023 · 4 comments
Labels
refactor Internal improvement validation

Comments

@vursen
Copy link
Contributor

vursen commented Oct 3, 2023

Motivation

There are several problems with the current approach based on the validated event:

  1. Server-side validation is triggered on blur even if nothing has been typed (#17429, #6146)
  2. Server-side validation is triggered too frequently, resulting in performance degradation (#4390).
  3. Server-side validation is triggered on client property changes, for example resulting in unexpected Binder revalidation after a bean reset (#5399, #5319, #5330, #5328).
  4. Server-side validation is triggered inconsistently for text fields: fields validate on change when valid but on input when invalid (#4986).

All these problems have emerged after the introduction of the validated event, so we need to find an alternative. We should remember that the underlying purpose of the validated event was to trigger server-side validation when the user enters bad input, as there is no change event in that case.

Solution

For components that allow entering bad input, let’s replace the validated event with a new event unparsable-change:

  • This event should be fired by the web component whenever the user attempts to commit or clear bad input. These actions usually happen on Enter, outside click or blur. However, the specific list of actions may vary depending on the component.
  • This event should be mutually exclusive with the change event to prevent sending two events to the server in one round-trip and thus rule out the possibility of duplicate server-side validation.
  • The Flow component is supposed to subscribe to this event (in addition to the change event) and trigger validation whenever it occurs.

However, switching to the new event will address (4) only partially. The validation will be still inconsistent in eager mode: text fields will validate on input when entering valid values but on change when entering bad input. To improve this, let’s additionally subscribe the text field Flow components to hasInputValue property change and trigger validation when it occurs in eager mode.

For the rest components, let's remove the validated event so that they will only validate on change event.

Affected components

  • DatePicker
  • TimePicker
  • NumberField
  • IntegerField
  • BigDecimalField
  • ComboBox
  • MultiSelectComboBox
  • Select
  • RadioGroup
  • CheckboxGroup

Note
DateTimePicker has more complex UX, which requires a more complex solution to achieve consistent validation behavior. There will be another proposal for it later.

Alternatives

Listen directly to the blur event in the Flow component and trigger validation when it’s fired.

Pros:

  • Reacts to bad input on blur.
  • Decreases the number of validation rounds on value change (3 => 2).
  • Removes other side-effects like “undesired initial validation when the field has constraints and value” (ticket)

Cons:

  • Doesn’t remove the eager validation on blur without workarounds (#6146)
  • Doesn’t address the case where the user attempts to commit bad input on Enter.
  • When closing with outside click, DatePicker tries to commit the entered value and restores focus to the input element, which prevents the blur event. This means that, in the case of bad input, you’ll need to click outside twice to get the field validated, which doesn't align with the web component's behavior.

Additional context

Related Github discussion: https://github.com/orgs/vaadin/discussions/4434

Related AC that introduced the validated event: vaadin/platform#3066

@vursen vursen changed the title Major validation refactoring Validation refactoring to address various issues Oct 3, 2023
@vursen vursen changed the title Validation refactoring to address various issues Validation refactoring to address issues related to the validated event Oct 3, 2023
@vursen vursen changed the title Validation refactoring to address issues related to the validated event Validation refactoring to fix issues related to the validated event Oct 3, 2023
@vursen vursen self-assigned this Oct 3, 2023
@TatuLund
Copy link
Contributor

TatuLund commented Oct 4, 2023

  1. Server-side validation is triggered on blur even if nothing has been typed

Note, also client side validation is triggered on blur even if nothing has been typed as seen here: vaadin/web-components#6587

@vursen
Copy link
Contributor Author

vursen commented Oct 4, 2023

Note, also client side validation is triggered on blur even if nothing has been typed as seen here: vaadin/web-components#6587

I'd like to clarify that this proposal focuses only on Flow components. It's specifically designed in a way that avoids modifying any existing behavior of the web components, such as validation on blur, for example. If you believe that the web components shouldn't validate on blur, feel free to create a separate ticket.

@vursen
Copy link
Contributor Author

vursen commented Oct 5, 2023

This refactor is planned to be delivered by 24.3-beta.

@vursen vursen added the refactor Internal improvement label Oct 5, 2023
@vursen vursen changed the title Validation refactoring to fix issues related to the validated event Validation refactor to fix issues related to the validated event Oct 5, 2023
@yuriy-fix
Copy link
Contributor

The refactor has covered all input components apart from DateTimePicker which will have a separate refactor planned.

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

No branches or pull requests

3 participants