Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

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

Merged
merged 27 commits into from
Nov 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
709f46c
Omit marks that are outside of range specified by min and max.
Nov 7, 2019
3bf1241
Merge branch 'dev' into 517-rangeslider-labels
Nov 7, 2019
480511f
Handle case in which marks prop is not defined.
Nov 7, 2019
01324bf
Add test for out-of-range numbers.
Nov 8, 2019
5cb8509
Use pickBy.
Nov 11, 2019
9c94308
Add mark at point below minimum value.
Nov 11, 2019
396756e
Also omit out-of-range marks for slider.
Nov 11, 2019
4bdaf71
Add test for slider.
Nov 11, 2019
1f5d15d
Add padding to Slider and RangeSlider containers.
Nov 12, 2019
a93fa42
Update test for persistence.
Nov 12, 2019
fdd296b
Change test for always visible rangeslider.
Nov 13, 2019
ad838fc
Only add top padding if there are always-visible tooltips on the top.
Nov 14, 2019
c8070d4
Preserve whitespace in marks.
Nov 14, 2019
9e2ee6b
Add optional verticalHeight prop for vertical sliders.
Nov 14, 2019
4d03221
Update slider stylesheet.
Nov 14, 2019
5a6a845
Merge branch 'dev' into 517-rangeslider-labels
Nov 14, 2019
bbb4fea
Update coordinates to reflect new padding.
Nov 14, 2019
a0aad56
Remove file.
Nov 14, 2019
1261f90
Use fixed-width slider for rangeslider test.
Nov 14, 2019
e4476ea
Merge branch 'dev' into 517-rangeslider-labels
Nov 14, 2019
b7dee1a
Fix persistence test.
Nov 14, 2019
edc2784
Memoize computation of style and move function to utils.
Nov 19, 2019
df14f7d
Simplify style code.
Nov 19, 2019
b9a8fcf
Fix eslint errors.
Nov 19, 2019
dfee39a
Merge branch 'dev' into 517-rangeslider-labels
Nov 19, 2019
3e32f28
Modify style object directly.
Nov 19, 2019
473e86a
Update CHANGELOG.
Nov 19, 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
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 27 additions & 3 deletions src/components/RangeSlider.react.js
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -14,6 +15,7 @@ export default class RangeSlider extends Component {
this.DashSlider = props.tooltip
? createSliderWithTooltip(Range)
: Range;
this._computeStyle = computeSliderStyle();
}

propsToState(newProps) {
Expand Down Expand Up @@ -42,6 +44,7 @@ export default class RangeSlider extends Component {
tooltip,
updatemode,
vertical,
verticalHeight,
} = this.props;
const value = this.state.value;

Expand All @@ -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 => {
Expand All @@ -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',
],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for now, but just because I'm thinking about it: I'd like us to consider moving away from omit for forwarding props to 3rd-party components, and use pick instead. IMO omit makes it too hard to follow which props are being passed on, too easy to forget to remove newly added props, and makes us lazy about accepting someone else's choice of names and values even if they don't make sense in our case. (cc @Marc-Andre-Rivet )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexcjohnson Agreed for DCC! There's also a bit of this happening in the table fragments / d3-format wrappers that should be looked into. Due to its generated nature I'd say whether this makes sense on HTML components would have to be evaluated independently. Can you open an issue for follow up and tag it for Dash v1.x milestone? Thanks.

this.props
)}
/>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -281,4 +304,5 @@ RangeSlider.defaultProps = {
updatemode: 'mouseup',
persisted_props: ['value'],
persistence_type: 'local',
verticalHeight: 400,
};
30 changes: 27 additions & 3 deletions src/components/Slider.react.js
Original file line number Diff line number Diff line change
@@ -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/[email protected]';
import computeSliderStyle from '../utils/computeSliderStyle';

/**
* A slider component with a single handle.
Expand All @@ -14,6 +15,7 @@ export default class Slider extends Component {
this.DashSlider = props.tooltip
? createSliderWithTooltip(ReactSlider)
: ReactSlider;
this._computeStyle = computeSliderStyle();
}

propsToState(newProps) {
Expand Down Expand Up @@ -42,6 +44,7 @@ export default class Slider extends Component {
tooltip,
updatemode,
vertical,
verticalHeight,
} = this.props;
const value = this.state.value;

Expand All @@ -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 => {
Expand All @@ -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
)}
/>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -262,4 +285,5 @@ Slider.defaultProps = {
updatemode: 'mouseup',
persisted_props: ['value'],
persistence_type: 'local',
verticalHeight: 400,
};
2 changes: 2 additions & 0 deletions src/components/css/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
text-align: center;
cursor: pointer;
color: #999;
white-space: pre;
width: fit-content;
}
.rc-slider-mark-text-active {
color: #666;
Expand Down
35 changes: 35 additions & 0 deletions src/utils/computeSliderStyle.js
Original file line number Diff line number Diff line change
@@ -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;
});
};
8 changes: 4 additions & 4 deletions tests/integration/misc/test_persistence.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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?",
Expand Down
42 changes: 37 additions & 5 deletions tests/integration/sliders/test_sliders.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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