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

[b-numberinput] remove fallback to minNumber if input is not a number #2945

Closed
elC0mpa opened this issue Oct 5, 2020 · 4 comments · Fixed by #2955
Closed

[b-numberinput] remove fallback to minNumber if input is not a number #2945

elC0mpa opened this issue Oct 5, 2020 · 4 comments · Fixed by #2955

Comments

@elC0mpa
Copy link
Contributor

elC0mpa commented Oct 5, 2020

Description

Inside Numberinput.vue file is the next code:

if (value === '' || value === undefined || value === null) {
                    newValue = this.minNumber || null
                }

Here is veriffied if the input value is not valid. In case it is not valid, you assign newValue to minNumber or to null. These are my points:

  1. Fisrt of all, I don't think it would be a good idea to fallback to the minNumber property. The reason si that if you want to validate the input, despite the input would be anything but a number, the validation library will accept it as a possible value because of the fallback. The next photo represents a b-numberinput which min property is set to 1 and even when there is not a valid input, the validation library accepts it, because of the fallback:
    numberinput-0
  2. In case you consider the fallback to minNumber should be kept, there is a problem when it is set to 0 and it is that this.minNumber will return as false. So the value will never fallback to minNumber. In the next image this is reflected with a b-numberinput which min property is set to 0:
    numberinput-1

Why Buefy need this feature

I think this feature will improve Buefy by making it more flexible. Validation is an important part of frontend development, so I think it will be better to fallback to minNumber in case input is not valid. By doing this, the validation library will be able to detect the incorrect user input

@jtommy
Copy link
Member

jtommy commented Oct 5, 2020

Validation is very important but why do you want to remove the current logic if it preserve you from invalid values?

@elC0mpa
Copy link
Contributor Author

elC0mpa commented Oct 5, 2020

I want to update the current logic because the user must know when the input is invalid.
numberinput-0
The normal behavior in validation is that the user must know when an input is not valid. If you don't modiffy this, it won't be possible to do that, becasue, as is reflected in the previous image, when the user inputs an invalid value, the library will take it as a valid value. In the previous image, the user input is +1, which is an invalid input, however, the validation library will validate minNumber as valid because of the fallback.
numberinput-1
The previous image shows the expected behavior, the user inputs an invalid value (5+) and the library reflects the user that there is a problem because like there is no fallback value, the library receives a null value.

@jtommy
Copy link
Member

jtommy commented Oct 6, 2020

Probably I don't understand well but you shouldn't use non and max so the user is free to set the value as want

@elC0mpa
Copy link
Contributor Author

elC0mpa commented Oct 8, 2020

Ok, I think its better to leave it in that way, I will make a pull request to fix a little bug related to when min property is set to 0.

In case you consider the fallback to minNumber should be kept, there is a problem when it is set to 0 and it is that this.minNumber will return as false. So the value will never fallback to minNumber. In the next image this is reflected with a b-numberinput which min property is set to 0:
numberinput-1

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

Successfully merging a pull request may close this issue.

2 participants