Fix slider change event issue with tapping #3617
Conversation
Signed-off-by: Itay Dafna <[email protected]>
|
|
|
@ibdafna - it does look like the linting test doesn't pass, so we do need to run the lint on these changes. |
Signed-off-by: Itay Dafna <[email protected]>
There was a problem hiding this comment.
The code changes themselves look good, but I would maybe want you to do an extra pass on the docstrings? Some of them don't match anymore, some are missing, and some could be made more precise (e.g. the update event happens for more than just the slide event, but it is maybe the most common case, and the change event happens for more than just mouse up).
| const actual_value = Math.pow(base, this._validate_slide_value(values[0])); | ||
| this.readout.textContent = this.valueToString(actual_value); | ||
|
|
||
| this.handleSliderChanged(values, handle); |
There was a problem hiding this comment.
I guess one problem that can occur is that when you tap or move by the arrow keys, you will get two events: one from the update event, and one from the change event. If we wanted to avoid this, we could conditionally only pass the event on if continuous_update is false here (and other similar event handlers). I'm just not sure how bad of an issue this duplicate is, and if it can lead to some issues with multiple users, or if backbone will automatically discard the second change as a no-op.
|
Just to confirm that this fixes #3597 also in our setting. Awesome. It'd be great to have this merged in the near future, as we need this to deploy an interactive course that relies heavily on sliders. |
|
This PR is released in ipywidgets 8.0.4 |
Signed-off-by: Itay Dafna [email protected]
Addresses #3597