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.

Sign up for GitHub

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

[Slider] Make the slider work as intended when max%step !== 0 #18438

Merged
merged 2 commits into from
Nov 18, 2019
Merged

[Slider] Make the slider work as intended when max%step !== 0 #18438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ba60b5
[Slider] Make the slider work as intended when max%step !== 0
Nov 18, 2019
c54accd
[Slider] Edit the component docs
Nov 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/pages/api/slider.md
View file
Open in desktop
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<number></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
View file
Open in desktop
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
View file
Open in desktop
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