Skip to content

Commit

Permalink
[Slider] Make the slider work as intended when max%step !== 0 (#18438)
Browse files Browse the repository at this point in the history
  • Loading branch information
julie-volkova authored and oliviertassinari committed Nov 18, 2019
1 parent 15e3336 commit 5becaa5
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 13 deletions.
2 changes: 1 addition & 1 deletion docs/pages/api/slider.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ You can learn more about the difference by [reading this guide](/guides/minimizi
| <span class="prop-name">onChange</span> | <span class="prop-type">func</span> | | Callback function that is fired when the slider's value changed.<br><br>**Signature:**<br>`function(event: object, value: any) => void`<br>*event:* The event source of the callback.<br>*value:* The new value. |
| <span class="prop-name">onChangeCommitted</span> | <span class="prop-type">func</span> | | Callback function that is fired when the `mouseup` is triggered.<br><br>**Signature:**<br>`function(event: object, value: any) => void`<br>*event:* The event source of the callback.<br>*value:* The new value. |
| <span class="prop-name">orientation</span> | <span class="prop-type">'horizontal'<br>&#124;&nbsp;'vertical'</span> | <span class="prop-default">'horizontal'</span> | The slider orientation. |
| <span class="prop-name">step</span> | <span class="prop-type">number</span> | <span class="prop-default">1</span> | The granularity with which the slider can step through values. (A "discrete" slider.) When step is `null`, the thumb can only be slid onto marks provided with the `marks` prop. |
| <span class="prop-name">step</span> | <span class="prop-type">number</span> | <span class="prop-default">1</span> | The granularity with which the slider can step through values. (A "discrete" slider.) The `min` prop serves as the origin for the valid values. We recommend (max - min) to be evenly divisible by the step.<br>When step is `null`, the thumb can only be slid onto marks provided with the `marks` prop. |
| <span class="prop-name">ThumbComponent</span> | <span class="prop-type">elementType</span> | <span class="prop-default">'span'</span> | The component used to display the value label. |
| <span class="prop-name">track</span> | <span class="prop-type">'normal'<br>&#124;&nbsp;false<br>&#124;&nbsp;'inverted'</span> | <span class="prop-default">'normal'</span> | The track presentation:<br>- `normal` the track will render a bar representing the slider value. - `inverted` the track will render a bar representing the remaining slider value. - `false` the track will render without a bar. |
| <span class="prop-name">value</span> | <span class="prop-type">number<br>&#124;&nbsp;Array&lt;number&gt;</span> | | The value of the slider. For ranged sliders, provide an array with two values. |
Expand Down
11 changes: 7 additions & 4 deletions packages/material-ui/src/Slider/Slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ function getDecimalPrecision(num) {
return decimalPart ? decimalPart.length : 0;
}

function roundValueToStep(value, step) {
const nearest = Math.round(value / step) * step;
function roundValueToStep(value, step, min) {
const nearest = Math.round((value - min) / step) * step + min;
return Number(nearest.toFixed(getDecimalPrecision(step)));
}

Expand Down Expand Up @@ -492,7 +492,7 @@ const Slider = React.forwardRef(function Slider(props, ref) {
event.preventDefault();

if (step) {
newValue = roundValueToStep(newValue, step);
newValue = roundValueToStep(newValue, step, min);
}

newValue = clamp(newValue, min, max);
Expand Down Expand Up @@ -547,7 +547,7 @@ const Slider = React.forwardRef(function Slider(props, ref) {
let newValue;
newValue = percentToValue(percent, min, max);
if (step) {
newValue = roundValueToStep(newValue, step);
newValue = roundValueToStep(newValue, step, min);
} else {
const marksValues = marks.map(mark => mark.value);
const closestIndex = findClosest(marksValues, newValue);
Expand Down Expand Up @@ -946,6 +946,9 @@ Slider.propTypes = {
orientation: PropTypes.oneOf(['horizontal', 'vertical']),
/**
* The granularity with which the slider can step through values. (A "discrete" slider.)
* The `min` prop serves as the origin for the valid values.
* We recommend (max - min) to be evenly divisible by the step.
*
* When step is `null`, the thumb can only be slid onto marks provided with the `marks` prop.
*/
step: PropTypes.number,
Expand Down
28 changes: 20 additions & 8 deletions packages/material-ui/src/Slider/Slider.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,40 +306,52 @@ describe('<Slider />', () => {
key: 'ArrowRight',
};

it('should use min as the step origin', () => {
const { getByRole } = render(<Slider defaultValue={150} step={100} max={750} min={150} />);
const thumb = getByRole('slider');
thumb.focus();

fireEvent.keyDown(document.activeElement, moveRightEvent);
expect(thumb).to.have.attribute('aria-valuenow', '250');

fireEvent.keyDown(document.activeElement, moveLeftEvent);
expect(thumb).to.have.attribute('aria-valuenow', '150');
});

it('should reach right edge value', () => {
const { getByRole } = render(<Slider defaultValue={90} min={6} max={108} step={10} />);
const thumb = getByRole('slider');
thumb.focus();

fireEvent.keyDown(document.activeElement, moveRightEvent);
expect(thumb).to.have.attribute('aria-valuenow', '100');
expect(thumb).to.have.attribute('aria-valuenow', '96');

fireEvent.keyDown(document.activeElement, moveRightEvent);
expect(thumb).to.have.attribute('aria-valuenow', '106');

fireEvent.keyDown(document.activeElement, moveRightEvent);
expect(thumb).to.have.attribute('aria-valuenow', '108');

fireEvent.keyDown(document.activeElement, moveLeftEvent);
expect(thumb).to.have.attribute('aria-valuenow', '100');
expect(thumb).to.have.attribute('aria-valuenow', '96');

fireEvent.keyDown(document.activeElement, moveLeftEvent);
expect(thumb).to.have.attribute('aria-valuenow', '90');
expect(thumb).to.have.attribute('aria-valuenow', '86');
});

it('should reach left edge value', () => {
const { getByRole } = render(<Slider defaultValue={20} min={6} max={108} step={10} />);
const thumb = getByRole('slider');
thumb.focus();

fireEvent.keyDown(document.activeElement, moveLeftEvent);
expect(thumb).to.have.attribute('aria-valuenow', '10');

fireEvent.keyDown(document.activeElement, moveLeftEvent);
expect(thumb).to.have.attribute('aria-valuenow', '6');

fireEvent.keyDown(document.activeElement, moveRightEvent);
expect(thumb).to.have.attribute('aria-valuenow', '20');
expect(thumb).to.have.attribute('aria-valuenow', '16');

fireEvent.keyDown(document.activeElement, moveRightEvent);
expect(thumb).to.have.attribute('aria-valuenow', '30');
expect(thumb).to.have.attribute('aria-valuenow', '26');
});

it('should round value to step precision', () => {
Expand Down

0 comments on commit 5becaa5

Please sign in to comment.