-
Notifications
You must be signed in to change notification settings - Fork 53
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
Handling onChange for Number Type #333
Comments
@minkimcello do you have a link to the runkit example that we created? |
It seems like you're trying to have a natural transformation from String to Number different than the default JavaScript one: |
To be more clear, if you want a different transformation, then you'll have use a function to do it: For example: class NumberInput {
get string() {
return valueOf(this);
}
get number() {
return Number(this.string.split('.').slice(0,2).map(Number).join("."))
}
}
let input = create(NumberInput, "5");
input.string //=> "5"
input.number //= 5
let twelveDot = input.set("12.");
twelveDot.string //=> "12."
twelveDot.number //=> 12 |
@cowboyd the issue is actually a little deeper. It shows itself in @minkimcello's example but it comes down to the fact that our current implementation of number has an initializer that can sometimes setup in state being a number and other it'll be something else. This makes it confusing when binding a number to an input field because it seems to work except when you start to add periods. When bound to an input, it feels broken because you can't enter a I'm guessing that the answer here is that input fields should always be bound to String types. In addition, we might want to coerce the value into a number so a NaN would be 0. I'm not sure what the answer is here, but the current initializer logic is misleading due to allowing some inputs to be non-numbers. |
I agree. I think there are two separate issues here.
|
I agree with both points.
@minkimcello can you please create a PR that changes how NumberType initialization works. Change it so that |
Slight wrinkle, you'll have to use the builtin isNaN() function since
|
I ran into an issue for number input fields with an onChange that directly invokes a set transition. This prevents the user from inputting decimals because
123.
would immediately return123
as demonstrated here:This isn't a problem when using an onBlur or a form, but I've been using the solution below for cases where onChange is necessary:
Is there a more appropriate way around this?
The text was updated successfully, but these errors were encountered: