scrollbar: Improve Scrollbar to get correct scroll size. (#985)

Continue #984

- Fixes there may Scrollbar size not match the content size. Close #979
- Improved Table scrollbar to only render at scrollable area when there
have fixed columns.

<img width="850" alt="image"
src="https://github.com/user-attachments/assets/84f54515-e61a-484f-b2b4-ea5abb98ddb8"
/>


## Break Changes

- The `Scrollbar` has been removed `entity_id` and `scroll_size`
argument from new method, it will get that value from `scroll_handle`.
- There still have a `scroll_size` optional method, if you want to give
your own.
  - Also changed the `scroll_state`, `scroll_handle` to receive a ref.

```diff
- Scrollbar::vertical(entity_id, scroll_state, scroll_handle, scroll_size);
+ Scrollbar::vertical(&scroll_state, &scroll_handle);
+ Scrollbar::vertical(&scroll_state, &scroll_handle).scroll_size(scroll_size);
```
This commit is contained in:
Jason Lee 2025-06-18 19:09:34 +08:00 committed by GitHub
parent 4c87c72f60
commit d7e04ecdd0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 100 additions and 201 deletions

View file

@ -198,8 +198,6 @@ impl Render for ScrollableStory {
_: &mut gpui::Window,
cx: &mut gpui::Context<Self>,
) -> impl gpui::IntoElement {
let view = cx.entity().clone();
v_flex()
.size_full()
.gap_4()
@ -273,13 +271,8 @@ impl Render for ScrollableStory {
.right_0()
.bottom_0()
.child(
Scrollbar::both(
view.entity_id(),
self.scroll_state.clone(),
self.scroll_handle.clone(),
self.scroll_size,
)
.axis(self.axis),
Scrollbar::both(&self.scroll_state, &self.scroll_handle)
.axis(self.axis),
)
}),
),
@ -298,7 +291,7 @@ impl Render for ScrollableStory {
.p_3()
.w(self.test_width)
.id("test-1")
.scrollable(cx.entity().entity_id(), Axis::Vertical)
.scrollable(Axis::Vertical)
.gap_1()
.child("Scrollable Example")
.children(self.items.iter().take(500).map(|item| {

View file

@ -1069,7 +1069,6 @@ impl EventEmitter<DismissEvent> for Tiles {}
impl Render for Tiles {
fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
let view = cx.entity().clone();
let view_id = view.entity_id();
let panels = self.sorted_panels();
let scroll_bounds =
self.panels
@ -1139,12 +1138,10 @@ impl Render for Tiles {
.left_0()
.right_0()
.bottom_0()
.child(Scrollbar::both(
view_id,
self.scroll_state.clone(),
self.scroll_handle.clone(),
scroll_size,
)),
.child(
Scrollbar::both(&self.scroll_state, &self.scroll_handle)
.scroll_size(scroll_size),
),
)
.size_full()
}

View file

@ -202,11 +202,10 @@ impl RenderOnce for Drawer {
)
.child(
// Body
div().flex_1().overflow_hidden().child(
v_flex()
.scrollable(window.current_view(), Axis::Vertical)
.child(self.content),
),
div()
.flex_1()
.overflow_hidden()
.child(v_flex().scrollable(Axis::Vertical).child(self.content)),
)
.when_some(self.footer, |this, footer| {
// Footer

View file

@ -298,10 +298,7 @@ impl RenderOnce for TextInput {
})
.refine_style(&self.style)
.when(state.is_multi_line(), |this| {
let entity_id = self.state.entity_id();
if state.last_layout.is_some() {
let scroll_size = state.scroll_size;
this.relative().child(
div()
.absolute()
@ -309,12 +306,10 @@ impl RenderOnce for TextInput {
.left_0()
.right(px(1.))
.bottom_0()
.child(Scrollbar::vertical(
entity_id,
state.scroll_state.clone(),
state.scroll_handle.clone(),
scroll_size,
)),
.child(
Scrollbar::vertical(&state.scroll_state, &state.scroll_handle)
.scroll_size(state.scroll_size),
),
)
} else {
this

View file

@ -287,15 +287,14 @@ where
self.selected_index
}
fn render_scrollbar(&self, _: &mut Window, cx: &mut Context<Self>) -> Option<impl IntoElement> {
fn render_scrollbar(&self, _: &mut Window, _: &mut Context<Self>) -> Option<impl IntoElement> {
if !self.scrollbar_visible {
return None;
}
Some(Scrollbar::uniform_scroll(
cx.entity().entity_id(),
self.scroll_state.clone(),
self.vertical_scroll_handle.clone(),
&self.scroll_state,
&self.vertical_scroll_handle,
))
}

View file

@ -943,12 +943,7 @@ impl Render for PopupMenu {
.left_0()
.right_0p5()
.bottom_0p5()
.child(Scrollbar::vertical(
cx.entity_id(),
self.scroll_state.clone(),
self.scroll_handle.clone(),
self.bounds.size,
)),
.child(Scrollbar::vertical(&self.scroll_state, &self.scroll_handle)),
)
})
}

View file

@ -475,7 +475,7 @@ impl RenderOnce for Modal {
v_flex()
.pl(padding_left)
.pr(padding_right)
.scrollable(window.current_view(), Axis::Vertical)
.scrollable(Axis::Vertical)
.child(self.content),
),
)

View file

@ -1,18 +1,15 @@
use std::{cell::Cell, rc::Rc};
use super::{Scrollbar, ScrollbarAxis, ScrollbarState};
use gpui::{
canvas, div, relative, AnyElement, App, Bounds, Div, Element, ElementId, EntityId,
GlobalElementId, InspectorElementId, InteractiveElement, Interactivity, IntoElement, LayoutId,
ParentElement, Pixels, Position, ScrollHandle, SharedString, Size, Stateful,
StatefulInteractiveElement, Style, StyleRefinement, Styled, Window,
div, relative, AnyElement, App, Bounds, Div, Element, ElementId, GlobalElementId,
InspectorElementId, InteractiveElement, Interactivity, IntoElement, LayoutId, ParentElement,
Pixels, Position, ScrollHandle, SharedString, Stateful, StatefulInteractiveElement, Style,
StyleRefinement, Styled, Window,
};
/// A scroll view is a container that allows the user to scroll through a large amount of content.
pub struct Scrollable<E> {
id: ElementId,
element: Option<E>,
view_id: EntityId,
axis: ScrollbarAxis,
/// This is a fake element to handle Styled, InteractiveElement, not used.
_element: Stateful<Div>,
@ -22,18 +19,15 @@ impl<E> Scrollable<E>
where
E: Element,
{
pub(crate) fn new(view_id: EntityId, element: E, axis: impl Into<ScrollbarAxis>) -> Self {
let id = ElementId::Name(SharedString::from(format!(
"scrollable-{}-{:?}",
view_id,
element.id(),
)));
pub(crate) fn new(axis: impl Into<ScrollbarAxis>, element: E) -> Self {
let id = ElementId::Name(SharedString::from(
format!("scrollable-{:?}", element.id(),),
));
Self {
element: Some(element),
_element: div().id("fake"),
id,
view_id,
axis: axis.into(),
}
}
@ -75,7 +69,6 @@ where
}
pub struct ScrollViewState {
scroll_size: Rc<Cell<Size<Pixels>>>,
state: ScrollbarState,
handle: ScrollHandle,
}
@ -84,7 +77,6 @@ impl Default for ScrollViewState {
fn default() -> Self {
Self {
handle: ScrollHandle::new(),
scroll_size: Rc::new(Cell::new(Size::default())),
state: ScrollbarState::default(),
}
}
@ -168,15 +160,10 @@ where
style.size.height = relative(1.0).into();
let axis = self.axis;
let view_id = self.view_id;
let scroll_id = self.id.clone();
let content = self.element.take().map(|c| c.into_any_element());
self.with_element_state(id.unwrap(), window, cx, |_, element_state, window, cx| {
let handle = element_state.handle.clone();
let state = element_state.state.clone();
let scroll_size = element_state.scroll_size.clone();
let mut element = div()
.relative()
.size_full()
@ -184,16 +171,11 @@ where
.child(
div()
.id(scroll_id)
.track_scroll(&handle)
.track_scroll(&element_state.handle)
.overflow_scroll()
.relative()
.size_full()
.child(div().children(content).child({
let scroll_size = element_state.scroll_size.clone();
canvas(move |b, _, _| scroll_size.set(b.size), |_, _, _, _| {})
.absolute()
.size_full()
})),
.child(div().children(content)),
)
.child(
div()
@ -203,8 +185,7 @@ where
.right_0()
.bottom_0()
.child(
Scrollbar::both(view_id, state, handle.clone(), scroll_size.get())
.axis(axis),
Scrollbar::both(&element_state.state, &element_state.handle).axis(axis),
),
)
.into_any_element();
@ -220,7 +201,7 @@ where
&mut self,
_: Option<&GlobalElementId>,
_: Option<&InspectorElementId>,
_: gpui::Bounds<Pixels>,
_: Bounds<Pixels>,
element: &mut Self::RequestLayoutState,
window: &mut Window,
cx: &mut App,

View file

@ -8,9 +8,9 @@ use std::{
use crate::{ActiveTheme, AxisExt};
use gpui::{
fill, point, px, relative, size, App, Axis, BorderStyle, Bounds, ContentMask, Corner,
CursorStyle, Edges, Element, EntityId, Hitbox, Hsla, IntoElement, MouseDownEvent,
MouseMoveEvent, MouseUpEvent, PaintQuad, Pixels, Point, Position, ScrollHandle,
ScrollWheelEvent, Style, UniformListScrollHandle, Window,
CursorStyle, Edges, Element, GlobalElementId, Hitbox, HitboxBehavior, Hsla, InspectorElementId,
IntoElement, LayoutId, MouseDownEvent, MouseMoveEvent, MouseUpEvent, PaintQuad, Pixels, Point,
Position, ScrollHandle, ScrollWheelEvent, Size, Style, UniformListScrollHandle, Window,
};
use serde::{Deserialize, Serialize};
@ -54,6 +54,8 @@ pub trait ScrollHandleOffsetable {
fn is_uniform_list(&self) -> bool {
false
}
/// The full size of the content, including padding.
fn content_size(&self) -> Size<Pixels>;
}
impl ScrollHandleOffsetable for ScrollHandle {
@ -64,6 +66,10 @@ impl ScrollHandleOffsetable for ScrollHandle {
fn set_offset(&self, offset: Point<Pixels>) {
self.set_offset(offset);
}
fn content_size(&self) -> Size<Pixels> {
self.padded_content_size()
}
}
impl ScrollHandleOffsetable for UniformListScrollHandle {
@ -78,6 +84,10 @@ impl ScrollHandleOffsetable for UniformListScrollHandle {
fn is_uniform_list(&self) -> bool {
true
}
fn content_size(&self) -> Size<Pixels> {
self.0.borrow().base_handle.padded_content_size()
}
}
#[derive(Debug, Clone)]
@ -250,11 +260,10 @@ impl ScrollbarAxis {
/// Scrollbar control for scroll-area or a uniform-list.
pub struct Scrollbar {
view_id: EntityId,
axis: ScrollbarAxis,
scroll_handle: Rc<Box<dyn ScrollHandleOffsetable>>,
scroll_size: gpui::Size<Pixels>,
state: ScrollbarState,
scroll_size: Option<Size<Pixels>>,
/// Maximum frames per second for scrolling by drag. Default is 120 FPS.
///
/// This is used to limit the update rate of the scrollbar when it is
@ -264,90 +273,57 @@ pub struct Scrollbar {
impl Scrollbar {
fn new(
view_id: EntityId,
state: ScrollbarState,
axis: impl Into<ScrollbarAxis>,
scroll_handle: impl ScrollHandleOffsetable + 'static,
scroll_size: gpui::Size<Pixels>,
state: &ScrollbarState,
scroll_handle: &(impl ScrollHandleOffsetable + Clone + 'static),
) -> Self {
Self {
view_id,
state,
state: state.clone(),
axis: axis.into(),
scroll_size,
scroll_handle: Rc::new(Box::new(scroll_handle)),
scroll_handle: Rc::new(Box::new(scroll_handle.clone())),
max_fps: 120,
scroll_size: None,
}
}
/// Create with vertical and horizontal scrollbar.
pub fn both(
view_id: EntityId,
state: ScrollbarState,
scroll_handle: impl ScrollHandleOffsetable + 'static,
scroll_size: gpui::Size<Pixels>,
state: &ScrollbarState,
scroll_handle: &(impl ScrollHandleOffsetable + Clone + 'static),
) -> Self {
Self::new(
view_id,
state,
ScrollbarAxis::Both,
scroll_handle,
scroll_size,
)
Self::new(ScrollbarAxis::Both, state, scroll_handle)
}
/// Create with horizontal scrollbar.
pub fn horizontal(
view_id: EntityId,
state: ScrollbarState,
scroll_handle: impl ScrollHandleOffsetable + 'static,
scroll_size: gpui::Size<Pixels>,
state: &ScrollbarState,
scroll_handle: &(impl ScrollHandleOffsetable + Clone + 'static),
) -> Self {
Self::new(
view_id,
state,
ScrollbarAxis::Horizontal,
scroll_handle,
scroll_size,
)
Self::new(ScrollbarAxis::Horizontal, state, scroll_handle)
}
/// Create with vertical scrollbar.
pub fn vertical(
view_id: EntityId,
state: ScrollbarState,
scroll_handle: impl ScrollHandleOffsetable + 'static,
scroll_size: gpui::Size<Pixels>,
state: &ScrollbarState,
scroll_handle: &(impl ScrollHandleOffsetable + Clone + 'static),
) -> Self {
Self::new(
view_id,
state,
ScrollbarAxis::Vertical,
scroll_handle,
scroll_size,
)
Self::new(ScrollbarAxis::Vertical, state, scroll_handle)
}
/// Create vertical scrollbar for uniform list.
pub fn uniform_scroll(
view_id: EntityId,
state: ScrollbarState,
scroll_handle: UniformListScrollHandle,
state: &ScrollbarState,
scroll_handle: &(impl ScrollHandleOffsetable + Clone + 'static),
) -> Self {
let scroll_size = scroll_handle
.0
.borrow()
.last_item_size
.map(|size| size.contents)
.unwrap_or_default();
Self::new(ScrollbarAxis::Vertical, state, scroll_handle)
}
Self::new(
view_id,
state,
ScrollbarAxis::Vertical,
scroll_handle,
scroll_size,
)
/// Set a special scroll size of the content area, default is None.
///
/// Default will sync the `content_size` from `scroll_handle`.
pub fn scroll_size(mut self, scroll_size: Size<Pixels>) -> Self {
self.scroll_size = Some(scroll_size);
self
}
/// Set scrollbar axis.
@ -448,7 +424,6 @@ pub struct AxisPrepaintState {
impl Element for Scrollbar {
type RequestLayoutState = ();
type PrepaintState = PrepaintState;
fn id(&self) -> Option<gpui::ElementId> {
@ -461,11 +436,11 @@ impl Element for Scrollbar {
fn request_layout(
&mut self,
_: Option<&gpui::GlobalElementId>,
_: Option<&gpui::InspectorElementId>,
_: Option<&GlobalElementId>,
_: Option<&InspectorElementId>,
window: &mut Window,
cx: &mut App,
) -> (gpui::LayoutId, Self::RequestLayoutState) {
) -> (LayoutId, Self::RequestLayoutState) {
let mut style = Style::default();
style.position = Position::Absolute;
style.flex_grow = 1.0;
@ -478,32 +453,34 @@ impl Element for Scrollbar {
fn prepaint(
&mut self,
_: Option<&gpui::GlobalElementId>,
_: Option<&gpui::InspectorElementId>,
_: Option<&GlobalElementId>,
_: Option<&InspectorElementId>,
bounds: Bounds<Pixels>,
_: &mut Self::RequestLayoutState,
window: &mut Window,
cx: &mut App,
) -> Self::PrepaintState {
let hitbox = window.with_content_mask(Some(ContentMask { bounds }), |window| {
window.insert_hitbox(bounds, gpui::HitboxBehavior::Normal)
window.insert_hitbox(bounds, HitboxBehavior::Normal)
});
let mut states = vec![];
let mut has_both = self.axis.is_both();
let scroll_size = self
.scroll_size
.unwrap_or(self.scroll_handle.content_size());
for axis in self.axis.all().into_iter() {
let is_vertical = axis.is_vertical();
let (scroll_area_size, container_size, scroll_position) = if is_vertical {
(
self.scroll_size.height,
scroll_size.height,
hitbox.size.height,
self.scroll_handle.offset().y,
)
} else {
(
self.scroll_size.width,
scroll_size.width,
hitbox.size.width,
self.scroll_handle.offset().x,
)
@ -660,14 +637,15 @@ impl Element for Scrollbar {
fn paint(
&mut self,
_: Option<&gpui::GlobalElementId>,
_: Option<&gpui::InspectorElementId>,
_: Option<&GlobalElementId>,
_: Option<&InspectorElementId>,
_: Bounds<Pixels>,
_: &mut Self::RequestLayoutState,
prepaint: &mut Self::PrepaintState,
window: &mut Window,
cx: &mut App,
) {
let view_id = window.current_view();
let hitbox_bounds = prepaint.hitbox.bounds;
let is_visible =
self.state.get().is_scrollbar_visible() || cx.theme().scrollbar_show.is_always();
@ -733,7 +711,6 @@ impl Element for Scrollbar {
window.on_mouse_event({
let state = self.state.clone();
let view_id = self.view_id;
let scroll_handle = self.scroll_handle.clone();
move |event: &ScrollWheelEvent, phase, _, cx| {
@ -754,7 +731,6 @@ impl Element for Scrollbar {
if is_hover_to_show || is_visible {
window.on_mouse_event({
let state = self.state.clone();
let view_id = self.view_id;
let scroll_handle = self.scroll_handle.clone();
move |event: &MouseDownEvent, phase, _, cx| {
@ -803,7 +779,6 @@ impl Element for Scrollbar {
window.on_mouse_event({
let scroll_handle = self.scroll_handle.clone();
let state = self.state.clone();
let view_id = self.view_id;
let max_fps_duration = Duration::from_millis((1000 / self.max_fps) as u64);
move |event: &MouseMoveEvent, _, _, cx| {
@ -888,7 +863,6 @@ impl Element for Scrollbar {
});
window.on_mouse_event({
let view_id = self.view_id;
let state = self.state.clone();
move |_event: &MouseUpEvent, phase, _, cx| {

View file

@ -185,8 +185,7 @@ impl RenderOnce for SidebarToggleButton {
}
impl<E: Collapsible + IntoElement> RenderOnce for Sidebar<E> {
fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement {
let view_id = window.current_view();
fn render(mut self, _: &mut Window, cx: &mut App) -> impl IntoElement {
v_flex()
.id("sidebar")
.w(self.width)
@ -215,7 +214,7 @@ impl<E: Collapsible + IntoElement> RenderOnce for Sidebar<E> {
.map(|(ix, c)| div().id(ix).child(c.collapsed(self.collapsed))),
)
.gap_2()
.scrollable(view_id, ScrollbarAxis::Vertical),
.scrollable(ScrollbarAxis::Vertical),
),
)
.when_some(self.footer.take(), |this, footer| {

View file

@ -5,8 +5,8 @@ use crate::{
ActiveTheme,
};
use gpui::{
div, px, App, Axis, DefiniteLength, Div, Edges, Element, ElementId, EntityId, FocusHandle,
Pixels, Refineable, StyleRefinement, Styled, Window,
div, px, App, Axis, DefiniteLength, Div, Edges, Element, ElementId, FocusHandle, Pixels,
Refineable, StyleRefinement, Styled, Window,
};
use serde::{Deserialize, Serialize};
@ -144,11 +144,11 @@ pub trait StyledExt: Styled + Sized {
///
/// Current this is only have a vertical scrollbar.
#[inline]
fn scrollable(self, view_id: EntityId, axis: impl Into<ScrollbarAxis>) -> Scrollable<Self>
fn scrollable(self, axis: impl Into<ScrollbarAxis>) -> Scrollable<Self>
where
Self: Element,
{
Scrollable::new(view_id, self, axis)
Scrollable::new(axis, self)
}
font_weight!(font_thin, THIN);

View file

@ -140,8 +140,6 @@ pub struct Table<D: TableDelegate> {
bounds: Bounds<Pixels>,
/// The bounds of the fixed head cols.
fixed_head_cols_bounds: Bounds<Pixels>,
/// The bounds of the table head content.
head_content_bounds: Bounds<Pixels>,
col_groups: Vec<ColGroup>,
fixed_cols: FixedCols,
@ -400,7 +398,6 @@ where
resizing_col: None,
bounds: Bounds::default(),
fixed_head_cols_bounds: Bounds::default(),
head_content_bounds: Bounds::default(),
stripe: false,
border: true,
size: Size::default(),
@ -806,22 +803,6 @@ where
}
}
/// Returns the size of the content area.
fn head_content_bounds(&self) -> gpui::Bounds<Pixels> {
let has_fixed_cols = self.fixed_head_cols_bounds.size.width > px(0.0);
Bounds {
origin: if has_fixed_cols {
self.fixed_head_cols_bounds.origin
} else {
self.head_content_bounds.origin
},
size: gpui::size(
self.fixed_head_cols_bounds.size.width + self.head_content_bounds.size.width,
self.head_content_bounds.size.height,
),
}
}
#[inline]
fn render_cell(&self, col_ix: usize, _window: &mut Window, _cx: &mut Context<Self>) -> Div {
let Some(col_group) = self.col_groups.get(col_ix) else {
@ -880,14 +861,7 @@ where
.on_scroll_wheel(cx.listener(|_, _: &ScrollWheelEvent, _, cx| {
cx.notify();
}))
.child(
Scrollbar::uniform_scroll(
cx.entity().entity_id(),
state,
self.vertical_scroll_handle.clone(),
)
.max_fps(60),
),
.child(Scrollbar::uniform_scroll(&state, &self.vertical_scroll_handle).max_fps(60)),
)
}
@ -901,7 +875,7 @@ where
div()
.occlude()
.absolute()
.left_0()
.left(self.fixed_head_cols_bounds.size.width)
.right_0()
.bottom_0()
.h(scroll::WIDTH)
@ -909,10 +883,8 @@ where
cx.notify();
}))
.child(Scrollbar::horizontal(
cx.entity().entity_id(),
state,
self.horizontal_scroll_handle.clone(),
self.head_content_bounds().size,
&state,
&self.horizontal_scroll_handle,
))
}
@ -1136,6 +1108,11 @@ where
let view = cx.entity().clone();
let horizontal_scroll_handle = self.horizontal_scroll_handle.clone();
// Reset fixed head columns bounds, if no fixed columns are present
if left_cols_count == 0 {
self.fixed_head_cols_bounds = Bounds::default();
}
h_flex()
.w_full()
.h(self.size.table_row_height())
@ -1203,17 +1180,7 @@ where
self.render_th(left_cols_count + col_ix, window, cx)
}),
)
.child(self.delegate.render_last_empty_col(window, cx))
.child(
canvas(
move |bounds, _, cx| {
view.update(cx, |r, _| r.head_content_bounds = bounds)
},
|_, _, _, _| {},
)
.absolute()
.size_full(),
),
.child(self.delegate.render_last_empty_col(window, cx)),
),
)
}