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

Value of input[range] incorrectly rounded after mounting (step attribute ommited) #7099

Closed
mariuszdev opened this issue Jun 22, 2016 · 6 comments

Comments

@mariuszdev
Copy link

Do you want to request a feature or report a bug?
Bug
What is the current behavior?
React at the beginning rounds input[type=range] value to 0 or 1, and then looks at step parameter which is 0.1 for example. The result is incorrect value of input after component mounts. When I put value attribute after step attributes, it works fine.
If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/reactjs/69z2wepo/).
Demo: https://jsfiddle.net/2Lm4gy5k/ Comment or uncomment line 13 and 14 to see the difference.

What is the expected behavior?
To always look at step parameter and then round the value (if required).
Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
v15.1.0 (Opera, Chrome, Firefox).

Yes, works in v.0.14.0 (https://jsfiddle.net/wrcnLugd/)

@syranide
Copy link
Contributor

syranide commented Jun 22, 2016

To clarify, if value comes before step then it's rounded, if it comes after then it's not. So yeah something is really off here, the order of the props should never matter.

cc @spicyj, it kind of makes sense that with the new renderer we need to apply certain properties in a specific order (because one depends on the value of another).

@sophiebits
Copy link
Collaborator

Ugh. Inputs.

We already put type first in getNativeProps in ReactDOMInput, probably we can just do the same for step although it's a little stranger since most inputs won't have that attribute.

@troydemonbreun
Copy link
Contributor

@spicyj I've been trying to do this one, as your suggestion seems a straight forward fix, but ironically I've been struggling with just writing the failing test. Some of it is probably my unfamiliarity with the testing utils. I tried this (among a ton of other variations) but could not repro the issue:

var Hello = React.createClass({
  getInitialState: function() {
    return {
        data: 0.6
    }
  },
  onChange: function(e) {
    this.setState({
        data: e.target.value
    });
  },
  render: function() {
    return <input type="range" onChange={this.onChange} value={this.state.data} min="0" max="1" step="0.1" />;
  }
});

stub = ReactTestUtils.renderIntoDocument("<Hello />");
var node = ReactDOM.findDOMNode(stub);

expect(node.value).toBe('0.6');

Thanks!

@sophiebits
Copy link
Collaborator

@troydemonbreun Thanks. It's possible that jsdom doesn't emulate this range behavior properly. I'm not sure.

@sophiebits
Copy link
Collaborator

If that's the case then I'm okay taking this without a unit test as long as you can verify the fix in a browser. Or you could mock out the setters and make sure they're called in the right order.

@troydemonbreun
Copy link
Contributor

Sure, I can provide that test.

@zpao zpao closed this as completed in fc04310 Jul 13, 2016
zpao pushed a commit that referenced this issue Jul 13, 2016
* Set step prop before value prop

* Embed comments on .step sequence behavior

(cherry picked from commit fc04310)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants