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

NumberField: setting min enables validation for step #420

Closed
pekam opened this issue Oct 7, 2019 · 1 comment · Fixed by #421
Closed

NumberField: setting min enables validation for step #420

pekam opened this issue Oct 7, 2019 · 1 comment · Fixed by #421
Assignees
Labels
bug Something isn't working

Comments

@pekam
Copy link
Contributor

pekam commented Oct 7, 2019

Steps to reproduce:

  1. Create a simple <vaadin-number-field> without any attributes/properties
  2. Set the value to 1.5 (notice that the field is valid)
  3. Set min=0 for the field

Expected outcome: the field is still valid

Actual outcome: the field becomes invalid

When the step is not explicitly set by user, it's used just for the plus and minus buttons, but not for validation. Setting the step property enables the step-based validation, but it's unexpected that setting a min-constraint does the same.

These lines reveal the issue:

if (this.min !== undefined || this.max !== undefined || this.step !== 1) {
this.invalid = !this.inputElement.checkValidity();
}

I suggest that instead of asking the native input element to validate the field, add the trivial implementation of min/max-validation here. Only if the step has changed, validate via the native input.

Another problem is that setting step=1 doesn't trigger the step-based validation, but step="1" does. This logic should be based on whether user has set the property, not based on its absolute value.

@pekam pekam added the bug Something isn't working label Oct 7, 2019
@pekam
Copy link
Contributor Author

pekam commented Oct 7, 2019

I suggest that instead of asking the native input element to validate the field, add the trivial implementation of min/max-validation here. Only if the step has changed, validate via the native input.

Another way to fix this: set this.inputElement.step = 'any' when setting the initial this.step = 1. When user overrides the step, propagate it to inputElement as usual.
See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/number#step

pekam added a commit that referenced this issue Oct 7, 2019
setting the min or max property should not trigger step-based validation

fix #420
pekam added a commit that referenced this issue Oct 7, 2019
setting the min or max property should not trigger step-based validation

fix #420
@pekam pekam self-assigned this Oct 7, 2019
pekam added a commit that referenced this issue Oct 7, 2019
setting the min or max property should not trigger step-based validation

fix #420
pekam added a commit that referenced this issue Oct 25, 2019
setting the min or max property should not trigger step-based validation

fix #420
pekam added a commit that referenced this issue Oct 28, 2019
setting the min or max property should not trigger step-based validation

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

Successfully merging a pull request may close this issue.

1 participant