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

[number-field] The field is not validated when trying to commit a non-parsable value #4258

Closed
vursen opened this issue Jul 21, 2022 · 4 comments · Fixed by #4268
Closed

[number-field] The field is not validated when trying to commit a non-parsable value #4258

vursen opened this issue Jul 21, 2022 · 4 comments · Fixed by #4268
Labels
bug Something isn't working vaadin-number-field

Comments

@vursen
Copy link
Contributor

vursen commented Jul 21, 2022

Description

When trying to commit a value that is not a valid number, validation doesn't happen.

The same goes for integer-field which is an extension of number-field.

Expected outcome

Validation should fail when the user tries to commit a non-parsable number into number-field or integer-field.

Minimal reproducible example

<script type="module">
  import '@vaadin/number-field';
  import '@vaadin/integer-field';
</script>

<vaadin-number-field label="Number Field"></vaadin-number-field>
<vaadin-integer-field label="Integer Field"></vaadin-integer-field>

Steps to reproduce

  1. Add the above snippet to an HTML page.
  2. Enter 12-- into either field and press Enter.

Environment

Vaadin version(s): 23.2
OS: Mac OS

Browsers

Issue is not browser related

@vursen vursen added the bug Something isn't working label Jul 21, 2022
@vursen
Copy link
Contributor Author

vursen commented Jul 21, 2022

The issue is caused by the fact of using input[type=number]. When you manage to enter something invalid into a number input, the browser resets the input's value to an empty string. This is native behavior that we apparently cannot control.

Related to #3102

@vursen
Copy link
Contributor Author

vursen commented Jul 21, 2022

UPD: After more investigation, the problem sounds like:

Validation doesn't happen on invalid user input because the input's value doesn't change = neither input nor change event is fired.

In other words, when the field already has an empty value, entering something invalid doesn't cause the input's value property to change because of the browser's native behavior mentioned above. Since the component doesn't receive any events, obviously nothing triggers the component's validation.

A possible solution might be to find a way to subscribe to native validation (an :invalid change) and trigger our validation whenever native validity changes to keep them in sync.

@web-padawan
Copy link
Member

Similar issue: #1263 - apparently the logic in this line isn't working well in such cases:

// Validate value to be numeric
if (newVal && isNaN(parseFloat(newVal))) {

We could use isNaN(Number(newVal)) instead, as described here.

@vursen vursen changed the title [number-field] The field doesn't get invalid when trying to commit a non-parsable value [number-field] The field is not validated when trying to commit a non-parsable value Jul 21, 2022
@vursen
Copy link
Contributor Author

vursen commented Jul 21, 2022

UPD: I apologize for so many metamorphoses in my conclusions of the root cause. I believe this is the final one. :)

Fact 1: I checked on the events on the input and both input and change are fired as usual, regardless of what has been typed in the input.

Fact 2: Validation of invalid input is only missing when no constraints are specified.

The last fact sheds light on everything. The cause is InputConstraintsMixin which doesn't validate the field when no constraints. It was missed that, for some components, validation also verifies whether the inputted value is parseable and hence it should be run regardless of constraints to be able to report bad input on all occasions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working vaadin-number-field
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants