From b0389c0b5bbb243cb85248ba8db9f8ab09aff222 Mon Sep 17 00:00:00 2001
From: Matthew Brookes <github@nospam.33m.co>
Date: Wed, 4 Jul 2018 20:07:25 +0100
Subject: [PATCH] [Slider] Prevent thumb from overflowing the margin, update
 some tests

---
 packages/material-ui-lab/src/Slider/Slider.js | 68 ++++++++++++-------
 .../material-ui-lab/src/Slider/Slider.test.js | 43 +++++++-----
 2 files changed, 71 insertions(+), 40 deletions(-)

diff --git a/packages/material-ui-lab/src/Slider/Slider.js b/packages/material-ui-lab/src/Slider/Slider.js
index fa5d2457fe8886..8aa49a1c4652f3 100644
--- a/packages/material-ui-lab/src/Slider/Slider.js
+++ b/packages/material-ui-lab/src/Slider/Slider.js
@@ -33,7 +33,7 @@ export const style = theme => {
     root: {
       position: 'relative',
       width: '100%',
-      padding: '16px 0',
+      padding: '16px 8px',
       cursor: 'pointer',
       WebkitTapHighlightColor: 'transparent',
       '&$disabled': {
@@ -41,7 +41,7 @@ export const style = theme => {
       },
       '&$vertical': {
         height: '100%',
-        padding: '0 16px',
+        padding: '8px 16px',
       },
       '&$reverse': {
         transform: 'scaleX(-1)',
@@ -50,6 +50,12 @@ export const style = theme => {
         transform: 'scaleY(-1)',
       },
     },
+    container: {
+      position: 'relative',
+      '&$vertical': {
+        height: '100%',
+      },
+    },
     /* Tracks styles */
     track: {
       position: 'absolute',
@@ -392,12 +398,13 @@ class Slider extends React.Component {
       component: Component,
       classes,
       className: classNameProp,
-      value,
-      min,
+      disabled,
       max,
-      vertical,
+      min,
       reverse,
-      disabled,
+      theme,
+      value,
+      vertical,
       ...other
     } = this.props;
 
@@ -410,12 +417,23 @@ class Slider extends React.Component {
       [classes.activated]: !disabled && currentState === 'activated',
     };
 
-    const rootClasses = classNames(classes.root, {
-      [classes.vertical]: vertical,
-      [classes.reverse]: reverse,
-      [classes.disabled]: disabled,
+    const rootClasses = classNames(
+      classes.root,
+      {
+        [classes.vertical]: vertical,
+        [classes.reverse]: reverse,
+        [classes.disabled]: disabled,
+      },
       classNameProp,
-    });
+    );
+
+    const containerClasses = classNames(
+      classes.container,
+      {
+        [classes.vertical]: vertical,
+      },
+      classNameProp,
+    );
 
     const trackBeforeClasses = classNames(classes.track, classes.trackBefore, commonClasses, {
       [classes.vertical]: vertical,
@@ -452,19 +470,21 @@ class Slider extends React.Component {
         }}
         {...other}
       >
-        <div className={trackBeforeClasses} style={inlineTrackBeforeStyles} />
-        <ButtonBase
-          className={thumbClasses}
-          disableRipple
-          style={inlineThumbStyles}
-          onBlur={this.handleBlur}
-          onKeyDown={this.handleKeyDown}
-          onMouseDown={this.handleMouseDown}
-          onTouchStartCapture={this.handleTouchStart}
-          onTouchMove={this.handleMouseMove}
-          onFocusVisible={this.handleFocus}
-        />
-        <div className={trackAfterClasses} style={inlineTrackAfterStyles} />
+        <div className={containerClasses}>
+          <div className={trackBeforeClasses} style={inlineTrackBeforeStyles} />
+          <ButtonBase
+            className={thumbClasses}
+            disableRipple
+            style={inlineThumbStyles}
+            onBlur={this.handleBlur}
+            onKeyDown={this.handleKeyDown}
+            onMouseDown={this.handleMouseDown}
+            onTouchStartCapture={this.handleTouchStart}
+            onTouchMove={this.handleMouseMove}
+            onFocusVisible={this.handleFocus}
+          />
+          <div className={trackAfterClasses} style={inlineTrackAfterStyles} />
+        </div>
       </Component>
     );
   }
diff --git a/packages/material-ui-lab/src/Slider/Slider.test.js b/packages/material-ui-lab/src/Slider/Slider.test.js
index 3b84838700d511..a9be9f44a3932f 100644
--- a/packages/material-ui-lab/src/Slider/Slider.test.js
+++ b/packages/material-ui-lab/src/Slider/Slider.test.js
@@ -11,18 +11,24 @@ describe('<Slider />', () => {
 
   before(() => {
     shallow = createShallow({ dive: true });
-    classes = getClasses(<Slider />);
+    classes = getClasses(<Slider value={0} />);
     mount = createMount();
   });
 
   it('should render a div', () => {
-    const wrapper = shallow(<Slider />);
+    const wrapper = shallow(<Slider value={0} />);
     assert.strictEqual(wrapper.name(), 'div');
   });
 
   it('should render with the default classes', () => {
-    const wrapper = shallow(<Slider />);
-    assert.strictEqual(wrapper.hasClass(classes.container), true);
+    const wrapper = shallow(<Slider value={0} />);
+    assert.strictEqual(wrapper.hasClass(classes.root), true);
+  });
+
+  it('should render with the default and user classes', () => {
+    const wrapper = shallow(<Slider value={0} className="mySliderClass" />);
+    assert.strictEqual(wrapper.hasClass(classes.root), true);
+    assert.strictEqual(wrapper.hasClass('mySliderClass'), true);
   });
 
   it('should call handlers', () => {
@@ -31,7 +37,12 @@ describe('<Slider />', () => {
     const handleDragEnd = spy();
 
     const wrapper = mount(
-      <Slider onChange={handleChange} onDragStart={handleDragStart} onDragEnd={handleDragEnd} />,
+      <Slider
+        onChange={handleChange}
+        onDragStart={handleDragStart}
+        onDragEnd={handleDragEnd}
+        value={0}
+      />,
     );
     const button = wrapper.find('button');
 
@@ -46,24 +57,24 @@ describe('<Slider />', () => {
 
   describe('prop: vertical', () => {
     it('should render with the default and vertical classes', () => {
-      const wrapper = shallow(<Slider vertical />);
-      assert.strictEqual(wrapper.hasClass(classes.container), true);
+      const wrapper = shallow(<Slider vertical value={0} />);
+      assert.strictEqual(wrapper.hasClass(classes.root), true);
       assert.strictEqual(wrapper.hasClass(classes.vertical), true);
     });
   });
 
   describe('prop: reverse', () => {
     it('should render with the default and reverse classes', () => {
-      const wrapper = shallow(<Slider reverse />);
-      assert.strictEqual(wrapper.hasClass(classes.container), true);
+      const wrapper = shallow(<Slider reverse value={0} />);
+      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(<Slider reverse vertical />);
-      assert.strictEqual(wrapper.hasClass(classes.container), true);
+      const wrapper = shallow(<Slider reverse vertical value={0} />);
+      assert.strictEqual(wrapper.hasClass(classes.root), true);
       assert.strictEqual(wrapper.hasClass(classes.reverse), true);
       assert.strictEqual(wrapper.hasClass(classes.vertical), true);
     });
@@ -74,7 +85,7 @@ describe('<Slider />', () => {
     let wrapper;
 
     before(() => {
-      wrapper = mount(<Slider onChange={handleChange} disabled />);
+      wrapper = mount(<Slider onChange={handleChange} disabled value={0} />);
     });
 
     it('should render thumb with the disabled classes', () => {
@@ -121,12 +132,12 @@ describe('<Slider />', () => {
       const trackBefore = tracks.at(0);
       const trackAfter = tracks.at(1);
 
-      assert.strictEqual(trackBefore.prop('style').width, 'calc(0% - 0px)');
+      assert.strictEqual(trackBefore.prop('style').width, '0%');
       assert.strictEqual(trackAfter.prop('style').width, 'calc(100% - 5px)');
     });
 
     it('after change value should change position of thumb', () => {
-      wrapper.setProps({ value: 0.5 });
+      wrapper.setProps({ value: 50 });
 
       clock.tick(transitionComplexDuration);
 
@@ -139,8 +150,8 @@ describe('<Slider />', () => {
       const trackBefore = tracks.at(0);
       const trackAfter = tracks.at(1);
 
-      assert.strictEqual(trackBefore.prop('style').width, 'calc(50% - 0px)');
-      assert.strictEqual(trackAfter.prop('style').width, 'calc(50% - 7px)');
+      assert.strictEqual(trackBefore.prop('style').width, '50%');
+      assert.strictEqual(trackAfter.prop('style').width, 'calc(100% - 5px)');
     });
   });
 });