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

NumericInput with value set does not behave as Controlled component #2186

Closed
ablakey opened this issue Feb 27, 2018 · 3 comments · Fixed by #4211
Closed

NumericInput with value set does not behave as Controlled component #2186

ablakey opened this issue Feb 27, 2018 · 3 comments · Fixed by #4211

Comments

@ablakey
Copy link

ablakey commented Feb 27, 2018

Bug report

  • Package version(s): 2.0.0-rc.2
  • Browser and OS versions: Ubuntu 16.04, Chrome 64.0.3282.186

Steps to reproduce

  1. Create a basic NumericInput component. It can be as simple as this:
<NumericInput
  value={1.0}
  onValueChange={v => console.log(v)}
/>
  1. Type in a different value.
  2. Notice that the value does change.

Actual behavior

The component freely changes value despite the value property being set to a constant.

Expected behavior

The component's value shall never change unless the value property is changed.

@ablakey
Copy link
Author

ablakey commented Feb 27, 2018

Forgive me if I'm missing something. It looks like NumericInput is simply not implemented to be a controlled component, despite the documentation:

https://github.com/palantir/blueprint/blob/f4e9f758/packages/core/src/components/forms/numericInput.tsx#L180

I don't see anything that enforces that this.props.value takes precedence over this.state.value

The best I can guess is that it's "controlled" in the sense that this.props.value will just override this.state.value each re-render. But that does not help us if the component is not being re-rendered by its parent. What should happen is that changing the value invokes the onValueChange callback, but it does not cause an internal re-render (because state mutates) since this.props.value is not undefined.

@benjaminbours
Copy link

👍 I'm facing the same unexpected behaviour. Would be nice and probably not a lot of work to fix it, I Will do it soon if I got some time. This behaviour currently prevent me to implement a cleaner force value to integer, as described here. Because finally what you push into value, is only the first displayed value in the field, due to the internal re-render described by @ablakey

@benjaminbours
Copy link

benjaminbours commented May 18, 2020

Investigating, I realised this issue has already a duplicate here => #3553.

By the way, looking at my problem and the one mentioned in #3553, we comeback again to this use case about having kind of a mode integer for the component. It has been proposed #1956 but not accepted. I would like to encourage to reconsider it. Of course it's easy to do it yourself (when you have a real controlled component...) but the simplicity of just enable it is valuable from a user point of view. At least ensuring an alternative is working if it's not this precise solution.

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