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

This reverts commit 82a7aca5a6.

Release Notes:

- N/A

crates/gpui/src/elements/div.rs

@@ -1559,20 +1559,32 @@ impl Interactivity {
     ) -> Point<Pixels> {
         if let Some(scroll_offset) = self.scroll_offset.as_ref() {
             let mut scroll_to_bottom = false;
-            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);
+            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 rem_size = window.rem_size();
-            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());
+            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());
             // 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();
@@ -1584,10 +1596,6 @@ 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()
@@ -2905,7 +2913,6 @@ 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>,
@@ -2968,11 +2975,6 @@ 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, size};
+use gpui::{Bounds, Point, size};
 use terminal::Terminal;
-use ui::{Pixels, ScrollableHandle, px};
+use ui::{ContentSize, Pixels, ScrollableHandle, px};
 
 #[derive(Debug)]
 struct ScrollHandleState {
@@ -46,9 +46,12 @@ impl TerminalScrollHandle {
 }
 
 impl ScrollableHandle for TerminalScrollHandle {
-    fn content_size(&self) -> Size<Pixels> {
+    fn content_size(&self) -> Option<ContentSize> {
         let state = self.state.borrow();
-        size(Pixels::ZERO, state.total_lines as f32 * state.line_height)
+        Some(ContentSize {
+            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, IsZero, LayoutId, ListState,
+    ElementId, Entity, EntityId, GlobalElementId, Hitbox, Hsla, LayoutId, ListState,
     MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels, Point, ScrollHandle, ScrollWheelEvent,
-    Size, Style, UniformListScrollHandle, Window, quad,
+    Size, Style, UniformListScrollHandle, Window, point, quad,
 };
 
 pub struct Scrollbar {
@@ -15,8 +15,11 @@ pub struct Scrollbar {
 }
 
 impl ScrollableHandle for UniformListScrollHandle {
-    fn content_size(&self) -> Size<Pixels> {
-        self.0.borrow().base_handle.content_size()
+    fn content_size(&self) -> Option<ContentSize> {
+        Some(ContentSize {
+            size: self.0.borrow().last_item_size.map(|size| size.contents)?,
+            scroll_adjustment: None,
+        })
     }
 
     fn set_offset(&self, point: Point<Pixels>) {
@@ -33,8 +36,11 @@ impl ScrollableHandle for UniformListScrollHandle {
 }
 
 impl ScrollableHandle for ListState {
-    fn content_size(&self) -> Size<Pixels> {
-        self.content_size_for_scrollbar()
+    fn content_size(&self) -> Option<ContentSize> {
+        Some(ContentSize {
+            size: self.content_size_for_scrollbar(),
+            scroll_adjustment: None,
+        })
     }
 
     fn set_offset(&self, point: Point<Pixels>) {
@@ -59,8 +65,27 @@ impl ScrollableHandle for ListState {
 }
 
 impl ScrollableHandle for ScrollHandle {
-    fn content_size(&self) -> Size<Pixels> {
-        self.padded_content_size()
+    fn content_size(&self) -> Option<ContentSize> {
+        let last_children_index = self.children_count().checked_sub(1)?;
+
+        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>) {
@@ -76,8 +101,14 @@ 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) -> Size<Pixels>;
+    fn content_size(&self) -> Option<ContentSize>;
     fn set_offset(&self, point: Point<Pixels>);
     fn offset(&self) -> Point<Pixels>;
     fn viewport(&self) -> Bounds<Pixels>;
@@ -118,26 +149,30 @@ impl ScrollbarState {
     }
 
     fn thumb_range(&self, axis: ScrollbarAxis) -> Option<Range<f32>> {
-        const MINIMUM_THUMB_SIZE: Pixels = px(25.);
-        let content_size = self.scroll_handle.content_size().along(axis);
-        let viewport_size = self.scroll_handle.viewport().size.along(axis);
-        if content_size.is_zero() || viewport_size.is_zero() || content_size < viewport_size {
+        const MINIMUM_THUMB_SIZE: f32 = 25.;
+        let ContentSize {
+            size: main_dimension_size,
+            scroll_adjustment,
+        } = 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;
@@ -212,38 +247,57 @@ 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 padded_bounds = Bounds::from_corners(
-                bounds
-                    .origin
-                    .apply_along(axis, |origin| origin + EXTRA_PADDING),
-                bounds
-                    .bottom_right()
-                    .apply_along(axis, |track_end| track_end - 3.0 * EXTRA_PADDING),
-            );
-
-            let thumb_offset = self.thumb.start * padded_bounds.size.along(axis);
-            let thumb_end = self.thumb.end * padded_bounds.size.along(axis);
-
-            let thumb_bounds = Bounds::new(
-                padded_bounds
-                    .origin
-                    .apply_along(axis, |origin| origin + thumb_offset),
-                padded_bounds
-                    .size
-                    .apply_along(axis, |_| thumb_end - thumb_offset)
-                    .apply_along(axis.invert(), |width| width / 1.5),
-            );
-
-            let corners = Corners::all(thumb_bounds.size.along(axis.invert()) / 2.0);
-
+            let mut thumb_bounds = if is_vertical {
+                let thumb_offset = self.thumb.start * padded_bounds.size.height;
+                let thumb_end = self.thumb.end * padded_bounds.size.height;
+                let thumb_upper_left = point(
+                    padded_bounds.origin.x,
+                    padded_bounds.origin.y + thumb_offset,
+                );
+                let thumb_lower_right = point(
+                    padded_bounds.origin.x + padded_bounds.size.width,
+                    padded_bounds.origin.y + thumb_end,
+                );
+                Bounds::from_corners(thumb_upper_left, thumb_lower_right)
+            } else {
+                let thumb_offset = self.thumb.start * padded_bounds.size.width;
+                let thumb_end = self.thumb.end * padded_bounds.size.width;
+                let thumb_upper_left = point(
+                    padded_bounds.origin.x + thumb_offset,
+                    padded_bounds.origin.y,
+                );
+                let thumb_lower_right = point(
+                    padded_bounds.origin.x + thumb_end,
+                    padded_bounds.origin.y + padded_bounds.size.height,
+                );
+                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,
@@ -254,39 +308,7 @@ impl Element for Scrollbar {
             ));
 
             let scroll = self.state.scroll_handle.clone();
-
-            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
-                };
+            let axis = self.kind;
 
             window.on_mouse_event({
                 let scroll = scroll.clone();
@@ -301,17 +323,39 @@ 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 {
-                        let click_offset = compute_click_offset(
-                            event.position,
-                            scroll.content_size(),
-                            ScrollbarMouseEvent::GutterClick,
-                        );
-                        scroll.set_offset(scroll.offset().apply_along(axis, |_| click_offset));
+                    } else if let Some(ContentSize {
+                        size: item_size, ..
+                    }) = scroll.content_size()
+                    {
+                        let click_offset = {
+                            let viewport_size = padded_bounds.size.along(axis);
+
+                            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, _| {
@@ -323,19 +367,44 @@ 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()) {
-                    let drag_offset = compute_click_offset(
-                        event.position,
-                        scroll.content_size(),
-                        ScrollbarMouseEvent::ThumbDrag(drag_state),
-                    );
-                    scroll.set_offset(scroll.offset().apply_along(axis, |_| drag_offset));
-                    window.refresh();
-                    if let Some(id) = state.parent_id {
-                        cx.notify(id);
+                    if let Some(ContentSize {
+                        size: item_size, ..
+                    }) = scroll.content_size()
+                    {
+                        let drag_offset = {
+                            let viewport_size = padded_bounds.size.along(axis);
+
+                            let thumb_size = thumb_bounds.size.along(axis);
+                            let thumb_start = (event.position.along(axis)
+                                - 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,7 +2671,11 @@ impl Pane {
                 }
             })
             .children(pinned_tabs.len().ne(&0).then(|| {
-                let content_width = self.tab_bar_scroll_handle.content_size().width;
+                let content_width = self
+                    .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;