From 4a582504d4aab388c6fa49ab74056e1a7eccfe94 Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Mon, 15 Sep 2025 11:53:33 +0200 Subject: [PATCH] ui: Follow-up improvements to the scrollbar component (#38178) This PR lands some more improvements to the reworked scrollbars. Namely, we will now explicitly paint a background in cases where a track is requested for the specific scrollbar, which prevents a flicker, and also reserve space only if space actually needs to be reserved. The latter was a regression introduced by the recent changes. Release Notes: - N/A --- crates/git_ui/src/git_panel.rs | 5 +- crates/outline_panel/src/outline_panel.rs | 5 +- crates/project_panel/src/project_panel.rs | 5 +- crates/terminal_view/src/terminal_view.rs | 5 +- crates/ui/src/components/scrollbar.rs | 84 +++++++++++++++++------ 5 files changed, 80 insertions(+), 24 deletions(-) diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 3063206723021d00aa21e0f8163b4fe3b941fcb7..e8306efc0fe6f6c554f16d3b873a0fd3c659c0b7 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -3748,7 +3748,10 @@ impl GitPanel { .custom_scrollbars( Scrollbars::for_settings::() .tracked_scroll_handle(self.scroll_handle.clone()) - .with_track_along(ScrollAxes::Horizontal), + .with_track_along( + ScrollAxes::Horizontal, + cx.theme().colors().panel_background, + ), window, cx, ), diff --git a/crates/outline_panel/src/outline_panel.rs b/crates/outline_panel/src/outline_panel.rs index 8f501b2f970019c24c36e65fa94099b80454dfe2..5b1c5b313d29751e53e08bd479eeb72d15527373 100644 --- a/crates/outline_panel/src/outline_panel.rs +++ b/crates/outline_panel/src/outline_panel.rs @@ -4692,7 +4692,10 @@ impl OutlinePanel { .custom_scrollbars( Scrollbars::for_settings::() .tracked_scroll_handle(self.scroll_handle.clone()) - .with_track_along(ScrollAxes::Horizontal) + .with_track_along( + ScrollAxes::Horizontal, + cx.theme().colors().panel_background, + ) .tracked_entity(cx.entity_id()), window, cx, diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index b53dfe6fdf8bdf81761299c075eee302a06a8956..aed3b44515554299b3d50fbcf5e2b58123495908 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -5601,7 +5601,10 @@ impl Render for ProjectPanel { .custom_scrollbars( Scrollbars::for_settings::() .tracked_scroll_handle(self.scroll_handle.clone()) - .with_track_along(ScrollAxes::Horizontal) + .with_track_along( + ScrollAxes::Horizontal, + cx.theme().colors().panel_background, + ) .notify_content(), window, cx, diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 5fdefc6a66224587bc746d21ecddb1a66a2bbe4f..133bde6175c02e6f8845d83c765f7820b77e37f7 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -1119,7 +1119,10 @@ impl Render for TerminalView { div.custom_scrollbars( Scrollbars::for_settings::() .show_along(ScrollAxes::Vertical) - .with_track_along(ScrollAxes::Vertical) + .with_track_along( + ScrollAxes::Vertical, + cx.theme().colors().editor_background, + ) .tracked_scroll_handle(self.scroll_handle.clone()), window, cx, diff --git a/crates/ui/src/components/scrollbar.rs b/crates/ui/src/components/scrollbar.rs index f3cc37d2f55356f05af3f1644dc4292a6add2660..6885a11c51647aafbf1e3136060d52105e3664b7 100644 --- a/crates/ui/src/components/scrollbar.rs +++ b/crates/ui/src/components/scrollbar.rs @@ -30,7 +30,7 @@ pub mod scrollbars { /// When to show the scrollbar in the editor. /// /// Default: auto - #[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] + #[derive(Copy, Clone, Debug, Default, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum ShowScrollbar { /// Show the scrollbar if there's important information or @@ -39,6 +39,7 @@ pub mod scrollbars { /// Match the system's configured behavior. System, /// Always show the scrollbar. + #[default] Always, /// Never show the scrollbar. Never, @@ -302,7 +303,7 @@ enum ReservedSpace { #[default] None, Thumb, - Track, + Track(Hsla), } impl ReservedSpace { @@ -311,7 +312,14 @@ impl ReservedSpace { } fn needs_scroll_track(&self) -> bool { - *self == ReservedSpace::Track + matches!(self, ReservedSpace::Track(_)) + } + + fn track_color(&self) -> Option { + match self { + ReservedSpace::Track(color) => Some(*color), + _ => None, + } } } @@ -333,20 +341,25 @@ impl ScrollbarWidth { } } +#[derive(Clone)] +enum Handle { + Tracked(T), + Untracked(fn() -> T), +} + #[derive(Clone)] pub struct Scrollbars { id: Option, get_visibility: fn(&App) -> ShowScrollbar, tracked_entity: Option>, - scrollable_handle: T, - handle_was_added: bool, + scrollable_handle: Handle, visibility: Point, scrollbar_width: ScrollbarWidth, } impl Scrollbars { pub fn new(show_along: ScrollAxes) -> Self { - Self::new_with_setting(show_along, |_| ShowScrollbar::Always) + Self::new_with_setting(show_along, |_| ShowScrollbar::default()) } pub fn for_settings() -> Scrollbars { @@ -359,8 +372,7 @@ impl Scrollbars { Self { id: None, get_visibility, - handle_was_added: false, - scrollable_handle: ScrollHandle::new(), + scrollable_handle: Handle::Untracked(ScrollHandle::new), tracked_entity: None, visibility: show_along.apply_to(Default::default(), ReservedSpace::Thumb), scrollbar_width: ScrollbarWidth::Normal, @@ -407,8 +419,7 @@ impl Scrollbars { } = self; Scrollbars { - handle_was_added: true, - scrollable_handle: tracked_scroll_handle, + scrollable_handle: Handle::Tracked(tracked_scroll_handle), id, tracked_entity: tracked_entity_id, visibility, @@ -422,8 +433,8 @@ impl Scrollbars { self } - pub fn with_track_along(mut self, along: ScrollAxes) -> Self { - self.visibility = along.apply_to(self.visibility, ReservedSpace::Track); + pub fn with_track_along(mut self, along: ScrollAxes, background_color: Hsla) -> Self { + self.visibility = along.apply_to(self.visibility, ReservedSpace::Track(background_color)); self } @@ -499,12 +510,17 @@ impl ScrollbarState { cx.observe_global_in::(window, Self::settings_changed) .detach(); + let (manually_added, scroll_handle) = match config.scrollable_handle { + Handle::Tracked(handle) => (true, handle), + Handle::Untracked(func) => (false, func()), + }; + let show_setting = (config.get_visibility)(cx); ScrollbarState { thumb_state: Default::default(), notify_id: config.tracked_entity.map(|id| id.unwrap_or(parent_id)), - manually_added: config.handle_was_added, - scroll_handle: config.scrollable_handle, + manually_added, + scroll_handle, width: config.scrollbar_width, visibility: config.visibility, show_setting, @@ -531,8 +547,10 @@ impl ScrollbarState { .await; scrollbar_state .update(cx, |state, cx| { - state.set_visibility(VisibilityState::Hidden, cx); - state._auto_hide_task.take() + if state.thumb_state == ThumbState::Inactive { + state.set_visibility(VisibilityState::Hidden, cx); + } + state._auto_hide_task.take(); }) .log_err(); }) @@ -578,8 +596,15 @@ impl ScrollbarState { } fn space_to_reserve_for(&self, axis: ScrollbarAxis) -> Option { - (self.show_state.is_disabled().not() && self.visibility.along(axis).needs_scroll_track()) - .then(|| self.space_to_reserve()) + (self.show_state.is_disabled().not() + && self.visibility.along(axis).needs_scroll_track() + && self + .scroll_handle() + .max_offset() + .along(axis) + .is_zero() + .not()) + .then(|| self.space_to_reserve()) } fn space_to_reserve(&self) -> Pixels { @@ -643,7 +668,8 @@ impl ScrollbarState { if state == ThumbState::Inactive { self.schedule_auto_hide(window, cx); } else { - self.show_scrollbars(window, cx); + self.set_visibility(VisibilityState::Visible, cx); + self._auto_hide_task.take(); } self.thumb_state = state; cx.notify(); @@ -848,6 +874,7 @@ struct ScrollbarLayout { track_bounds: Bounds, cursor_hitbox: Hitbox, reserved_space: ReservedSpace, + track_background: Option<(Bounds, Hsla)>, axis: ScrollbarAxis, } @@ -1035,6 +1062,9 @@ impl Element for ScrollbarElement { }, HitboxBehavior::BlockMouseExceptScroll, ), + track_background: reserved_space + .track_color() + .map(|color| (padded_bounds.dilate(SCROLLBAR_PADDING), color)), reserved_space, } }) @@ -1076,6 +1106,7 @@ impl Element for ScrollbarElement { cursor_hitbox, axis, reserved_space, + track_background, .. } in &prepaint_state.thumbs { @@ -1092,7 +1123,9 @@ impl Element for ScrollbarElement { }; let blending_color = if hovered || reserved_space.needs_scroll_track() { - colors.surface_background + track_background + .map(|(_, background)| background) + .unwrap_or(colors.surface_background) } else { let blend_color = colors.surface_background; blend_color.min(blend_color.alpha(MAXIMUM_OPACITY)) @@ -1100,6 +1133,17 @@ impl Element for ScrollbarElement { let thumb_background = blending_color.blend(thumb_base_color); + if let Some((track_bounds, color)) = track_background { + window.paint_quad(quad( + *track_bounds, + Corners::default(), + *color, + Edges::default(), + Hsla::transparent_black(), + BorderStyle::default(), + )); + } + window.paint_quad(quad( *thumb_bounds, Corners::all(Pixels::MAX).clamp_radii_for_quad_size(thumb_bounds.size),