From b47ba624ab50ec1efd9b372b0bd356e7b8f3ae42 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sun, 23 Sep 2018 13:13:42 +0200 Subject: [PATCH 1/2] [Slider] Replace reversed with rtl support on horizontal sliders This follows WAI-ARIA slider authoring practices as well as the material design specs. The vertical slider is kept regardless. --- docs/src/pages/lab/slider/ReverseSlider.js | 38 ------------------- docs/src/pages/lab/slider/VerticalSlider.js | 1 - docs/src/pages/lab/slider/slider.md | 5 ++- .../material-ui-lab/src/Slider/Slider.d.ts | 2 - packages/material-ui-lab/src/Slider/Slider.js | 35 +++++++---------- .../material-ui-lab/src/Slider/Slider.test.js | 17 --------- pages/lab/api/slider.md | 2 - pages/lab/slider.js | 7 ---- 8 files changed, 17 insertions(+), 90 deletions(-) delete mode 100644 docs/src/pages/lab/slider/ReverseSlider.js diff --git a/docs/src/pages/lab/slider/ReverseSlider.js b/docs/src/pages/lab/slider/ReverseSlider.js deleted file mode 100644 index 6bf3ead624b9b2..00000000000000 --- a/docs/src/pages/lab/slider/ReverseSlider.js +++ /dev/null @@ -1,38 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import { withStyles } from '@material-ui/core/styles'; -import Slider from '@material-ui/lab/Slider'; - -const styles = { - root: { - width: 300, - }, -}; - -class ReverseSlider extends React.Component { - state = { - value: 50, - }; - - handleChange = (event, value) => { - this.setState({ value }); - }; - - render() { - const { classes } = this.props; - const { value } = this.state; - - return ( -
- - -
- ); - } -} - -ReverseSlider.propTypes = { - classes: PropTypes.object.isRequired, -}; - -export default withStyles(styles)(ReverseSlider); diff --git a/docs/src/pages/lab/slider/VerticalSlider.js b/docs/src/pages/lab/slider/VerticalSlider.js index 42fd8d6905a700..e7b44b20454121 100644 --- a/docs/src/pages/lab/slider/VerticalSlider.js +++ b/docs/src/pages/lab/slider/VerticalSlider.js @@ -26,7 +26,6 @@ class VerticalSlider extends React.Component { return (
-
); } diff --git a/docs/src/pages/lab/slider/slider.md b/docs/src/pages/lab/slider/slider.md index 6b739113a6da10..ca7aa5b1d803af 100644 --- a/docs/src/pages/lab/slider/slider.md +++ b/docs/src/pages/lab/slider/slider.md @@ -37,7 +37,10 @@ Sliders reflect the current state of the settings they control. ## Reverse slider -{{"demo": "pages/lab/slider/ReverseSlider.js"}} +Per [material-design specification](https://material.io/design/components/sliders.html#usage) +the position of the minimum value should depend on the language direction: On the left for left-to-right +and on the right for right-to-left languages. Check the [Right-to-left guide](/guides/right-to-left/) +to learn how to switch to right-to-left languages. ## Custom thumb diff --git a/packages/material-ui-lab/src/Slider/Slider.d.ts b/packages/material-ui-lab/src/Slider/Slider.d.ts index 879a0b4816c73b..401386fba27b6a 100644 --- a/packages/material-ui-lab/src/Slider/Slider.d.ts +++ b/packages/material-ui-lab/src/Slider/Slider.d.ts @@ -7,7 +7,6 @@ import { TransitionHandlerProps } from '@material-ui/core/transitions/transition export interface SliderProps extends StandardProps, SliderClassKey, 'onChange'> { disabled?: boolean; - reverse?: boolean; vertical?: boolean; max?: number; min?: number; @@ -29,7 +28,6 @@ export type SliderClassKey = | 'activated' | 'disabled' | 'vertical' - | 'reverse' | 'jumped'; declare const Slider: React.ComponentType; diff --git a/packages/material-ui-lab/src/Slider/Slider.js b/packages/material-ui-lab/src/Slider/Slider.js index f8652836d998a9..6fb3617ae3ceb4 100644 --- a/packages/material-ui-lab/src/Slider/Slider.js +++ b/packages/material-ui-lab/src/Slider/Slider.js @@ -15,7 +15,7 @@ export const styles = theme => { }; const commonTransitions = theme.transitions.create( - ['width', 'height', 'left', 'top', 'box-shadow'], + ['width', 'height', 'left', 'right', 'top', 'box-shadow'], commonTransitionsOptions, ); // no transition on the position @@ -44,12 +44,6 @@ export const styles = theme => { height: '100%', padding: '8px 16px', }, - '&$reverse': { - transform: 'scaleX(-1)', - }, - '&$vertical$reverse': { - transform: 'scaleY(-1)', - }, }, /* Styles applied to the container element. */ container: { @@ -130,8 +124,6 @@ export const styles = theme => { height: 'inherit', width: 'inherit', }, - /* Class applied to the root element to trigger JSS nested styles if `reverse={true}`. */ - reverse: {}, /* Class applied to the track and thumb elements to trigger JSS nested styles if `disabled`. */ disabled: {}, /* Class applied to the track and thumb elements to trigger JSS nested styles if `jumped`. */ @@ -177,7 +169,7 @@ function getMousePosition(event) { }; } -function calculatePercent(node, event, isVertical, isReverted) { +function calculatePercent(node, event, isVertical, isRtl) { const { width, height } = node.getBoundingClientRect(); const { top, left } = getOffset(node); const { x, y } = getMousePosition(event); @@ -185,7 +177,7 @@ function calculatePercent(node, event, isVertical, isReverted) { const value = isVertical ? y - top : x - left; const onePercent = (isVertical ? height : width) / 100; - return isReverted ? 100 - clamp(value / onePercent) : clamp(value / onePercent); + return isRtl && !isVertical ? 100 - clamp(value / onePercent) : clamp(value / onePercent); } function preventPageScrolling(event) { @@ -275,8 +267,8 @@ class Slider extends React.Component { }; handleClick = event => { - const { min, max, vertical, reverse } = this.props; - const percent = calculatePercent(this.containerRef, event, vertical, reverse); + const { min, max, vertical } = this.props; + const percent = calculatePercent(this.containerRef, event, vertical, this.isReverted()); const value = percentToValue(percent, min, max); this.emitChange(event, value, () => { @@ -320,8 +312,8 @@ class Slider extends React.Component { }; handleMouseMove = event => { - const { min, max, vertical, reverse } = this.props; - const percent = calculatePercent(this.containerRef, event, vertical, reverse); + const { min, max, vertical } = this.props; + const percent = calculatePercent(this.containerRef, event, vertical, this.isReverted()); const value = percentToValue(percent, min, max); this.emitChange(event, value); @@ -383,6 +375,10 @@ class Slider extends React.Component { }); } + isReverted() { + return this.props.theme.direction === 'rtl'; + } + render() { const { currentState } = this.state; const { @@ -396,7 +392,6 @@ class Slider extends React.Component { onChange, onDragEnd, onDragStart, - reverse, step, theme, value, @@ -417,7 +412,6 @@ class Slider extends React.Component { classes.root, { [classes.vertical]: vertical, - [classes.reverse]: reverse, [classes.disabled]: disabled, }, classNameProp, @@ -436,7 +430,8 @@ class Slider extends React.Component { }); const trackProperty = vertical ? 'height' : 'width'; - const thumbProperty = vertical ? 'top' : 'left'; + const horizontalMinimumPosition = theme.direction === 'ltr' ? 'left' : 'right'; + const thumbProperty = vertical ? 'top' : horizontalMinimumPosition; const inlineTrackBeforeStyles = { [trackProperty]: this.calculateTrackBeforeStyles(percent) }; const inlineTrackAfterStyles = { [trackProperty]: this.calculateTrackAfterStyles(percent) }; const inlineThumbStyles = { [thumbProperty]: `${percent}%` }; @@ -537,10 +532,6 @@ Slider.propTypes = { * Callback function that is fired when the slider has begun to move. */ onDragStart: PropTypes.func, - /** - * If `true`, the slider will be reversed. - */ - reverse: PropTypes.bool, /** * The granularity the slider can step through values. */ diff --git a/packages/material-ui-lab/src/Slider/Slider.test.js b/packages/material-ui-lab/src/Slider/Slider.test.js index 78f6e1b5d3ca1c..1452823884194d 100644 --- a/packages/material-ui-lab/src/Slider/Slider.test.js +++ b/packages/material-ui-lab/src/Slider/Slider.test.js @@ -93,23 +93,6 @@ describe('', () => { }); }); - describe('prop: reverse', () => { - it('should render with the default and reverse classes', () => { - const wrapper = shallow(); - assert.strictEqual(wrapper.hasClass(classes.root), true); - assert.strictEqual(wrapper.hasClass(classes.reverse), true); - }); - }); - - describe('props: vertical & reverse', () => { - it('should render with the default, reverse and vertical classes', () => { - const wrapper = shallow(); - assert.strictEqual(wrapper.hasClass(classes.root), true); - assert.strictEqual(wrapper.hasClass(classes.reverse), true); - assert.strictEqual(wrapper.hasClass(classes.vertical), true); - }); - }); - describe('prop: disabled', () => { const handleChange = spy(); let wrapper; diff --git a/pages/lab/api/slider.md b/pages/lab/api/slider.md index 7d8d3d4f3f2a3b..dad60f2500c76a 100644 --- a/pages/lab/api/slider.md +++ b/pages/lab/api/slider.md @@ -27,7 +27,6 @@ import Slider from '@material-ui/lab/Slider'; | onChange | func |   | Callback function that is fired when the slider's value changed. | | onDragEnd | func |   | Callback function that is fired when the slide has stopped moving. | | onDragStart | func |   | Callback function that is fired when the slider has begun to move. | -| reverse | bool |   | If `true`, the slider will be reversed. | | step | number |   | The granularity the slider can step through values. | | thumb | element |   | The component used for the slider icon. This is optional, if provided should be a react element. | | value * | number |   | The value of the slider. | @@ -51,7 +50,6 @@ This property accepts the following keys: | thumb | Styles applied to the thumb element. | thumbIconWrapper | Class applied to the thumb element if custom thumb icon provided. | thumbIcon | -| reverse | Class applied to the root element to trigger JSS nested styles if `reverse={true}`. | disabled | Class applied to the track and thumb elements to trigger JSS nested styles if `disabled`. | jumped | Class applied to the track and thumb elements to trigger JSS nested styles if `jumped`. | focused | Class applied to the track and thumb elements to trigger JSS nested styles if `focused`. diff --git a/pages/lab/slider.js b/pages/lab/slider.js index 8fc025c880cc49..5b3e0a6b923b31 100644 --- a/pages/lab/slider.js +++ b/pages/lab/slider.js @@ -34,13 +34,6 @@ module.exports = require('fs') raw: preval` module.exports = require('fs') .readFileSync(require.resolve('docs/src/pages/lab/slider/VerticalSlider'), 'utf8') -`, - }, - 'pages/lab/slider/ReverseSlider.js': { - js: require('docs/src/pages/lab/slider/ReverseSlider').default, - raw: preval` -module.exports = require('fs') - .readFileSync(require.resolve('docs/src/pages/lab/slider/ReverseSlider'), 'utf8') `, }, 'pages/lab/slider/CustomIconSlider.js': { From dcb6572b23a7e2955759a94fff109c0a5f70e788 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sun, 23 Sep 2018 18:02:17 +0200 Subject: [PATCH 2/2] [docs] Remove obsolete reverse slider section --- docs/src/pages/lab/slider/slider.md | 7 ------- 1 file changed, 7 deletions(-) diff --git a/docs/src/pages/lab/slider/slider.md b/docs/src/pages/lab/slider/slider.md index ca7aa5b1d803af..268eaab6f0e6d8 100644 --- a/docs/src/pages/lab/slider/slider.md +++ b/docs/src/pages/lab/slider/slider.md @@ -35,13 +35,6 @@ Sliders reflect the current state of the settings they control. {{"demo": "pages/lab/slider/VerticalSlider.js"}} -## Reverse slider - -Per [material-design specification](https://material.io/design/components/sliders.html#usage) -the position of the minimum value should depend on the language direction: On the left for left-to-right -and on the right for right-to-left languages. Check the [Right-to-left guide](/guides/right-to-left/) -to learn how to switch to right-to-left languages. - ## Custom thumb {{"demo": "pages/lab/slider/CustomIconSlider.js"}} \ No newline at end of file