Omit marks that are outside of range specified by min and max. (#695)

Shammamah Hossain · web-flow · commit b098058ad7dc · 2019-11-19T12:45:39.000-05:00
* Omit marks that are outside of range specified by min and max.

* Handle case in which marks prop is not defined.

* Add test for out-of-range numbers.

* Use pickBy.

* Add mark at point below minimum value.

* Also omit out-of-range marks for slider.

* Add test for slider.

* Add padding to Slider and RangeSlider containers.

* Update test for persistence.

With the new padding values, the '0' selection is no longer at the very edge of the container div.

* Change test for always visible rangeslider.

* Only add top padding if there are always-visible tooltips on the top.

* Preserve whitespace in marks.

* Add optional verticalHeight prop for vertical sliders.

* Update slider stylesheet.

* Update coordinates to reflect new padding.

* Remove file.

* Use fixed-width slider for rangeslider test.

* Fix persistence test.

* Memoize computation of style and move function to utils.

* Simplify style code.

* Fix eslint errors.

* Modify style object directly.

* Update CHANGELOG.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,17 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## [Unreleased]
+
+### Changed
+- [#695](https://github.com/plotly/dash-core-components/pull/695) Improvements to Slider and RangeSlider
+  - Marks outside of the range specified by `min` and `max` are now omitted when the slider renders.
+  - Padding is now dependent on the orientation (vertical or horizontal), and whether or not tooltips are always displayed.
+  - The whitespace is now preserved for `marks` labels.
+
+### Added
+- [#695](https://github.com/plotly/dash-core-components/pull/695) Added new property `verticalHeight` to Slider and RangeSlider, to allow the user to specify the height (in px) of vertical sliders. This defaults to `400`.
+
 ## [1.5.1] - 2019-11-14
 ### Fixed
 - [#696](https://github.com/plotly/dash-core-components/pull/696) Fix IE11 compatibility issues and ES5 compatibility and validation
diff --git a/src/components/RangeSlider.react.js b/src/components/RangeSlider.react.js
@@ -1,7 +1,8 @@
 import React, {Component} from 'react';
 import PropTypes from 'prop-types';
-import {assoc, omit} from 'ramda';
+import {assoc, omit, pickBy} from 'ramda';
 import {Range, createSliderWithTooltip} from 'rc-slider';
+import computeSliderStyle from '../utils/computeSliderStyle';
 
 /**
  * A double slider with two handles.
@@ -14,6 +15,7 @@ export default class RangeSlider extends Component {
         this.DashSlider = props.tooltip
             ? createSliderWithTooltip(Range)
             : Range;
+        this._computeStyle = computeSliderStyle();
     }
 
     propsToState(newProps) {
@@ -42,6 +44,7 @@ export default class RangeSlider extends Component {
             tooltip,
             updatemode,
             vertical,
+            verticalHeight,
         } = this.props;
         const value = this.state.value;
 
@@ -58,14 +61,21 @@ export default class RangeSlider extends Component {
             tipProps = tooltip;
         }
 
+        const truncatedMarks =
+            this.props.marks &&
+            pickBy(
+                (k, mark) => mark >= this.props.min && mark <= this.props.max,
+                this.props.marks
+            );
+
         return (
             <div
                 id={id}
                 data-dash-is-loading={
                     (loading_state && loading_state.is_loading) || undefined
                 }
                 className={className}
-                style={vertical ? {height: '100%'} : {}}
+                style={this._computeStyle(vertical, verticalHeight, tooltip)}
             >
                 <this.DashSlider
                     onChange={value => {
@@ -82,8 +92,16 @@ export default class RangeSlider extends Component {
                     }}
                     tipProps={tipProps}
                     value={value}
+                    marks={truncatedMarks}
                     {...omit(
-                        ['className', 'value', 'setProps', 'updatemode'],
+                        [
+                            'className',
+                            'value',
+                            'setProps',
+                            'marks',
+                            'updatemode',
+                            'verticalHeight',
+                        ],
                         this.props
                     )}
                 />
@@ -213,6 +231,11 @@ RangeSlider.propTypes = {
      */
     vertical: PropTypes.bool,
 
+    /**
+     * The height, in px, of the slider if it is vertical.
+     */
+    verticalHeight: PropTypes.number,
+
     /**
      * Determines when the component should update
      * its value. If `mouseup`, then the slider
@@ -281,4 +304,5 @@ RangeSlider.defaultProps = {
     updatemode: 'mouseup',
     persisted_props: ['value'],
     persistence_type: 'local',
+    verticalHeight: 400,
 };
diff --git a/src/components/Slider.react.js b/src/components/Slider.react.js
@@ -1,8 +1,9 @@
 import React, {Component} from 'react';
 import ReactSlider, {createSliderWithTooltip} from 'rc-slider';
 import PropTypes from 'prop-types';
-import {assoc, omit} from 'ramda';
+import {assoc, omit, pickBy} from 'ramda';
 import './css/rc-slider@6.1.2.css';
+import computeSliderStyle from '../utils/computeSliderStyle';
 
 /**
  * A slider component with a single handle.
@@ -14,6 +15,7 @@ export default class Slider extends Component {
         this.DashSlider = props.tooltip
             ? createSliderWithTooltip(ReactSlider)
             : ReactSlider;
+        this._computeStyle = computeSliderStyle();
     }
 
     propsToState(newProps) {
@@ -42,6 +44,7 @@ export default class Slider extends Component {
             tooltip,
             updatemode,
             vertical,
+            verticalHeight,
         } = this.props;
         const value = this.state.value;
 
@@ -58,14 +61,21 @@ export default class Slider extends Component {
             tipProps = tooltip;
         }
 
+        const truncatedMarks = this.props.marks
+            ? pickBy(
+                  (k, mark) => mark >= this.props.min && mark <= this.props.max,
+                  this.props.marks
+              )
+            : this.props.marks;
+
         return (
             <div
                 id={id}
                 data-dash-is-loading={
                     (loading_state && loading_state.is_loading) || undefined
                 }
                 className={className}
-                style={vertical ? {height: '100%'} : {}}
+                style={this._computeStyle(vertical, verticalHeight, tooltip)}
             >
                 <this.DashSlider
                     onChange={value => {
@@ -82,8 +92,16 @@ export default class Slider extends Component {
                     }}
                     tipProps={tipProps}
                     value={value}
+                    marks={truncatedMarks}
                     {...omit(
-                        ['className', 'setProps', 'updatemode', 'value'],
+                        [
+                            'className',
+                            'setProps',
+                            'updatemode',
+                            'value',
+                            'marks',
+                            'verticalHeight',
+                        ],
                         this.props
                     )}
                 />
@@ -194,6 +212,11 @@ Slider.propTypes = {
      */
     vertical: PropTypes.bool,
 
+    /**
+     * The height, in px, of the slider if it is vertical.
+     */
+    verticalHeight: PropTypes.number,
+
     /**
      * Determines when the component should update
      * its value. If `mouseup`, then the slider
@@ -262,4 +285,5 @@ Slider.defaultProps = {
     updatemode: 'mouseup',
     persisted_props: ['value'],
     persistence_type: 'local',
+    verticalHeight: 400,
 };
diff --git a/src/components/css/rc-slider@6.1.2.css b/src/components/css/rc-slider@6.1.2.css
@@ -56,6 +56,8 @@
   text-align: center;
   cursor: pointer;
   color: #999;
+  white-space: pre;
+  width: fit-content;
 }
 .rc-slider-mark-text-active {
   color: #666;
diff --git a/src/utils/computeSliderStyle.js b/src/utils/computeSliderStyle.js
@@ -0,0 +1,35 @@
+import {memoizeWith, identity, contains} from 'ramda';
+
+export default () => {
+    return memoizeWith(identity, (vertical, verticalHeight, tooltip) => {
+        const style = {
+            padding: '25px',
+        };
+
+        if (vertical) {
+            style.height = verticalHeight + 'px';
+
+            if (
+                !tooltip ||
+                !tooltip.always_visible ||
+                !contains(tooltip.placement, [
+                    'left',
+                    'topRight',
+                    'bottomRight',
+                ])
+            ) {
+                style.paddingLeft = '0px';
+            }
+        } else {
+            if (
+                !tooltip ||
+                !tooltip.always_visible ||
+                !contains(tooltip.placement, ['top', 'topLeft', 'topRight'])
+            ) {
+                style.paddingTop = '0px';
+            }
+        }
+
+        return style;
+    });
+};
diff --git a/tests/integration/misc/test_persistence.py b/tests/integration/misc/test_persistence.py
@@ -147,11 +147,11 @@ def make_output(*args):
     dash_dcc.find_element("#radioitems label:first-child input").click()  # red
 
     range_slider = dash_dcc.find_element("#rangeslider")
-    dash_dcc.click_at_coord_fractions(range_slider, 0.01, 0.5)  # 0
-    dash_dcc.click_at_coord_fractions(range_slider, 0.5, 0.5)  # 5
+    dash_dcc.click_at_coord_fractions(range_slider, 0.5, 0.25)  # 5
+    dash_dcc.click_at_coord_fractions(range_slider, 0.8, 0.25)  # 8
 
     slider = dash_dcc.find_element("#slider")
-    dash_dcc.click_at_coord_fractions(slider, 0.2, 0.5)  # 22
+    dash_dcc.click_at_coord_fractions(slider, 0.2, 0.25)  # 22
 
     dash_dcc.find_element("#tabs .tab:last-child").click()  # C
 
@@ -166,7 +166,7 @@ def make_output(*args):
         [u"4️⃣", u"6️⃣"],
         "yes maybe",
         "r",
-        [0, 5],
+        [5, 8],
         22,
         "C",
         "knock knock\nwho's there?",
diff --git a/tests/integration/sliders/test_sliders.py b/tests/integration/sliders/test_sliders.py
@@ -26,15 +26,15 @@ def update_output(value):
     dash_dcc.wait_for_text_to_equal("#out", "You have selected 5")
 
     slider = dash_dcc.find_element("#slider")
-    dash_dcc.click_at_coord_fractions(slider, 0.5, 0.5)
+    dash_dcc.click_at_coord_fractions(slider, 0.5, 0.25)
     dash_dcc.wait_for_text_to_equal("#out", "You have selected 10")
-    dash_dcc.click_at_coord_fractions(slider, 0.75, 0.5)
+    dash_dcc.click_at_coord_fractions(slider, 0.75, 0.25)
     dash_dcc.wait_for_text_to_equal("#out", "You have selected 15")
 
 
 def test_slsl002_always_visible_rangeslider(dash_dcc):
     app = dash.Dash(__name__)
-    app.layout = html.Div([
+    app.layout = html.Div(style={'width': '400px'}, children=[
         dcc.RangeSlider(
             id="rangeslider",
             min=0,
@@ -54,7 +54,39 @@ def update_output(rng):
     dash_dcc.wait_for_text_to_equal("#out", "You have selected 5-15")
 
     slider = dash_dcc.find_element("#rangeslider")
-    dash_dcc.click_at_coord_fractions(slider, 0.1, 0.5)
+    dash_dcc.click_at_coord_fractions(slider, 0.15, 0.25)
     dash_dcc.wait_for_text_to_equal("#out", "You have selected 2-15")
-    dash_dcc.click_at_coord_fractions(slider, 0.5, 0.5)
+    dash_dcc.click_at_coord_fractions(slider, 0.5, 0.25)
     dash_dcc.wait_for_text_to_equal("#out", "You have selected 2-10")
+
+
+def test_slsl003_out_of_range_marks_slider(dash_dcc):
+
+    app = dash.Dash(__name__)
+    app.layout = html.Div([
+        dcc.Slider(
+            min=0,
+            max=5,
+            marks={i: 'Label {}'.format(i) for i in range(-1, 10)}
+        )
+    ])
+
+    dash_dcc.start_server(app)
+
+    assert len(dash_dcc.find_elements('span.rc-slider-mark-text')) == 6
+
+
+def test_slsl004_out_of_range_marks_rangeslider(dash_dcc):
+
+    app = dash.Dash(__name__)
+    app.layout = html.Div([
+        dcc.RangeSlider(
+            min=0,
+            max=5,
+            marks={i: 'Label {}'.format(i) for i in range(-1, 10)}
+        )
+    ])
+
+    dash_dcc.start_server(app)
+
+    assert len(dash_dcc.find_elements('span.rc-slider-mark-text')) == 6

Original file line number	Diff line number	Diff line change
`@@ -56,6 +56,8 @@`
`56`	`56`	`text-align: center;`
`57`	`57`	`cursor: pointer;`
`58`	`58`	`color: #999;`
	`59`	`+ white-space: pre;`
	`60`	`+ width: fit-content;`
`59`	`61`	`}`
`60`	`62`	`.rc-slider-mark-text-active {`
`61`	`63`	`color: #666;`