Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make slider scrollable by mouse wheel #2104

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

gordonshieh
Copy link

When hovered over a slider, and a user scrolls with their mouse wheel, the slider will move

@jneem
Copy link
Collaborator

jneem commented Jan 4, 2022

Thanks for the PR! We have a long-standing slider improvement PR that I'd like to get in first, though (#1979). Maybe this can be rebased on that afterwards?

Copy link
Collaborator

@xarvic xarvic left a comment

Choose a reason for hiding this comment

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

Looks good! Could you add your changes to the Changelog.

let increment = if let Some(step) = self.step {
step
} else {
0.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

In some cases the difference between min and max is smaller than 0.1. This value should depend on min and max.

@gordonshieh
Copy link
Author

@jneem Sure. I'm in no rush to get this PR in.
@xarvic Updated the step to be 10% of the min and max range; also updated the changelog

@jneem
Copy link
Collaborator

jneem commented Jan 5, 2022

Ok, I merged #1979 so that isn't a blocker anymore. But you will need to rebase, because Slider changed a lot...

@gordonshieh
Copy link
Author

Cool. Also planning to add mouse wheel support to the RangeSlider too then.

@xarvic
Copy link
Collaborator

xarvic commented Jan 10, 2022

Scrolling the RangeSlider feels a little strange. If the knob moves too far away, the slider starts moving the other knob. I think unless the mouse is moved we should always move the same Knob when scrolling.

2022-01-10.20-29-18.mp4

self.mapping
.calculate_value(me.pos, knob_size, ctx.size(), 0.0);

if press_value - data.0 < data.1 - press_value {
Copy link
Collaborator

@xarvic xarvic Jan 10, 2022

Choose a reason for hiding this comment

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

We need to remember which knob was scrolled last

Copy link
Member

@xStrom xStrom left a comment

Choose a reason for hiding this comment

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

What's the status here? @gordonshieh do you still want to move forward with this PR?

Please rebase on master so that we can resolve the merge conflicts.

@@ -64,6 +64,7 @@ You can find its changes [documented below](#070---2021-01-01).
- Scope: expose scoped state using state() and state_mut() ([#2082] by [@rjwittams]
- Tabs: allow getting and setting the tab index of a Tabs widget ([#2082] by [@rjwittams]
- `RangeSlider` and `Annotated` ([#1979] by [@xarvic])
- `Slider` widget now scrollable by mouse wheel ([#2104] by [@gordonshieh])
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately the links aren't automatically generated. Please manually define both [#2104] and [@gordonshieh] at the bottom of the file.

@xStrom xStrom added widget concerns a particular widget S-waiting-on-author waits for changes from the submitter labels Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author waits for changes from the submitter widget concerns a particular widget
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants