oak-team/oak-gpui
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Refactor the scrollbar component (#36105)

Browse Source 
Closes https://github.com/zed-industries/zed/issues/37621
Improves https://github.com/zed-industries/zed/issues/24623

Adding scrollbars withing Zed's UI currently is rather cumbersome, as it
requires the copying of a lot of code in order for these to work. Wiring
up settings for scrollbar visibilty always has to be done at the call
site and the state has to be saved and maintained by the caller as well.
Similarly, reserving space has to also be handled by the caller.

This PR changes the way scrollbars work in Zed fundamentally by making
use of the new `use_keyed_state` APIs: Instead of saving the state at
the call site, the window now keeps track of the state corresponding to
scrollbars. This enables us to add scrollbars with e.g. one simple call
on divs:
```rust
div()
    .vertical_scrollbar(window, cx)
```
will add a scrollbar to the corresponding container. There are some more
improvements regarding tracking of scrollbar visibility settings (which
is now handled by a trait for each setting that supports this) as well
as reserving space.
Additionally, all needed stuff for layouting, catching events and
reserving space is also now managed by the scrollbar component instead.
This drastically reduces the amount of event listeners and makes
layouting of two scrollbars easier.

Furthermore, this paves the way for more improvements to scrollbars,
such as graceful auto-hide. Only downsight here is that we lose some
customizability in a few areas. However, once this lands, we gain the
ability to quickly follow these up without breaking stuff elsewhere.

This also already fixes a few bugs:
- Scrollbars no longer flicker on first render. 
- Auto-hide now properly works for all scrollbars.
- If the content size changes, the scrollbar is updated on the same
frame. Both of these happened because we were computing the scrollbar
sizes too early, causing us to use the sizes from the previous frame or
unitialized sizes.
- The project panel no longer jumps if scrolled all the way to the
bottom and the scrollbar actually auto-hides.

Still TODO:
- [x] Fix scrolling in the debugger memory view
- [x] Clean up some more in the scrollbar component and reduce clones
there
- [x] Ensure we don't over-notify the entity the scrollbar is rendered
within
- [x] Make sure auto-hide properly works for all cases
- [x] Check whether we want to implement the scrollbar trait for
`UniformList`s as well
    - ~~ [ ] Use for uniformlist where possible~~ Postponed
- [x] Improve layout for cases where we render both scrollbars.

Release Notes:

- N/A
This commit is contained in:
Finn Evers
2025-09-13 00:43:16 +02:00
committed by GitHub
parent 53c5db4495
commit ded6467604
35 changed files with 1566 additions and 2386 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/keymap_editor/src/ui_components/table.rs
+33 -438
View File
@@ -1,20 +1,20 @@
use std::{ops::Range, rc::Rc, time::Duration};
use std::{ops::Range, rc::Rc};
use editor::{EditorSettings, ShowScrollbar, scroll::ScrollbarAutoHide};
use editor::EditorSettings;
use gpui::{
AbsoluteLength, AppContext, Axis, Context, DefiniteLength, DragMoveEvent, Entity, EntityId,
FocusHandle, Length, ListHorizontalSizingBehavior, ListSizingBehavior, MouseButton, Point,
Stateful, Task, UniformListScrollHandle, WeakEntity, transparent_black, uniform_list,
AbsoluteLength, AppContext, Context, DefiniteLength, DragMoveEvent, Entity, EntityId,
FocusHandle, Length, ListHorizontalSizingBehavior, ListSizingBehavior, Point, Stateful,
UniformListScrollHandle, WeakEntity, transparent_black, uniform_list,
};
use itertools::intersperse_with;
use settings::Settings as _;
use ui::{
ActiveTheme as _, AnyElement, App, Button, ButtonCommon as _, ButtonStyle, Color, Component,
ComponentScope, Div, ElementId, FixedWidth as _, FluentBuilder as _, Indicator,
InteractiveElement, IntoElement, ParentElement, Pixels, RegisterComponent, RenderOnce,
Scrollbar, ScrollbarState, SharedString, StatefulInteractiveElement, Styled, StyledExt as _,
StyledTypography, Window, div, example_group_with_title, h_flex, px, single_example, v_flex,
ScrollableHandle, Scrollbars, SharedString, StatefulInteractiveElement, Styled, StyledExt as _,
StyledTypography, Window, WithScrollbar, div, example_group_with_title, h_flex, px,
single_example, v_flex,
};
const RESIZE_COLUMN_WIDTH: f32 = 8.0;
@@ -56,136 +56,22 @@ impl<const COLS: usize> TableContents<COLS> {
pub struct TableInteractionState {
pub focus_handle: FocusHandle,
pub scroll_handle: UniformListScrollHandle,
pub horizontal_scrollbar: ScrollbarProperties,
pub vertical_scrollbar: ScrollbarProperties,
}
impl TableInteractionState {
pub fn new(window: &mut Window, cx: &mut App) -> Entity<Self> {
cx.new(|cx| {
let focus_handle = cx.focus_handle();
cx.on_focus_out(&focus_handle, window, |this: &mut Self, _, window, cx| {
this.hide_scrollbars(window, cx);
})
.detach();
let scroll_handle = UniformListScrollHandle::new();
let vertical_scrollbar = ScrollbarProperties {
axis: Axis::Vertical,
state: ScrollbarState::new(scroll_handle.clone()).parent_entity(&cx.entity()),
show_scrollbar: false,
show_track: false,
auto_hide: false,
hide_task: None,
};
let horizontal_scrollbar = ScrollbarProperties {
axis: Axis::Horizontal,
state: ScrollbarState::new(scroll_handle.clone()).parent_entity(&cx.entity()),
show_scrollbar: false,
show_track: false,
auto_hide: false,
hide_task: None,
};
let mut this = Self {
focus_handle,
scroll_handle,
horizontal_scrollbar,
vertical_scrollbar,
};
this.update_scrollbar_visibility(cx);
this
pub fn new(cx: &mut App) -> Entity<Self> {
cx.new(|cx| Self {
focus_handle: cx.focus_handle(),
scroll_handle: UniformListScrollHandle::new(),
})
}
pub fn get_scrollbar_offset(&self, axis: Axis) -> Point<Pixels> {
match axis {
Axis::Vertical => self.vertical_scrollbar.state.scroll_handle().offset(),
Axis::Horizontal => self.horizontal_scrollbar.state.scroll_handle().offset(),
}
pub fn scroll_offset(&self) -> Point<Pixels> {
self.scroll_handle.offset()
}
pub fn set_scrollbar_offset(&self, axis: Axis, offset: Point<Pixels>) {
match axis {
Axis::Vertical => self
.vertical_scrollbar
.state
.scroll_handle()
.set_offset(offset),
Axis::Horizontal => self
.horizontal_scrollbar
.state
.scroll_handle()
.set_offset(offset),
}
}
fn update_scrollbar_visibility(&mut self, cx: &mut Context<Self>) {
let show_setting = EditorSettings::get_global(cx).scrollbar.show;
let scroll_handle = self.scroll_handle.0.borrow();
let autohide = |show: ShowScrollbar, cx: &mut Context<Self>| match show {
ShowScrollbar::Auto => true,
ShowScrollbar::System => cx
.try_global::<ScrollbarAutoHide>()
.map_or_else(|| cx.should_auto_hide_scrollbars(), |autohide| autohide.0),
ShowScrollbar::Always => false,
ShowScrollbar::Never => false,
};
let longest_item_width = scroll_handle.last_item_size.and_then(|size| {
(size.contents.width > size.item.width).then_some(size.contents.width)
});
// is there an item long enough that we should show a horizontal scrollbar?
let item_wider_than_container = if let Some(longest_item_width) = longest_item_width {
longest_item_width > px(scroll_handle.base_handle.bounds().size.width.0)
} else {
true
};
let show_scrollbar = match show_setting {
ShowScrollbar::Auto | ShowScrollbar::System | ShowScrollbar::Always => true,
ShowScrollbar::Never => false,
};
let show_vertical = show_scrollbar;
let show_horizontal = item_wider_than_container && show_scrollbar;
let show_horizontal_track =
show_horizontal && matches!(show_setting, ShowScrollbar::Always);
// TODO: we probably should hide the scroll track when the list doesn't need to scroll
let show_vertical_track = show_vertical && matches!(show_setting, ShowScrollbar::Always);
self.vertical_scrollbar = ScrollbarProperties {
axis: self.vertical_scrollbar.axis,
state: self.vertical_scrollbar.state.clone(),
show_scrollbar: show_vertical,
show_track: show_vertical_track,
auto_hide: autohide(show_setting, cx),
hide_task: None,
};
self.horizontal_scrollbar = ScrollbarProperties {
axis: self.horizontal_scrollbar.axis,
state: self.horizontal_scrollbar.state.clone(),
show_scrollbar: show_horizontal,
show_track: show_horizontal_track,
auto_hide: autohide(show_setting, cx),
hide_task: None,
};
cx.notify();
}
fn hide_scrollbars(&mut self, window: &mut Window, cx: &mut Context<Self>) {
self.horizontal_scrollbar.hide(window, cx);
self.vertical_scrollbar.hide(window, cx);
pub fn set_scroll_offset(&self, offset: Point<Pixels>) {
self.scroll_handle.set_offset(offset);
}
pub fn listener<E: ?Sized>(
@@ -280,183 +166,6 @@ impl TableInteractionState {
.children(dividers)
.into_any_element()
}
fn render_vertical_scrollbar_track(
this: &Entity<Self>,
parent: Div,
scroll_track_size: Pixels,
cx: &mut App,
) -> Div {
if !this.read(cx).vertical_scrollbar.show_track {
return parent;
}
let child = v_flex()
.h_full()
.flex_none()
.w(scroll_track_size)
.bg(cx.theme().colors().background)
.child(
div()
.size_full()
.flex_1()
.border_l_1()
.border_color(cx.theme().colors().border),
);
parent.child(child)
}
fn render_vertical_scrollbar(this: &Entity<Self>, parent: Div, cx: &mut App) -> Div {
if !this.read(cx).vertical_scrollbar.show_scrollbar {
return parent;
}
let child = div()
.id(("table-vertical-scrollbar", this.entity_id()))
.occlude()
.flex_none()
.h_full()
.cursor_default()
.absolute()
.right_0()
.top_0()
.bottom_0()
.w(px(12.))
.on_mouse_move(Self::listener(this, |_, _, _, cx| {
cx.notify();
cx.stop_propagation()
}))
.on_hover(|_, _, cx| {
cx.stop_propagation();
})
.on_mouse_up(
MouseButton::Left,
Self::listener(this, |this, _, window, cx| {
if !this.vertical_scrollbar.state.is_dragging()
&& !this.focus_handle.contains_focused(window, cx)
{
this.vertical_scrollbar.hide(window, cx);
cx.notify();
}
cx.stop_propagation();
}),
)
.on_any_mouse_down(|_, _, cx| {
cx.stop_propagation();
})
.on_scroll_wheel(Self::listener(this, |_, _, _, cx| {
cx.notify();
}))
.children(Scrollbar::vertical(
this.read(cx).vertical_scrollbar.state.clone(),
));
parent.child(child)
}
/// Renders the horizontal scrollbar.
///
/// The right offset is used to determine how far to the right the
/// scrollbar should extend to, useful for ensuring it doesn't collide
/// with the vertical scrollbar when visible.
fn render_horizontal_scrollbar(
this: &Entity<Self>,
parent: Div,
right_offset: Pixels,
cx: &mut App,
) -> Div {
if !this.read(cx).horizontal_scrollbar.show_scrollbar {
return parent;
}
let child = div()
.id(("table-horizontal-scrollbar", this.entity_id()))
.occlude()
.flex_none()
.w_full()
.cursor_default()
.absolute()
.bottom_neg_px()
.left_0()
.right_0()
.pr(right_offset)
.on_mouse_move(Self::listener(this, |_, _, _, cx| {
cx.notify();
cx.stop_propagation()
}))
.on_hover(|_, _, cx| {
cx.stop_propagation();
})
.on_any_mouse_down(|_, _, cx| {
cx.stop_propagation();
})
.on_mouse_up(
MouseButton::Left,
Self::listener(this, |this, _, window, cx| {
if !this.horizontal_scrollbar.state.is_dragging()
&& !this.focus_handle.contains_focused(window, cx)
{
this.horizontal_scrollbar.hide(window, cx);
cx.notify();
}
cx.stop_propagation();
}),
)
.on_scroll_wheel(Self::listener(this, |_, _, _, cx| {
cx.notify();
}))
.children(Scrollbar::horizontal(
// percentage as f32..end_offset as f32,
this.read(cx).horizontal_scrollbar.state.clone(),
));
parent.child(child)
}
fn render_horizontal_scrollbar_track(
this: &Entity<Self>,
parent: Div,
scroll_track_size: Pixels,
cx: &mut App,
) -> Div {
if !this.read(cx).horizontal_scrollbar.show_track {
return parent;
}
let child = h_flex()
.w_full()
.h(scroll_track_size)
.flex_none()
.relative()
.child(
div()
.w_full()
.flex_1()
// for some reason the horizontal scrollbar is 1px
// taller than the vertical scrollbar??
.h(scroll_track_size - px(1.))
.bg(cx.theme().colors().background)
.border_t_1()
.border_color(cx.theme().colors().border),
)
.when(this.read(cx).vertical_scrollbar.show_track, |parent| {
parent
.child(
div()
.flex_none()
// -1px prevents a missing pixel between the two container borders
.w(scroll_track_size - px(1.))
.h_full(),
)
.child(
// HACK: Fill the missing 1px 🥲
div()
.absolute()
.right(scroll_track_size - px(1.))
.bottom(scroll_track_size - px(1.))
.size_px()
.bg(cx.theme().colors().border),
)
});
parent.child(child)
}
}
#[derive(Debug, Copy, Clone, PartialEq)]
@@ -1054,17 +763,6 @@ impl<const COLS: usize> RenderOnce for Table<COLS> {
.and_then(|widths| Some((widths.current.as_ref()?, widths.resizable, widths.initial)))
.map(|(curr, resize_behavior, initial)| (curr.downgrade(), resize_behavior, initial));
let scroll_track_size = px(16.);
let h_scroll_offset = if interaction_state
.as_ref()
.is_some_and(|state| state.read(cx).vertical_scrollbar.show_scrollbar)
{
// magic number
px(3.)
} else {
px(0.)
};
let width = self.width;
let no_rows_rendered = self.rows.is_empty();
@@ -1115,8 +813,8 @@ impl<const COLS: usize> RenderOnce for Table<COLS> {
})
}
})
.child(
div()
.child({
let content = div()
.flex_grow()
.w_full()
.relative()
@@ -1187,25 +885,21 @@ impl<const COLS: usize> RenderOnce for Table<COLS> {
)
}))
},
)
.when_some(interaction_state.as_ref(), |this, interaction_state| {
this.map(|this| {
TableInteractionState::render_vertical_scrollbar_track(
interaction_state,
this,
scroll_track_size,
cx,
)
})
.map(|this| {
TableInteractionState::render_vertical_scrollbar(
interaction_state,
this,
cx,
)
})
}),
)
);
if let Some(state) = interaction_state.as_ref() {
content
.custom_scrollbars(
Scrollbars::for_settings::<EditorSettings>()
.tracked_scroll_handle(state.read(cx).scroll_handle.clone()),
window,
cx,
)
.into_any_element()
} else {
content.into_any_element()
}
})
.when_some(
no_rows_rendered
.then_some(self.empty_table_callback)
@@ -1220,52 +914,12 @@ impl<const COLS: usize> RenderOnce for Table<COLS> {
.child(callback(window, cx)),
)
},
)
.when_some(
width.and(interaction_state.as_ref()),
|this, interaction_state| {
this.map(|this| {
TableInteractionState::render_horizontal_scrollbar_track(
interaction_state,
this,
scroll_track_size,
cx,
)
})
.map(|this| {
TableInteractionState::render_horizontal_scrollbar(
interaction_state,
this,
h_scroll_offset,
cx,
)
})
},
);
if let Some(interaction_state) = interaction_state.as_ref() {
table
.track_focus(&interaction_state.read(cx).focus_handle)
.id(("table", interaction_state.entity_id()))
.on_hover({
let interaction_state = interaction_state.downgrade();
move |hovered, window, cx| {
interaction_state
.update(cx, |interaction_state, cx| {
if *hovered {
interaction_state.horizontal_scrollbar.show(cx);
interaction_state.vertical_scrollbar.show(cx);
cx.notify();
} else if !interaction_state
.focus_handle
.contains_focused(window, cx)
{
interaction_state.hide_scrollbars(window, cx);
}
})
.ok();
}
})
.into_any_element()
} else {
table.into_any_element()
@@ -1273,65 +927,6 @@ impl<const COLS: usize> RenderOnce for Table<COLS> {
}
}
// computed state related to how to render scrollbars
// one per axis
// on render we just read this off the keymap editor
// we update it when
// - settings change
// - on focus in, on focus out, on hover, etc.
#[derive(Debug)]
pub struct ScrollbarProperties {
axis: Axis,
show_scrollbar: bool,
show_track: bool,
auto_hide: bool,
hide_task: Option<Task<()>>,
state: ScrollbarState,
}
impl ScrollbarProperties {
// Shows the scrollbar and cancels any pending hide task
fn show(&mut self, cx: &mut Context<TableInteractionState>) {
if !self.auto_hide {
return;
}
self.show_scrollbar = true;
self.hide_task.take();
cx.notify();
}
fn hide(&mut self, window: &mut Window, cx: &mut Context<TableInteractionState>) {
const SCROLLBAR_SHOW_INTERVAL: Duration = Duration::from_secs(1);
if !self.auto_hide {
return;
}
let axis = self.axis;
self.hide_task = Some(cx.spawn_in(window, async move |keymap_editor, cx| {
cx.background_executor()
.timer(SCROLLBAR_SHOW_INTERVAL)
.await;
if let Some(keymap_editor) = keymap_editor.upgrade() {
keymap_editor
.update(cx, |keymap_editor, cx| {
match axis {
Axis::Vertical => {
keymap_editor.vertical_scrollbar.show_scrollbar = false
}
Axis::Horizontal => {
keymap_editor.horizontal_scrollbar.show_scrollbar = false
}
}
cx.notify();
})
.ok();
}
}));
}
}
impl Component for Table<3> {
fn scope() -> ComponentScope {
ComponentScope::Layout