ui: Account for padding of parent container during scrollbar layout (#27402)

Closes https://github.com/zed-industries/zed/issues/23386

This PR updates the scrollbar-component to account for padding present
in the parent container.

Since the linked issue was opened,
https://github.com/zed-industries/zed/pull/25288 improved the behaviour
so that the scrollbar does allow scrolling the entire container, however
the scrollbar thumb still does not go the entire way to the bottom. This
can be seen here:


https://github.com/user-attachments/assets/89204355-e6b8-428b-9fa9-bb614051b6fa

This happens because during layouting of the scrollbar, padding of the
parent container is not taken into account. The scrollbar thumb size is
calculated as if no padding was present.

With this change, padding is now included in the calculation, which
resolves the issue:


https://github.com/user-attachments/assets/1d4c62e0-4555-4332-a9ab-4e114684b4b3

The change here is to store the calculated content size during prepaint
_including_ padding and use this for layouting the scrollbar. This
ensures that the actual scroll max and the content size are always in
sync. Furthermore, the existing `TODO`-comment is also resolved, as we
now no longer look at the size of the last child but the actual parent
size instead.

This also removes an existing panic of the scrollbar-component in cases
where the content size was 0, which was previously not accounted for
(this never happened in practice so far, for example because of the
padding added here:

43712285bf/crates/editor/src/hover_popover.rs (L802-L809)
which prevented the container size from ever being 0).

---

Lastly, as I was wiring through the changes of the `content_size` I
noticed that some code was duplicated during the initial layouting as
well as in the click handlers. I refactored this in the second commit to
use `along` where possible as well as computing the new click offset in
one closure which can be passed to both event listeners. As always,
should any of these changes not be wanted, feel free to let me know and
I will revert these.

Looking forward to your feedback 😄 

Release Notes:

- Fixed scrollbars sometimes not scrolling all the way to the bottom.
This commit is contained in:
Finn Evers 2025-05-11 21:40:45 +02:00 committed by GitHub
parent b34f19a46f
commit 82a7aca5a6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 125 additions and 203 deletions

View file

@ -1559,32 +1559,20 @@ impl Interactivity {
) -> Point<Pixels> {
if let Some(scroll_offset) = self.scroll_offset.as_ref() {
let mut scroll_to_bottom = false;
if let Some(scroll_handle) = &self.tracked_scroll_handle {
let mut state = scroll_handle.0.borrow_mut();
state.overflow = style.overflow;
scroll_to_bottom = mem::take(&mut state.scroll_to_bottom);
let mut tracked_scroll_handle = self
.tracked_scroll_handle
.as_ref()
.map(|handle| handle.0.borrow_mut());
if let Some(mut scroll_handle_state) = tracked_scroll_handle.as_deref_mut() {
scroll_handle_state.overflow = style.overflow;
scroll_to_bottom = mem::take(&mut scroll_handle_state.scroll_to_bottom);
}
let rem_size = window.rem_size();
let padding_size = size(
style
.padding
.left
.to_pixels(bounds.size.width.into(), rem_size)
+ style
.padding
.right
.to_pixels(bounds.size.width.into(), rem_size),
style
.padding
.top
.to_pixels(bounds.size.height.into(), rem_size)
+ style
.padding
.bottom
.to_pixels(bounds.size.height.into(), rem_size),
);
let scroll_max = (self.content_size + padding_size - bounds.size).max(&Size::default());
let padding = style.padding.to_pixels(bounds.size.into(), rem_size);
let padding_size = size(padding.left + padding.right, padding.top + padding.bottom);
let padded_content_size = self.content_size + padding_size;
let scroll_max = (padded_content_size - bounds.size).max(&Size::default());
// Clamp scroll offset in case scroll max is smaller now (e.g., if children
// were removed or the bounds became larger).
let mut scroll_offset = scroll_offset.borrow_mut();
@ -1596,6 +1584,10 @@ impl Interactivity {
scroll_offset.y = scroll_offset.y.clamp(-scroll_max.height, px(0.));
}
if let Some(mut scroll_handle_state) = tracked_scroll_handle {
scroll_handle_state.padded_content_size = padded_content_size;
}
*scroll_offset
} else {
Point::default()
@ -2913,6 +2905,7 @@ impl ScrollAnchor {
struct ScrollHandleState {
offset: Rc<RefCell<Point<Pixels>>>,
bounds: Bounds<Pixels>,
padded_content_size: Size<Pixels>,
child_bounds: Vec<Bounds<Pixels>>,
scroll_to_bottom: bool,
overflow: Point<Overflow>,
@ -2975,6 +2968,11 @@ impl ScrollHandle {
self.0.borrow().child_bounds.get(ix).cloned()
}
/// Get the size of the content with padding of the container.
pub fn padded_content_size(&self) -> Size<Pixels> {
self.0.borrow().padded_content_size
}
/// scroll_to_item scrolls the minimal amount to ensure that the child is
/// fully visible
pub fn scroll_to_item(&self, ix: usize) {