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

Buggy controlled number input on 15.5+ #10539

Closed
hyperknot opened this issue Aug 25, 2017 · 12 comments
Closed

Buggy controlled number input on 15.5+ #10539

hyperknot opened this issue Aug 25, 2017 · 12 comments

Comments

@hyperknot
Copy link

hyperknot commented Aug 25, 2017

Do you want to request a feature or report a bug?
Bug

What is the current behavior?

The following code example works well with text input type, but allows inputs like 012 or 0012 to be entered when using the number input type.

The console.log line always runs and shows the right value, and the App state is also correct when checked with React Developer Tools. It's only that the controlled input is not being "controlled" somehow.

class App extends Component {
  constructor() {
    super()
    this.state = {
      value: '',
    }
  }

  handleChange = e => {
    const value = e.target.value
    const num = parseInt(value, 10)
    console.log(num)
    this.setState({ value: isNaN(num) ? '' : num })
  }

  render() {
    return (
      <div className="App">
        <input type="number" value={this.state.value} onChange={this.handleChange} />
      </div>
    )
  }
}

Present with latest create react app or in this jsfiddle: https://jsfiddle.net/Lhj0j3ok/
Not present in this jsfiddle: https://jsfiddle.net/mayankshukla5031/08ecc97d/

What is the expected behavior?
Input should reflect the state, thus not allowing strings like 012, etc. to be displayed.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

Latest create-react-app:
"react": "15.6.1",
"react-dom": "15.6.1",
"react-scripts": "1.0.11"

Browser: Chrome 60.0.3112.101
OS X 10.12.6

Present with latest create react app or in this jsfiddle: https://jsfiddle.net/Lhj0j3ok/
Not present in this jsfiddle: https://jsfiddle.net/mayankshukla5031/08ecc97d/

@BTMPL
Copy link

BTMPL commented Aug 25, 2017

Just to add to this report, it looks like React is not updating the field that triggered the update event in the first place. See https://codesandbox.io/s/pj5jr090q0 for visual explanation

@hyperknot
Copy link
Author

Just tested it: works well with 15.4 and broken starting from 15.5.

@hyperknot
Copy link
Author

Jsfiddle with 15.4: https://jsfiddle.net/757p0rkt/
Jsfiddle with 15.5: https://jsfiddle.net/6c4e2z1c/

@hyperknot
Copy link
Author

hyperknot commented Aug 25, 2017

Broken in @next, tested via create-react-app / yarn add react@next react-dom@next.

@hyperknot
Copy link
Author

So I'm 99% sure it got introduced via #7359 to 15.5.0.

@hyperknot
Copy link
Author

@BTMPL 's code example working in 15.4.2:
https://codesandbox.io/s/6yk2o7kxyk

@gaearon
Copy link
Collaborator

gaearon commented Aug 25, 2017

cc @nhunzaker

@jquense
Copy link
Contributor

jquense commented Aug 25, 2017

People ask we web devs keep writing ui input libraries instead of using the native inputs...this, this stuff.

@hyperknot hyperknot changed the title Buggy controlled number input on Chrome Buggy controlled number input on 15.5+ Aug 25, 2017
@hyperknot
Copy link
Author

I just checked, it is present in Firefox and Safari as well, so not Chrome related, but was introduced in the Chrome related PR.

@aweary
Copy link
Contributor

aweary commented Aug 25, 2017

This is a duplicate of #9402. What's happening is React parses a number input's value as a number before doing a comparison check. So if node.value is "01" it will call parseFloat which parses as 1. Then it will do a comparison with the new incoming value to decide if it should update the input's value. Since you're already parsing the input as a number the values end up being the same and it decides it doesn't need to update the input.

One solution might be to compare the values as strings as well.

@nickserv
Copy link

Thanks @aweary, that's what I thought. Should we close this and stick to #9402?

@aweary
Copy link
Contributor

aweary commented Aug 29, 2017

Yup! Closing as a duplicate of #9402

@aweary aweary closed this as completed Aug 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants