Skip to content

Commit

Permalink
fix(Slider): use value from input as-is (#5712)
Browse files Browse the repository at this point in the history
This change ensures that `<Slider>` won't force changing the value that
user types in the `<input>`. For example, if we set 3 to `min` and user
wants to type 100 in the `<input>`, this change ensures `<input>` won't
become 3 (the minimum value) as soon as user types 1 (the first
character in 100).

Fixes #3107.
  • Loading branch information
asudoh committed Mar 26, 2020
1 parent 25ab3f2 commit 713ed57
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 8 deletions.
4 changes: 2 additions & 2 deletions packages/react/src/components/Slider/Slider-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@ describe('Slider', () => {
},
};
wrapper.instance().onChange(evt);
expect(wrapper.state().value).toEqual(100);
expect(handleChange).lastCalledWith({ value: 100 });
expect(wrapper.state().value).toEqual(999);
expect(handleChange).lastCalledWith({ value: 999 });
});
});

Expand Down
25 changes: 19 additions & 6 deletions packages/react/src/components/Slider/Slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export default class Slider extends PureComponent {
* Sets up initial slider position and value in response to component mount.
*/
componentDidMount() {
const { value, left } = this.calcValue({});
const { value, left } = this.calcValue({ useRawValue: true });
this.setState({ value, left });
}

Expand Down Expand Up @@ -339,8 +339,14 @@ export default class Slider extends PureComponent {
delta *= stepMultiplier;
}

Math.floor(this.state.value / this.props.step) * this.props.step;
const { value, left } = this.calcValue({
value: this.state.value + delta,
// Ensures custom value from `<input>` won't cause skipping next stepping point with right arrow key,
// e.g. Typing 51 in `<input>`, moving focus onto the thumb and the hitting right arrow key should yield 52 instead of 54
value:
(delta > 0
? Math.floor(this.state.value / this.props.step) * this.props.step
: this.state.value) + delta,
});
this.setState({ value, left });
};
Expand Down Expand Up @@ -369,10 +375,11 @@ export default class Slider extends PureComponent {
if (isNaN(targetValue)) {
this.setState({ value: evt.target.value });
} else {
targetValue = this.clamp(targetValue, this.props.min, this.props.max);

// Recalculate the state's value and update the Slider
const { value, left } = this.calcValue({ value: targetValue });
const { value, left } = this.calcValue({
value: targetValue,
useRawValue: true,
});
this.setState({ value, left, needsOnRelease: true });
}
};
Expand All @@ -394,8 +401,9 @@ export default class Slider extends PureComponent {
* an event fired by one of the Slider's `DRAG_EVENT_TYPES` events.
* @param {number} [params.value] Optional value use during calculations if
* clientX is not provided.
* @param {boolean} [params.useRawValue=false] `true` to use the given value as-is.
*/
calcValue = ({ clientX = null, value = null }) => {
calcValue = ({ clientX = null, value = null, useRawValue = false }) => {
const range = this.props.max - this.props.min;
const boundingRect = this.element.getBoundingClientRect();
const totalSteps = range / this.props.step;
Expand All @@ -419,6 +427,11 @@ export default class Slider extends PureComponent {
leftPercent = value / (range - this.props.min);
}

if (useRawValue) {
// Adjusts only for min/max of thumb position
return { value, left: Math.min(1, Math.max(0, leftPercent)) * 100 };
}

let steppedValue = Math.round(leftPercent * totalSteps) * this.props.step;
let steppedPercent = this.clamp(steppedValue / range, 0, 1);

Expand Down

0 comments on commit 713ed57

Please sign in to comment.