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.

Sign up for GitHub

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

Jump to bottom

Improve layout of the scrollbar #1715

Merged
merged 12 commits into from
Apr 19, 2021
Merged

Improve layout of the scrollbar #1715

merged 12 commits into from
Apr 19, 2021

Conversation

andrewhickman
Copy link
Contributor

This PR fixes the scrollbar clipping observed here #1709 (comment). There are a few small changes, best viewed as individual commits

  • Merged some code which was duplicated for horizontal/vertical cases
  • I did a bunch of testing with super small scroll components and fixed some cases where the scrollbar was drawn with negative size or outside the boundaries
  • The scroll component reserves some vertical padding space for the horizontal scroll bar (and vice versa). I've changed it to only do this if both scroll bars are enabled, otherwise it looks a bit odd
  • TextBox wasn't actually benefiting from the last point because it sets Scroll::set_horizontal_scroll_enabled but that didn't change whether the scrollbar is actually enabled - that is now fixed.

@andrewhickman
Remove some horizontal/vertical code duplication in ScrollComponent
094ad57
@andrewhickman
Handle scroll bar edge cases better
42cd1ec 
- Don't draw out-of-bounds when the widget is smaller than theme::SCROLLBAR_MIN_SIZE
- Don't draw the scroll bar if the widget is smaller than the padding

Fixes #1709.
@andrewhickman
Don't pad scrollbar for other bar if it is disabled
0536037
@andrewhickman
Disable the scroll bar when disabling scrolling in Scroll
11cdf93 
This allows TextBox (and other widgets
which disable scrolling dynamically) to use the reduced padding from 217fabf.
@andrewhickman
Fix scrollbar length being shortened slightly
0d5fc6a
@andrewhickman
Add tests for ScrollComponent
c48eeda
@andrewhickman
Fix clippy::float_cmp errors
2316c59
@SecondFlight
Copy link
Collaborator

Hey, looks like this is missing a changelog update. Just want to make sure that it's intentional or otherwise that you get a chance to add this there.

@SecondFlight SecondFlight added the S-needs-review waits for review label Apr 12, 2021
@andrewhickman
Copy link
Contributor Author

Hey, looks like this is missing a changelog update. Just want to make sure that it's intentional or otherwise that you get a chance to add this there.

Completely forgot, thanks 👍 . I've updated it.

xarvic
xarvic approved these changes Apr 19, 2021
View reviewed changes
Copy link
Collaborator

@xarvic xarvic left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, thank you!

@xarvic xarvic merged commit 2fd7b7e into linebender:master Apr 19, 2021
@andrewhickman andrewhickman deleted the scroll-fixes branch April 19, 2021 21:59
@maan2003 maan2003 removed the S-needs-review waits for review label May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xarvic xarvic xarvic approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@andrewhickman @SecondFlight @xarvic @maan2003