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 scrolling behave like you'd expect it to #95

Merged
merged 8 commits into from
Dec 4, 2019

Conversation

Friz64
Copy link
Contributor

@Friz64 Friz64 commented Nov 30, 2019

This PR adds proper scrolling logic to the scrollable widget.
Here i define a "floater" to be the element that you can drag around with your mouse to scroll.
The new behaviour is the following:

  • if the user clicks off the floater, the floater jumps and attaches centered around the cursor
  • if the user clicks on the floater, the floater stays where it is and attaches to the cursor position

@Friz64
Copy link
Contributor Author

Friz64 commented Nov 30, 2019

Ohhh, i completely forgot about the null renderer! Lemme fix this really quickly

Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Thanks @Friz64! I tested it and it works nicely! 🎉

There are a couple of implementation details that I think we should address before merging.

Let me know what you think!

wgpu/src/renderer/widget/scrollable.rs Outdated Show resolved Hide resolved
wgpu/src/renderer/widget/scrollable.rs Show resolved Hide resolved
native/src/widget/scrollable.rs Outdated Show resolved Hide resolved
native/src/widget/scrollable.rs Show resolved Hide resolved
@hecrj
Copy link
Member

hecrj commented Nov 30, 2019

Oh, I forgot to mention that it'd be great if you could squash the WIP commits too into one with a more descriptive message.

@Friz64 Friz64 requested a review from hecrj December 2, 2019 19:00
@hecrj hecrj added the improvement An internal improvement label Dec 3, 2019
@hecrj hecrj added this to the 0.1.0 milestone Dec 3, 2019
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Hey @Friz64,

I have introduced a Scrollbar type and tried to refactor things a bit. I hope I didn't break anything! Let me know what you think.

@Friz64
Copy link
Contributor Author

Friz64 commented Dec 3, 2019

Thank you for those changes, the idea of a Scrollbar type greatly improves the code quality! I just corrected a little oversight in the documentation, it now looks good to me, are we ready to merge this?

@hecrj
Copy link
Member

hecrj commented Dec 4, 2019

Awesome! Yes, let's merge this! 🎉

Thank you again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement An internal improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants