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

Remove problematic clip code from ScrollArea #2839

Closed
wants to merge 1 commit into from
Closed

Remove problematic clip code from ScrollArea #2839

wants to merge 1 commit into from

Conversation

Barugon
Copy link
Contributor

@Barugon Barugon commented Mar 25, 2023

This code causes incorrect clipping. It also uses incorrect indexing (1 - d instead of just d). With this code removed, I have not been able to find a case where the scroll area contents invade the scroll bar area.

Closes #2811.

@Barugon Barugon changed the title Remove unnecessary clip code from ScrollArea Remove problematic clip code from ScrollArea Mar 25, 2023
Comment on lines -451 to -456
if state.show_scroll[d] {
// Make sure content doesn't cover scroll bars
let tiny_gap = 1.0;
content_clip_rect.max[1 - d] =
inner_rect.max[1 - d] + scroll_bar_inner_margin - tiny_gap;
}
Copy link
Owner

Choose a reason for hiding this comment

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

The 1-d is not incorrect. When we have a horizontal scroll-bar (d = 0), the scrollbar itself is at he bottom of the vertical dimension (1-d), and the other way around.

I believe this code was added to handle nested scroll-areas. Have you tried that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have not. I'll tinker with it and see what I can find.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this code was added to handle nested scroll-areas. Have you tried that?

Do you have some code to specifically test this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I mean is something that specifically shows what the change was trying to correct.

@Barugon
Copy link
Contributor Author

Barugon commented Mar 30, 2023

Closing this for now.

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

Successfully merging this pull request may close these issues.

ScrollArea ignores Spacing::scroll_bar_inner_margin for vertical scrollbar
2 participants