-
Notifications
You must be signed in to change notification settings - Fork 47.3k
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
Controlled number input doesn't handle e, - and . properly #6556
Comments
Sounds like a bug, probably a good first one. |
It's probably not a "good first bug" (it will surely involve at least looking at our event system, which is anything but good for a first dive into the code). But people are welcome to try :) |
Yeah, I was torn on labeling this one. We need a "good third bug" label :). The reason I thought it might be a reasonable first one is that the code from controlled text inputs can probably be copy-pasted here nearly verbatim. In fact, I'm a little surprised it doesn't already follow the same code path. |
I'm pretty sure that typing in a |
I'm pretty sure this is just another case of these special inputs not playing well with being controlled, |
Yep, @syranide is correct. https://jsfiddle.net/1tscfoLf/ Not sure if there is anything we can do about this at the moment, short of writing our own handlers based on cursor position and keystrokes. Maybe that's what we need to do. (or request revisions from W3C, but that's a longer term thing) |
I found this bug too! https://jsfiddle.net/tmbbau6a/ Any ideas for work arounds? |
This bug happens when using decimal in an input with type number. When adding a decimal to a number, there is no problem. When removing decimal places from a number, after removing the first decimal place, just prior to removing the decimal, the decimal disappears and the cursor jumps to the beginning of the input. For instance, if the value is 30, and I type '.', the value in the event is '30', and event though the input is controlled, the '.' is allowed to remain in the input. If the value is 30.1, and I delete '1', the value in the event is '30', but the decimal disappears and the cursor jumps to the start. |
See #7359 |
This should be addressed in 15.5, now that we've merged the number input PR: |
Wow, what a thread. Thank you! |
@nhunzaker I'm sorry, but I don't think this issue has been addressed, you can still quite clearly type - . and e into a controlled number input and have them appear even though I have not updated the value see I think this is because the browser (I'm on Chrome Version 60.0.3112.90 64-bit) doens't seem to be firing an onChange event when the value is invalid, I'm still a bit hazy as to whether that is down to the browser or react. Either way this is still an issue and I suggest it gets re-opened |
It looks like this happens because Chrome reports the value as an empty string: So React sees that the value hasn't changed, so it doesn't assign the value property. Unfortunately React has to avoid touching I don't know what to do here. Is this a bug? |
At this point, I almost think this is an acceptable concession with controlled inputs. As far as I can tell, we'd have to introduce a lot more logic using key events to manually determine if we need to remove these numerical characters. The decimal dropping bug was a huge pain, so we need to avoid introducing any more changes that may compromise that fix. |
Coming back to this, I still think accepting this as a concession is our best choice. We are limited by the data/events that the DOM will provide us, and in this case, it's not enough to implement the desired behavior without hacky solutions. If you absolutely need to ensure that invalid values cannot be entered you can use a text input and manually validate the value in the With that said, I think it's best to close this and possibly document the behavior if we need to. What do you think @nhunzaker @jquense? |
I agree yeah. This is an endless rabbit hole and I think the line should be drawn somewhere. Tbh the only real "correct" way to handle each edge case is to be culture aware and i18n adjacent. |
I agree. @jquense is spot on too. I think, to do this right, we'd need spot on keystroke detection for at least number inputs, and that would have to work with every language. This is an extremely hard problem, but we should the design challenges if we move forward with removing the |
FWIW I should mention, using Using It's the way I overcome this in my usage: TwilightCoders/card-games#13 (comment) |
It turns out that <input type="number">'s builtin validation is more of a hint than a constraint, and not really reliable. See for instance: facebook/react#6556 https://bugzilla.mozilla.org/show_bug.cgi?id=1398528 Therefore use <input type="text"> and validate ourselves.
A controlled number input, without a corresponding
setState
function, still allowse
,-
and.
to be entered (and numbers can be inputted afterwards). I could be incorrect but I don't believe this is the desired behavior.Here's a JSFiddle demonstrating the issue.
The text was updated successfully, but these errors were encountered: