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

Add optional numerical input box to dcc.Slider #944

Open
wants to merge 29 commits into
base: dev
Choose a base branch
from
Open

Add optional numerical input box to dcc.Slider #944

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c858dc9
add input that shows slider value
Feb 25, 2021
6aeea78
sync slider from input
Mar 4, 2021
d57bf70
lint
Mar 4, 2021
7fc4de4
fix lint errors
Mar 4, 2021
e24c3f5
fix lint errors
Mar 4, 2021
796d30d
debounce and components on same line
Mar 6, 2021
c5b7e51
set min width of synced input
Mar 12, 2021
168a99c
pass through props for synced input
Mar 18, 2021
2e2dbe0
let user override default styles for input
Mar 29, 2021
57936d4
run linter
Mar 29, 2021
205b9ed
set proper props for input
Apr 1, 2021
3168a64
refactor input state/props sync
Apr 1, 2021
128017b
update tests
Apr 6, 2021
b555515
Merge branch 'dev' into synced-input-slider
Apr 6, 2021
3e62912
add tests
Apr 6, 2021
3820ce4
lint tests
Apr 6, 2021
884cccb
run prettier
Apr 6, 2021
403ee9e
small fixups
Apr 6, 2021
761103c
make sure that pressing enter when input is focused changes state of …
Apr 8, 2021
1709c21
clear debounce timeout after every state sync
Apr 8, 2021
2db424b
Merge branch 'dev' into synced-input-slider
Apr 13, 2021
da7d116
Merge branch 'synced-input-slider' of https://github.com/plotly/dash-…
Apr 13, 2021
4b45284
refactor synced input event handling
Apr 13, 2021
b7c3154
Merge branch 'dev' into synced-input-slider
Apr 20, 2021
aa1ab08
Merge branch 'dev' into synced-input-slider
Apr 27, 2021
0111a74
camelCase to snake_case new props
Apr 27, 2021
02c10c6
Merge branch 'dev' into synced-input-slider
wbrgss May 14, 2021
aebce8d
add some ui ;ogic
May 21, 2021
f445f58
Merge branch 'synced-input-slider' of https://github.com/plotly/dash-…
May 21, 2021
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
34 changes: 33 additions & 1 deletion src/components/Slider.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,40 @@ Slider.propTypes = {
}),

/**
* Value by which increments or decrements are made
* Value by which increments or decrements are made.
*/
step: PropTypes.number,

/**
* If true, display an Input component whose value is synced with the Slider's value.
*/
syncedInput: PropTypes.bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Per the discussion @chriddyp and I had just now and summarized on Slack: let's convert all these new props to snake_case. In the short term this will make slider a little funny since it already has some camelCase props. That's OK, we'll get to it soon, and adding the backward-compatible conversion of other props is going to take some more work, especially since we use omit in this component.


/**
* The classname to be given to the synced Input component.
*/
syncedInputClassName: PropTypes.string,

/**
* The CSS to be applied to the class of the input (div).
*/
syncedInputStyle: PropTypes.object,

/**
* The id to be applied to the input (div). Default is "syncedInput".
*/
syncedInputID: PropTypes.string,

/**
* The CSS to be applied to the class of the slider (div).
*/
style: PropTypes.object,

/**
* The amount of time the synced Input should wait before passing along state changes without a change of focus or the user pressing Enter. In milliseconds.
*/
syncedInputDebounceTime: PropTypes.number,

/**
* If true, the slider will be vertical
*/
Expand Down Expand Up @@ -199,6 +229,8 @@ Slider.defaultProps = {
persisted_props: ['value'],
persistence_type: 'local',
verticalHeight: 400,
syncedInputDebounceTime: 450,
syncedInputID: 'syncedInput',
};

export const propTypes = Slider.propTypes;
Expand Down
78 changes: 77 additions & 1 deletion src/fragments/Slider.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,30 @@ export default class Slider extends Component {
: ReactSlider;
this._computeStyle = computeSliderStyle();
this.state = {value: props.value};
this.syncInputWithSlider = this.syncInputWithSlider.bind(this);
this.input = React.createRef();
}

syncInputWithSlider() {
if (this.input.current.value > this.props.max) {
this.input.current.value = this.props.max;
}

if (this.input.current.value < this.props.min) {
this.input.current.value = this.props.min;
}

if (this.timeout) {
clearTimeout(this.timeout);
}

const valueAsNumber = Number(this.input.current.value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jdamiba looking good! Playing with this using pytest -k slsl008 --pause - a little bit of behavior we might want to tweak: It's possible to delete the text in the input box (in which case it gets treated as zero)
Screen Shot 2021-04-21 at 4 27 06 PM

or enter a value that's incompatible with the step
Screen Shot 2021-04-21 at 4 26 55 PM

Also: if you enter an out-of-range number, after the timeout time it will be replaced by the closest limit value. This is a problem if the limit is bigger than zero, it can prevent you from typing in the number you want. For example set min=10, max=20 and try to type 13 - if you're quick you can get it in various ways, but if you're slower than the timeout you'll end up with 10, then you might type the 3 making 103 which gets pushed to 20.

In all of these cases I think we should allow the value to stay in the input box until blur, and NOT update the slider or the prop. Then on blur my gut reaction is:

  • if you cleared the value, reset to the previous prop value. There's also a case where you type a partial number that's not a number, for example "2e" is allowed because "2e3" etc is valid. Also "." - seems like all of these should be treated the same way, reset to the previous prop value.
  • if you're out of range, push to the closest limit
  • if you're between steps, round to the nearest step

Finally, the reason I commented on this particular line: you're casting to number here, but you already used it above as though it was a number, when it was still a string.


this.setState({value: valueAsNumber});
this.props.setProps({
value: valueAsNumber,
drag_value: valueAsNumber,
});
}

UNSAFE_componentWillReceiveProps(newProps) {
Expand Down Expand Up @@ -47,6 +71,13 @@ export default class Slider extends Component {
setProps,
tooltip,
updatemode,
syncedInput,
syncedInputDebounceTime,
syncedInputClassName,
syncedInputStyle,
syncedInputID,
style,
step,
vertical,
verticalHeight,
} = this.props;
Expand All @@ -72,15 +103,54 @@ export default class Slider extends Component {
)
: this.props.marks;

const computedStyle = this._computeStyle(
vertical,
verticalHeight,
tooltip
);

const defaultInputStyle = {
width: '60px',
marginRight: vertical && syncedInput ? '' : '25px',
marginBottom: vertical && syncedInput ? '25px' : '',
};

return (
<div
id={id}
data-dash-is-loading={
(loading_state && loading_state.is_loading) || undefined
}
className={className}
style={this._computeStyle(vertical, verticalHeight, tooltip)}
style={{...computedStyle, ...style}}
>
{syncedInput ? (
<input
onChange={() => {
this.timeout = setTimeout(
function() {
this.syncInputWithSlider();
}.bind(this),
syncedInputDebounceTime
);
}}
onBlur={() => {
this.syncInputWithSlider();
}}
onKeyPress={event => {
if (event.key === 'Enter') {
this.syncInputWithSlider();
}
}}
type="number"
defaultValue={value}
step={step}
className={syncedInputClassName}
id={syncedInputID}
style={{...defaultInputStyle, ...syncedInputStyle}}
ref={this.input}
/>
) : null}
<this.DashSlider
onChange={value => {
if (updatemode === 'drag') {
Expand All @@ -89,11 +159,17 @@ export default class Slider extends Component {
this.setState({value: value});
setProps({drag_value: value});
}
if (syncedInput) {
this.input.current.value = value;
}
}}
onAfterChange={value => {
if (updatemode === 'mouseup') {
setProps({value});
}
if (syncedInput) {
this.input.current.value = value;
}
}}
/*
if/when rc-slider or rc-tooltip are updated to latest versions,
Expand Down
3 changes: 3 additions & 0 deletions src/utils/computeSliderStyle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ export default () => {
return memoizeWith(identity, (vertical, verticalHeight, tooltip) => {
const style = {
padding: '25px',
display: 'flex',
};

if (vertical) {
style.height = verticalHeight + 'px';
style.flexDirection = 'column';

if (
!tooltip ||
Expand All @@ -28,6 +30,7 @@ export default () => {
) {
style.paddingTop = '0px';
}
style.alignItems = 'center';
}

return style;
Expand Down
99 changes: 99 additions & 0 deletions tests/integration/sliders/test_sliders.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,3 +241,102 @@ def update_output(value):
dash_dcc.wait_for_text_to_equal("#out-value", "You have selected 5-15")
dash_dcc.release()
dash_dcc.wait_for_text_to_equal("#out-value", "You have selected 10-15")


def test_slsl008_horizontal_slider_with_input(dash_dcc):
app = dash.Dash(__name__)
app.layout = html.Div(
[
dcc.Slider(
id="slider",
min=0,
max=20,
step=1,
value=5,
syncedInput=True,
syncedInputID="syncedInput",
tooltip={"always_visible": True},
),
html.Div(id="out"),
]
)

@app.callback(Output("out", "children"), [Input("slider", "value")])
def update_output(value):
return "You have selected {}".format(value)

dash_dcc.start_server(app)
dash_dcc.wait_for_text_to_equal("#out", "You have selected 5")

input_ = dash_dcc.find_element("#syncedInput")

input_.clear()
input_.send_keys("8")

dash_dcc.wait_for_text_to_equal("#out", "You have selected 8")


def test_slsl009_vertical_slider_with_input(dash_dcc):
app = dash.Dash(__name__)
app.layout = html.Div(
[
dcc.Slider(
id="slider",
min=0,
max=20,
step=1,
value=5,
vertical=True,
syncedInput=True,
syncedInputID="arbitraryID",
tooltip={"always_visible": True},
),
html.Div(id="out"),
]
)

@app.callback(Output("out", "children"), [Input("slider", "value")])
def update_output(value):
return "You have selected {}".format(value)

dash_dcc.start_server(app)
dash_dcc.wait_for_text_to_equal("#out", "You have selected 5")

input_ = dash_dcc.find_element("#arbitraryID")

input_.clear()
input_.send_keys("8")

dash_dcc.wait_for_text_to_equal("#out", "You have selected 8")


def test_slsl010_horizontal_slider_with_input(dash_dcc):
app = dash.Dash(__name__)
app.layout = html.Div(
[
dcc.Slider(
id="slider",
min=0,
max=20,
step=1,
value=5,
vertical=True,
syncedInput=True,
syncedInputClassName="slider-input",
syncedInputID="arbitraryID",
tooltip={"always_visible": True},
),
html.Div(id="out"),
]
)

@app.callback(Output("out", "children"), [Input("slider", "value")])
def update_output(value):
return "You have selected {}".format(value)

dash_dcc.start_server(app)
dash_dcc.wait_for_text_to_equal("#out", "You have selected 5")

input_ = dash_dcc.find_element("#arbitraryID")

assert input_.get_attribute("class") == "slider-input"
55 changes: 55 additions & 0 deletions tests/test_integration_1.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,61 @@ def test_vertical_slider(self):
for entry in self.get_log():
raise Exception("browser error logged during test", entry)

def test_horizontal_slider_with_input(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like a great test, but let's not add any more tests to the top-level files test_integration_1 or test_integration_2. These use the legacy unittest framework rather than dash.testing. If anything we should take opportunities when we're working in neighboring code to move more tests OUT of these files and port them to use dash.testing in the subdirectories.

app = dash.Dash(__name__)

app.layout = html.Div(
[
html.Label("Horizontal Slider with Input"),
dcc.Slider(
id="horizontal-slider-with-input",
min=0,
max=9,
value=5,
syncedInputClassName="arbitraryClassName",
syncedInput=True,
),
],
style={"height": "500px"},
)
self.startServer(app)

self.wait_for_element_by_css_selector("#horizontal-slider-with-input")
self.wait_for_element_by_css_selector(".arbitraryClassName")

self.snapshot("horizontal slider with input")

for entry in self.get_log():
raise Exception("browser error logged during test", entry)

def test_vertical_slider_with_input(self):
app = dash.Dash(__name__)

app.layout = html.Div(
[
html.Label("Vertical Slider with Input"),
dcc.Slider(
id="vertical-slider-with-input",
min=0,
max=9,
value=5,
vertical=True,
syncedInputClassName="arbitraryClassName",
syncedInput=True,
),
],
style={"height": "500px"},
)
self.startServer(app)

self.wait_for_element_by_css_selector("#vertical-slider-with-input")
self.wait_for_element_by_css_selector(".arbitraryClassName")

self.snapshot("vertical slider with input")

for entry in self.get_log():
raise Exception("browser error logged during test", entry)

def test_loading_range_slider(self):
lock = Lock()

Expand Down