Reapply "ui: Account for padding of parent container during scrollbar layout" (#30577)

This PR reapplies #27402 which was reverted in
https://github.com/zed-industries/zed/pull/30544 due to the issue
@ConradIrwin reported in
https://github.com/zed-industries/zed/pull/27402#issuecomment-2871745132.
The reported issue is already present on main but not visible, see
https://github.com/zed-industries/zed/pull/27402#issuecomment-2872546903
for more context and reproduction steps.

The fix here was to move the padding for the hover popover up to the
parent container. This does not fix the underlying problem but serves as
workaround without any disadvantages until a better solution is found. I
would currently guess that the underlying issue might be related to some
rem-size calculations for small font sizes or something similar (e.g.
https://github.com/zed-industries/zed/pull/22732 could possibly be
somewhat related).

Notably, the fix here does not cause any difference in layouting (the
following screenshots are actually distinct images), yet fixes the
problem at hand.

### Default font size (`15px`) 

| `main` | This PR |
| --- | --- |
|
![main_large](https://github.com/user-attachments/assets/66d38827-9023-4f78-9ceb-54fb13c21e41)
|![PR](https://github.com/user-attachments/assets/7af82bd2-2732-4cba-8d4b-54605d6ff101)
|

### Smaller font size (`12px`)

| `main` | This PR |
| --- | --- |
|
![pr_large](https://github.com/user-attachments/assets/d43be6e6-6840-422c-baf0-368aab733dac)
|
![PR](https://github.com/user-attachments/assets/43f60b2b-2578-45d2-bcab-44edf2612ce2)
|

Furthermore, for the second scenario, the popover would be scrollable on
main. As there is no scrollbar in the second image for this PR, this no
longer happens with this branch.


Release Notes:

- N/A

crates/editor/src/hover_popover.rs

@@ -884,6 +884,7 @@ impl InfoPopover {
                 *keyboard_grace = false;
                 cx.stop_propagation();
             })
+            .p_2()
             .when_some(self.parsed_content.clone(), |this, markdown| {
                 this.child(
                     div()
@@ -891,7 +892,6 @@ impl InfoPopover {
                         .overflow_y_scroll()
                         .max_w(max_size.width)
                         .max_h(max_size.height)
-                        .p_2()
                         .track_scroll(&self.scroll_handle)
                         .child(
                             MarkdownElement::new(markdown, hover_markdown_style(window, cx))

crates/gpui/src/elements/div.rs

@@ -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 tracked_scroll_handle = self
-                let mut state = scroll_handle.0.borrow_mut();
+                .tracked_scroll_handle
-                state.overflow = style.overflow;
+                .as_ref()
-                scroll_to_bottom = mem::take(&mut state.scroll_to_bottom);
+                .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(
+            let padding = style.padding.to_pixels(bounds.size.into(), rem_size);
-                style
+            let padding_size = size(padding.left + padding.right, padding.top + padding.bottom);
-                    .padding
+            let padded_content_size = self.content_size + padding_size;
-                    .left
+            let scroll_max = (padded_content_size - bounds.size).max(&Size::default());
-                    .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());
             // 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) {

crates/terminal_view/src/terminal_scrollbar.rs

@@ -3,9 +3,9 @@ use std::{
     rc::Rc,
 };
 
-use gpui::{Bounds, Point, size};
+use gpui::{Bounds, Point, Size, size};
 use terminal::Terminal;
-use ui::{ContentSize, Pixels, ScrollableHandle, px};
+use ui::{Pixels, ScrollableHandle, px};
 
 #[derive(Debug)]
 struct ScrollHandleState {
@@ -46,12 +46,9 @@ impl TerminalScrollHandle {
 }
 
 impl ScrollableHandle for TerminalScrollHandle {
-    fn content_size(&self) -> Option<ContentSize> {
+    fn content_size(&self) -> Size<Pixels> {
         let state = self.state.borrow();
-        Some(ContentSize {
+        size(Pixels::ZERO, state.total_lines as f32 * state.line_height)
-            size: size(px(0.), px(state.total_lines as f32 * state.line_height.0)),
-            scroll_adjustment: Some(Point::new(px(0.), px(0.))),
-        })
     }
 
     fn offset(&self) -> Point<Pixels> {

crates/ui/src/components/scrollbar.rs

@@ -3,9 +3,9 @@ use std::{any::Any, cell::Cell, fmt::Debug, ops::Range, rc::Rc, sync::Arc};
 use crate::{IntoElement, prelude::*, px, relative};
 use gpui::{
     Along, App, Axis as ScrollbarAxis, BorderStyle, Bounds, ContentMask, Corners, Edges, Element,
-    ElementId, Entity, EntityId, GlobalElementId, Hitbox, Hsla, LayoutId, ListState,
+    ElementId, Entity, EntityId, GlobalElementId, Hitbox, Hsla, IsZero, LayoutId, ListState,
     MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels, Point, ScrollHandle, ScrollWheelEvent,
-    Size, Style, UniformListScrollHandle, Window, point, quad,
+    Size, Style, UniformListScrollHandle, Window, quad,
 };
 
 pub struct Scrollbar {
@@ -15,11 +15,8 @@ pub struct Scrollbar {
 }
 
 impl ScrollableHandle for UniformListScrollHandle {
-    fn content_size(&self) -> Option<ContentSize> {
+    fn content_size(&self) -> Size<Pixels> {
-        Some(ContentSize {
+        self.0.borrow().base_handle.content_size()
-            size: self.0.borrow().last_item_size.map(|size| size.contents)?,
-            scroll_adjustment: None,
-        })
     }
 
     fn set_offset(&self, point: Point<Pixels>) {
@@ -36,11 +33,8 @@ impl ScrollableHandle for UniformListScrollHandle {
 }
 
 impl ScrollableHandle for ListState {
-    fn content_size(&self) -> Option<ContentSize> {
+    fn content_size(&self) -> Size<Pixels> {
-        Some(ContentSize {
+        self.content_size_for_scrollbar()
-            size: self.content_size_for_scrollbar(),
-            scroll_adjustment: None,
-        })
     }
 
     fn set_offset(&self, point: Point<Pixels>) {
@@ -65,27 +59,8 @@ impl ScrollableHandle for ListState {
 }
 
 impl ScrollableHandle for ScrollHandle {
-    fn content_size(&self) -> Option<ContentSize> {
+    fn content_size(&self) -> Size<Pixels> {
-        let last_children_index = self.children_count().checked_sub(1)?;
+        self.padded_content_size()
-
-        let mut last_item = self.bounds_for_item(last_children_index)?;
-        let mut scroll_adjustment = None;
-
-        if last_children_index != 0 {
-            // todo: PO: this is slightly wrong for horizontal scrollbar, as the last item is not necessarily the longest one.
-            let first_item = self.bounds_for_item(0)?;
-            last_item.size.height += last_item.origin.y;
-            last_item.size.width += last_item.origin.x;
-
-            scroll_adjustment = Some(first_item.origin);
-            last_item.size.height -= first_item.origin.y;
-            last_item.size.width -= first_item.origin.x;
-        }
-
-        Some(ContentSize {
-            size: last_item.size,
-            scroll_adjustment,
-        })
     }
 
     fn set_offset(&self, point: Point<Pixels>) {
@@ -101,14 +76,8 @@ impl ScrollableHandle for ScrollHandle {
     }
 }
 
-#[derive(Debug)]
-pub struct ContentSize {
-    pub size: Size<Pixels>,
-    pub scroll_adjustment: Option<Point<Pixels>>,
-}
-
 pub trait ScrollableHandle: Any + Debug {
-    fn content_size(&self) -> Option<ContentSize>;
+    fn content_size(&self) -> Size<Pixels>;
     fn set_offset(&self, point: Point<Pixels>);
     fn offset(&self) -> Point<Pixels>;
     fn viewport(&self) -> Bounds<Pixels>;
@@ -149,30 +118,26 @@ impl ScrollbarState {
     }
 
     fn thumb_range(&self, axis: ScrollbarAxis) -> Option<Range<f32>> {
-        const MINIMUM_THUMB_SIZE: f32 = 25.;
+        const MINIMUM_THUMB_SIZE: Pixels = px(25.);
-        let ContentSize {
+        let content_size = self.scroll_handle.content_size().along(axis);
-            size: main_dimension_size,
+        let viewport_size = self.scroll_handle.viewport().size.along(axis);
-            scroll_adjustment,
+        if content_size.is_zero() || viewport_size.is_zero() || content_size < viewport_size {
-        } = self.scroll_handle.content_size()?;
-        let content_size = main_dimension_size.along(axis).0;
-        let mut current_offset = self.scroll_handle.offset().along(axis).min(px(0.)).abs().0;
-        if let Some(adjustment) = scroll_adjustment.and_then(|adjustment| {
-            let adjust = adjustment.along(axis).0;
-            if adjust < 0.0 { Some(adjust) } else { None }
-        }) {
-            current_offset -= adjustment;
-        }
-        let viewport_size = self.scroll_handle.viewport().size.along(axis).0;
-        if content_size < viewport_size {
             return None;
         }
+
+        let max_offset = content_size - viewport_size;
+        let current_offset = self
+            .scroll_handle
+            .offset()
+            .along(axis)
+            .clamp(-max_offset, Pixels::ZERO)
+            .abs();
+
         let visible_percentage = viewport_size / content_size;
         let thumb_size = MINIMUM_THUMB_SIZE.max(viewport_size * visible_percentage);
         if thumb_size > viewport_size {
             return None;
         }
-        let max_offset = content_size - viewport_size;
-        current_offset = current_offset.clamp(0., max_offset);
         let start_offset = (current_offset / max_offset) * (viewport_size - thumb_size);
         let thumb_percentage_start = start_offset / viewport_size;
         let thumb_percentage_end = (start_offset + thumb_size) / viewport_size;
@@ -247,57 +212,38 @@ impl Element for Scrollbar {
         window: &mut Window,
         cx: &mut App,
     ) {
+        const EXTRA_PADDING: Pixels = px(5.0);
         window.with_content_mask(Some(ContentMask { bounds }), |window| {
+            let axis = self.kind;
             let colors = cx.theme().colors();
             let thumb_background = colors
                 .surface_background
                 .blend(colors.scrollbar_thumb_background);
-            let is_vertical = self.kind == ScrollbarAxis::Vertical;
-            let extra_padding = px(5.0);
-            let padded_bounds = if is_vertical {
-                Bounds::from_corners(
-                    bounds.origin + point(Pixels::ZERO, extra_padding),
-                    bounds.bottom_right() - point(Pixels::ZERO, extra_padding * 3),
-                )
-            } else {
-                Bounds::from_corners(
-                    bounds.origin + point(extra_padding, Pixels::ZERO),
-                    bounds.bottom_right() - point(extra_padding * 3, Pixels::ZERO),
-                )
-            };
 
-            let mut thumb_bounds = if is_vertical {
+            let padded_bounds = Bounds::from_corners(
-                let thumb_offset = self.thumb.start * padded_bounds.size.height;
+                bounds
-                let thumb_end = self.thumb.end * padded_bounds.size.height;
+                    .origin
-                let thumb_upper_left = point(
+                    .apply_along(axis, |origin| origin + EXTRA_PADDING),
-                    padded_bounds.origin.x,
+                bounds
-                    padded_bounds.origin.y + thumb_offset,
+                    .bottom_right()
-                );
+                    .apply_along(axis, |track_end| track_end - 3.0 * EXTRA_PADDING),
-                let thumb_lower_right = point(
+            );
-                    padded_bounds.origin.x + padded_bounds.size.width,
+
-                    padded_bounds.origin.y + thumb_end,
+            let thumb_offset = self.thumb.start * padded_bounds.size.along(axis);
-                );
+            let thumb_end = self.thumb.end * padded_bounds.size.along(axis);
-                Bounds::from_corners(thumb_upper_left, thumb_lower_right)
+
-            } else {
+            let thumb_bounds = Bounds::new(
-                let thumb_offset = self.thumb.start * padded_bounds.size.width;
+                padded_bounds
-                let thumb_end = self.thumb.end * padded_bounds.size.width;
+                    .origin
-                let thumb_upper_left = point(
+                    .apply_along(axis, |origin| origin + thumb_offset),
-                    padded_bounds.origin.x + thumb_offset,
+                padded_bounds
-                    padded_bounds.origin.y,
+                    .size
-                );
+                    .apply_along(axis, |_| thumb_end - thumb_offset)
-                let thumb_lower_right = point(
+                    .apply_along(axis.invert(), |width| width / 1.5),
-                    padded_bounds.origin.x + thumb_end,
+            );
-                    padded_bounds.origin.y + padded_bounds.size.height,
+
-                );
+            let corners = Corners::all(thumb_bounds.size.along(axis.invert()) / 2.0);
-                Bounds::from_corners(thumb_upper_left, thumb_lower_right)
+
-            };
-            let corners = if is_vertical {
-                thumb_bounds.size.width /= 1.5;
-                Corners::all(thumb_bounds.size.width / 2.0)
-            } else {
-                thumb_bounds.size.height /= 1.5;
-                Corners::all(thumb_bounds.size.height / 2.0)
-            };
             window.paint_quad(quad(
                 thumb_bounds,
                 corners,
@@ -308,7 +254,39 @@ impl Element for Scrollbar {
             ));
 
             let scroll = self.state.scroll_handle.clone();
-            let axis = self.kind;
+
+            enum ScrollbarMouseEvent {
+                GutterClick,
+                ThumbDrag(Pixels),
+            }
+
+            let compute_click_offset =
+                move |event_position: Point<Pixels>,
+                      item_size: Size<Pixels>,
+                      event_type: ScrollbarMouseEvent| {
+                    let viewport_size = padded_bounds.size.along(axis);
+
+                    let thumb_size = thumb_bounds.size.along(axis);
+
+                    let thumb_offset = match event_type {
+                        ScrollbarMouseEvent::GutterClick => thumb_size / 2.,
+                        ScrollbarMouseEvent::ThumbDrag(thumb_offset) => thumb_offset,
+                    };
+
+                    let thumb_start = (event_position.along(axis)
+                        - padded_bounds.origin.along(axis)
+                        - thumb_offset)
+                        .clamp(px(0.), viewport_size - thumb_size);
+
+                    let max_offset = (item_size.along(axis) - viewport_size).max(px(0.));
+                    let percentage = if viewport_size > thumb_size {
+                        thumb_start / (viewport_size - thumb_size)
+                    } else {
+                        0.
+                    };
+
+                    -max_offset * percentage
+                };
 
             window.on_mouse_event({
                 let scroll = scroll.clone();
@@ -323,39 +301,17 @@ impl Element for Scrollbar {
                     if thumb_bounds.contains(&event.position) {
                         let offset = event.position.along(axis) - thumb_bounds.origin.along(axis);
                         state.drag.set(Some(offset));
-                    } else if let Some(ContentSize {
+                    } else {
-                        size: item_size, ..
+                        let click_offset = compute_click_offset(
-                    }) = scroll.content_size()
+                            event.position,
-                    {
+                            scroll.content_size(),
-                        let click_offset = {
+                            ScrollbarMouseEvent::GutterClick,
-                            let viewport_size = padded_bounds.size.along(axis);
+                        );
-
+                        scroll.set_offset(scroll.offset().apply_along(axis, |_| click_offset));
-                            let thumb_size = thumb_bounds.size.along(axis);
-                            let thumb_start = (event.position.along(axis)
-                                - padded_bounds.origin.along(axis)
-                                - (thumb_size / 2.))
-                                .clamp(px(0.), viewport_size - thumb_size);
-
-                            let max_offset = (item_size.along(axis) - viewport_size).max(px(0.));
-                            let percentage = if viewport_size > thumb_size {
-                                thumb_start / (viewport_size - thumb_size)
-                            } else {
-                                0.
-                            };
-
-                            -max_offset * percentage
-                        };
-                        match axis {
-                            ScrollbarAxis::Horizontal => {
-                                scroll.set_offset(point(click_offset, scroll.offset().y));
-                            }
-                            ScrollbarAxis::Vertical => {
-                                scroll.set_offset(point(scroll.offset().x, click_offset));
-                            }
-                        }
                     }
                 }
             });
+
             window.on_mouse_event({
                 let scroll = scroll.clone();
                 move |event: &ScrollWheelEvent, phase, window, _| {
@@ -367,44 +323,19 @@ impl Element for Scrollbar {
                     }
                 }
             });
+
             let state = self.state.clone();
-            let axis = self.kind;
             window.on_mouse_event(move |event: &MouseMoveEvent, _, window, cx| {
                 if let Some(drag_state) = state.drag.get().filter(|_| event.dragging()) {
-                    if let Some(ContentSize {
+                    let drag_offset = compute_click_offset(
-                        size: item_size, ..
+                        event.position,
-                    }) = scroll.content_size()
+                        scroll.content_size(),
-                    {
+                        ScrollbarMouseEvent::ThumbDrag(drag_state),
-                        let drag_offset = {
+                    );
-                            let viewport_size = padded_bounds.size.along(axis);
+                    scroll.set_offset(scroll.offset().apply_along(axis, |_| drag_offset));
-
+                    window.refresh();
-                            let thumb_size = thumb_bounds.size.along(axis);
+                    if let Some(id) = state.parent_id {
-                            let thumb_start = (event.position.along(axis)
+                        cx.notify(id);
-                                - padded_bounds.origin.along(axis)
-                                - drag_state)
-                                .clamp(px(0.), viewport_size - thumb_size);
-
-                            let max_offset = (item_size.along(axis) - viewport_size).max(px(0.));
-                            let percentage = if viewport_size > thumb_size {
-                                thumb_start / (viewport_size - thumb_size)
-                            } else {
-                                0.
-                            };
-
-                            -max_offset * percentage
-                        };
-                        match axis {
-                            ScrollbarAxis::Horizontal => {
-                                scroll.set_offset(point(drag_offset, scroll.offset().y));
-                            }
-                            ScrollbarAxis::Vertical => {
-                                scroll.set_offset(point(scroll.offset().x, drag_offset));
-                            }
-                        };
-                        window.refresh();
-                        if let Some(id) = state.parent_id {
-                            cx.notify(id);
-                        }
                     }
                 } else {
                     state.drag.set(None);

crates/workspace/src/pane.rs

@@ -2671,11 +2671,7 @@ impl Pane {
                 }
             })
             .children(pinned_tabs.len().ne(&0).then(|| {
-                let content_width = self
+                let content_width = self.tab_bar_scroll_handle.content_size().width;
-                    .tab_bar_scroll_handle
-                    .content_size()
-                    .map(|content_size| content_size.size.width)
-                    .unwrap_or(px(0.));
                 let viewport_width = self.tab_bar_scroll_handle.viewport().size.width;
                 // We need to check both because offset returns delta values even when the scroll handle is not scrollable
                 let is_scrollable = content_width > viewport_width;