Skip to content

Commit

Permalink
Merge pull request #1857 from plotly/fix-slider-marks
Browse files Browse the repository at this point in the history
Fix slider marks and steps bugs
  • Loading branch information
alexcjohnson authored Dec 15, 2021
2 parents 6cfb787 + c947600 commit 94cbc3b
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 10 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ This project adheres to [Semantic Versioning](https://semver.org/).
```

### Fixed

- [#1858](https://github.com/plotly/dash/pull/1858) Support `mini-css-extract-plugin` Webpack plugin with `@plotly/webpack-dash-dynamic-import` node package - used by components to support dash async chunks. Updated dependencies of other `@plotly` node packages.

- [#1836](https://github.com/plotly/dash/pull/1836) Fix `__all__` in dcc and table for extras: dcc download helpers and table format helpers. This also restores this functionality to the obsolete top-level packages `dash_core_components` and `dash_table`.
Expand All @@ -185,6 +184,9 @@ This project adheres to [Semantic Versioning](https://semver.org/).
- [#1707](https://github.com/plotly/dash/pull/1707) Change the default value of the `compress` argument to the `dash.Dash` constructor to `False`. This change reduces CPU usage, and was made in recognition of the fact that many deployment platforms (e.g. Dash Enterprise) already apply their own compression. If deploying to an environment that does not already provide compression, the Dash 1 behavior may be restored by adding `compress=True` to the `dash.Dash` constructor.
- [#1734](https://github.com/plotly/dash/pull/1734) Added `npm run build` script to simplify build process involving `dash-renderer` and subcomponent libraries within `dash`.

### Fixed
- [#1857](https://github.com/plotly/dash/pull/1857) Fixed a regression with `dcc.Slider` and `dcc.RangeSlider` where steps were not being set to marks if None was passed as the prop argument. Added a check to set the min and max based on the range of marks if they are not explicitly defined (for more info, see [#1843](https://github.com/plotly/dash/issues/1843) and [#1851](https://github.com/plotly/dash/issues/1843)).


## Dash Core Components
### Added
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, {Component} from 'react';
import {assoc, omit} from 'ramda';
import {assoc, omit, isNil} from 'ramda';
import {Range, createSliderWithTooltip} from 'rc-slider';
import computeSliderStyle from '../utils/computeSliderStyle';

Expand All @@ -8,6 +8,7 @@ import {
calcValue,
sanitizeMarks,
calcStep,
setUndefined,
} from '../utils/computeSliderMarkers';
import {propTypes, defaultProps} from '../components/RangeSlider.react';

Expand Down Expand Up @@ -104,7 +105,13 @@ export default class RangeSlider extends Component {
style={{position: 'relative'}}
value={value ? value : calcValue(min, max, value)}
marks={sanitizeMarks({min, max, marks, step})}
step={calcStep(min, max, step)}
max={setUndefined(min, max, marks).max_mark}
min={setUndefined(min, max, marks).min_mark}
step={
step === null && !isNil(marks)
? null
: calcStep(min, max, step)
}
{...omit(
[
'className',
Expand Down
16 changes: 13 additions & 3 deletions components/dash-core-components/src/fragments/Slider.react.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import React, {Component} from 'react';
import ReactSlider, {createSliderWithTooltip} from 'rc-slider';
import {assoc, omit} from 'ramda';
import {assoc, isNil, omit} from 'ramda';
import computeSliderStyle from '../utils/computeSliderStyle';

import 'rc-slider/assets/index.css';

import {sanitizeMarks, calcStep} from '../utils/computeSliderMarkers';
import {
sanitizeMarks,
calcStep,
setUndefined,
} from '../utils/computeSliderMarkers';
import {propTypes, defaultProps} from '../components/Slider.react';

/**
Expand Down Expand Up @@ -104,7 +108,13 @@ export default class Slider extends Component {
style={{position: 'relative'}}
value={value}
marks={sanitizeMarks({min, max, marks, step})}
step={calcStep(min, max, step)}
max={setUndefined(min, max, marks).max_mark}
min={setUndefined(min, max, marks).min_mark}
step={
step === null && !isNil(marks)
? null
: calcStep(min, max, step)
}
{...omit(
[
'className',
Expand Down
33 changes: 29 additions & 4 deletions components/dash-core-components/src/utils/computeSliderMarkers.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {pickBy, isEmpty} from 'ramda';
import {pickBy, isEmpty, isNil} from 'ramda';
import {formatPrefix} from 'd3-format';

/**
Expand Down Expand Up @@ -85,6 +85,29 @@ export const calcStep = (min, max, step) => {
].sort((a, b) => Math.abs(a - v) - Math.abs(b - v))[0];
};

/**
* Set min and max if they are undefined and marks are defined
*/
export const setUndefined = (min, max, marks) => {
const definedMarks = {min_mark: min, max_mark: max};

if (isNil(marks)) {
return definedMarks;
}

const marksObject = Object.keys(marks).map(Number);

if (isNil(min)) {
definedMarks.min_mark = Math.min(...marksObject);
}

if (isNil(max)) {
definedMarks.max_mark = Math.max(...marksObject);
}

return definedMarks;
};

export const applyD3Format = (mark, min, max) => {
const mu_ten_factor = -3;
const k_ten_factor = 3;
Expand Down Expand Up @@ -124,7 +147,6 @@ export const autoGenerateMarks = (min, max, step) => {
marks.pop();
}
}

const marksObject = {};
marks.forEach(mark => {
marksObject[mark] = applyD3Format(mark, min, max);
Expand All @@ -144,15 +166,18 @@ export const sanitizeMarks = ({min, max, marks, step}) => {
return undefined;
}

const {min_mark, max_mark} = setUndefined(min, max, marks);

const truncated_marks =
marks && isEmpty(marks) === false
? truncateMarks(min, max, marks)
? truncateMarks(min_mark, max_mark, marks)
: marks;

if (truncated_marks && isEmpty(truncated_marks) === false) {
return truncated_marks;
}
return autoGenerateMarks(min, max, step);

return autoGenerateMarks(min_mark, max_mark, step);
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -523,3 +523,63 @@ def test_slsl014_vertical_range_slider(dash_dcc):
'#vertical-range-slider div.rc-slider-handle-2[role="slider"]'
).click()
assert dash_dcc.get_logs() == []


def test_slsl015_range_slider_step_none(dash_dcc):
app = Dash(__name__)
app.layout = html.Div(
[
html.Label("Steps = Marks Slider"),
dcc.Slider(
id="none-step-slider",
min=0,
max=6,
marks={
i: "Label {}".format(i) if i == 1 else str(i) for i in range(1, 6)
},
step=None,
value=4.6,
vertical=False,
),
],
style={"height": "500px"},
)

dash_dcc.start_server(app)
dash_dcc.wait_for_element("#none-step-slider")
dash_dcc.percy_snapshot("none step slider")

dash_dcc.wait_for_element(
'#none-step-slider div.rc-slider-handle[aria-valuenow="5"]'
)

assert dash_dcc.get_logs() == []


def test_slsl015_range_slider_no_min_max(dash_dcc):
app = Dash(__name__)
app.layout = html.Div(
[
html.Label("No Min or Max Slider"),
dcc.Slider(
id="no-min-max-step-slider",
marks={
i: "Label {}".format(i) if i == 1 else str(i) for i in range(1, 6)
},
step=None,
value=5,
vertical=False,
),
],
style={"height": "500px"},
)

dash_dcc.start_server(app)
dash_dcc.wait_for_element("#no-min-max-step-slider")
dash_dcc.percy_snapshot("no-min-max step slider")

dash_dcc.wait_for_element(
'#no-min-max-step-slider div.rc-slider-handle[aria-valuemax="5"]'
)

assert dash_dcc.get_logs() == []

0 comments on commit 94cbc3b

Please # to comment.