Move Keymap Editor Table component to UI crate (#38157)

Ben Kunkle created

Closes #ISSUE

Move the data table component created for the Keymap Editor to the UI
crate. Additionally includes simplifications to the scrollbar component
in UI necessary for the table component to support scrollbar
configurations, and a fix for an issue with the table component where
when used with the `.row` API instead of `uniform_list` the rows would
render on top of each other.

Release Notes:

- N/A *or* Added/Fixed/Improved ...

Change summary

crates/keymap_editor/src/keymap_editor.rs     |  29 +++--
crates/keymap_editor/src/ui_components/mod.rs |   1 
crates/ui/src/components.rs                   |   2 
crates/ui/src/components/data_table.rs        | 101 +++++++++++--------
crates/ui/src/components/scrollbar.rs         | 110 ++++++++------------
5 files changed, 121 insertions(+), 122 deletions(-)

Detailed changes

crates/keymap_editor/src/keymap_editor.rs 🔗

@@ -27,7 +27,8 @@ use settings::{BaseKeymap, KeybindSource, KeymapFile, Settings as _, SettingsAss
 use ui::{
     ActiveTheme as _, App, Banner, BorrowAppContext, ContextMenu, IconButtonShape, Indicator,
     Modal, ModalFooter, ModalHeader, ParentElement as _, Render, Section, SharedString,
-    Styled as _, Tooltip, Window, prelude::*, right_click_menu,
+    Styled as _, Table, TableColumnWidths, TableInteractionState, TableResizeBehavior, Tooltip,
+    Window, prelude::*, right_click_menu,
 };
 use ui_input::SingleLineInput;
 use util::ResultExt;
@@ -41,9 +42,8 @@ use zed_actions::OpenKeymapEditor;
 
 use crate::{
     persistence::KEYBINDING_EDITORS,
-    ui_components::{
-        keystroke_input::{ClearKeystrokes, KeystrokeInput, StartRecording, StopRecording},
-        table::{ColumnWidths, ResizeBehavior, Table, TableInteractionState},
+    ui_components::keystroke_input::{
+        ClearKeystrokes, KeystrokeInput, StartRecording, StopRecording,
     },
 };
 
@@ -369,7 +369,7 @@ struct KeymapEditor {
     context_menu: Option<(Entity<ContextMenu>, Point<Pixels>, Subscription)>,
     previous_edit: Option<PreviousEdit>,
     humanized_action_names: HumanizedActionNameCache,
-    current_widths: Entity<ColumnWidths<6>>,
+    current_widths: Entity<TableColumnWidths<6>>,
     show_hover_menus: bool,
     /// In order for the JSON LSP to run in the actions arguments editor, we
     /// require a backing file In order to avoid issues (primarily log spam)
@@ -425,7 +425,10 @@ impl KeymapEditor {
     fn new(workspace: WeakEntity<Workspace>, window: &mut Window, cx: &mut Context<Self>) -> Self {
         let _keymap_subscription =
             cx.observe_global_in::<KeymapEventChannel>(window, Self::on_keymap_changed);
-        let table_interaction_state = TableInteractionState::new(cx);
+        let table_interaction_state = cx.new(|cx| {
+            TableInteractionState::new(cx)
+                .with_custom_scrollbar(ui::Scrollbars::for_settings::<editor::EditorSettings>())
+        });
 
         let keystroke_editor = cx.new(|cx| {
             let mut keystroke_editor = KeystrokeInput::new(None, window, cx);
@@ -496,7 +499,7 @@ impl KeymapEditor {
             show_hover_menus: true,
             action_args_temp_dir: None,
             action_args_temp_dir_worktree: None,
-            current_widths: cx.new(|cx| ColumnWidths::new(cx)),
+            current_widths: cx.new(|cx| TableColumnWidths::new(cx)),
         };
 
         this.on_keymap_changed(window, cx);
@@ -1780,12 +1783,12 @@ impl Render for KeymapEditor {
                     ])
                     .resizable_columns(
                         [
-                            ResizeBehavior::None,
-                            ResizeBehavior::Resizable,
-                            ResizeBehavior::Resizable,
-                            ResizeBehavior::Resizable,
-                            ResizeBehavior::Resizable,
-                            ResizeBehavior::Resizable, // this column doesn't matter
+                            TableResizeBehavior::None,
+                            TableResizeBehavior::Resizable,
+                            TableResizeBehavior::Resizable,
+                            TableResizeBehavior::Resizable,
+                            TableResizeBehavior::Resizable,
+                            TableResizeBehavior::Resizable, // this column doesn't matter
                         ],
                         &self.current_widths,
                         cx,

crates/ui/src/components.rs 🔗

@@ -6,6 +6,7 @@ mod callout;
 mod chip;
 mod content_group;
 mod context_menu;
+mod data_table;
 mod disclosure;
 mod divider;
 mod dropdown_menu;
@@ -49,6 +50,7 @@ pub use callout::*;
 pub use chip::*;
 pub use content_group::*;
 pub use context_menu::*;
+pub use data_table::*;
 pub use disclosure::*;
 pub use divider::*;
 pub use dropdown_menu::*;

crates/keymap_editor/src/ui_components/table.rs → crates/ui/src/components/data_table.rs 🔗

@@ -1,14 +1,12 @@
 use std::{ops::Range, rc::Rc};
 
-use editor::EditorSettings;
 use gpui::{
     AbsoluteLength, AppContext, Context, DefiniteLength, DragMoveEvent, Entity, EntityId,
     FocusHandle, Length, ListHorizontalSizingBehavior, ListSizingBehavior, Point, Stateful,
     UniformListScrollHandle, WeakEntity, transparent_black, uniform_list,
 };
 
-use itertools::intersperse_with;
-use ui::{
+use crate::{
     ActiveTheme as _, AnyElement, App, Button, ButtonCommon as _, ButtonStyle, Color, Component,
     ComponentScope, Div, ElementId, FixedWidth as _, FluentBuilder as _, Indicator,
     InteractiveElement, IntoElement, ParentElement, Pixels, RegisterComponent, RenderOnce,
@@ -16,6 +14,7 @@ use ui::{
     StyledTypography, Window, WithScrollbar, div, example_group_with_title, h_flex, px,
     single_example, v_flex,
 };
+use itertools::intersperse_with;
 
 const RESIZE_COLUMN_WIDTH: f32 = 8.0;
 
@@ -56,14 +55,21 @@ impl<const COLS: usize> TableContents<COLS> {
 pub struct TableInteractionState {
     pub focus_handle: FocusHandle,
     pub scroll_handle: UniformListScrollHandle,
+    pub custom_scrollbar: Option<Scrollbars>,
 }
 
 impl TableInteractionState {
-    pub fn new(cx: &mut App) -> Entity<Self> {
-        cx.new(|cx| Self {
+    pub fn new(cx: &mut App) -> Self {
+        Self {
             focus_handle: cx.focus_handle(),
             scroll_handle: UniformListScrollHandle::new(),
-        })
+            custom_scrollbar: None,
+        }
+    }
+
+    pub fn with_custom_scrollbar(mut self, custom_scrollbar: Scrollbars) -> Self {
+        self.custom_scrollbar = Some(custom_scrollbar);
+        self
     }
 
     pub fn scroll_offset(&self) -> Point<Pixels> {
@@ -87,9 +93,9 @@ impl TableInteractionState {
     fn render_resize_handles<const COLS: usize>(
         &self,
         column_widths: &[Length; COLS],
-        resizable_columns: &[ResizeBehavior; COLS],
+        resizable_columns: &[TableResizeBehavior; COLS],
         initial_sizes: [DefiniteLength; COLS],
-        columns: Option<Entity<ColumnWidths<COLS>>>,
+        columns: Option<Entity<TableColumnWidths<COLS>>>,
         window: &mut Window,
         cx: &mut App,
     ) -> AnyElement {
@@ -121,7 +127,7 @@ impl TableInteractionState {
 
                 if resizable_columns
                     .next()
-                    .is_some_and(ResizeBehavior::is_resizable)
+                    .is_some_and(TableResizeBehavior::is_resizable)
                 {
                     let hovered = window.use_state(cx, |_window, _cx| false);
 
@@ -169,34 +175,34 @@ impl TableInteractionState {
 }
 
 #[derive(Debug, Copy, Clone, PartialEq)]
-pub enum ResizeBehavior {
+pub enum TableResizeBehavior {
     None,
     Resizable,
     MinSize(f32),
 }
 
-impl ResizeBehavior {
+impl TableResizeBehavior {
     pub fn is_resizable(&self) -> bool {
-        *self != ResizeBehavior::None
+        *self != TableResizeBehavior::None
     }
 
     pub fn min_size(&self) -> Option<f32> {
         match self {
-            ResizeBehavior::None => None,
-            ResizeBehavior::Resizable => Some(0.05),
-            ResizeBehavior::MinSize(min_size) => Some(*min_size),
+            TableResizeBehavior::None => None,
+            TableResizeBehavior::Resizable => Some(0.05),
+            TableResizeBehavior::MinSize(min_size) => Some(*min_size),
         }
     }
 }
 
-pub struct ColumnWidths<const COLS: usize> {
+pub struct TableColumnWidths<const COLS: usize> {
     widths: [DefiniteLength; COLS],
     visible_widths: [DefiniteLength; COLS],
     cached_bounds_width: Pixels,
     initialized: bool,
 }
 
-impl<const COLS: usize> ColumnWidths<COLS> {
+impl<const COLS: usize> TableColumnWidths<COLS> {
     pub fn new(_: &mut App) -> Self {
         Self {
             widths: [DefiniteLength::default(); COLS],
@@ -220,7 +226,7 @@ impl<const COLS: usize> ColumnWidths<COLS> {
         &mut self,
         double_click_position: usize,
         initial_sizes: &[DefiniteLength; COLS],
-        resize_behavior: &[ResizeBehavior; COLS],
+        resize_behavior: &[TableResizeBehavior; COLS],
         window: &mut Window,
     ) {
         let bounds_width = self.cached_bounds_width;
@@ -245,7 +251,7 @@ impl<const COLS: usize> ColumnWidths<COLS> {
         col_idx: usize,
         mut widths: [f32; COLS],
         initial_sizes: [f32; COLS],
-        resize_behavior: &[ResizeBehavior; COLS],
+        resize_behavior: &[TableResizeBehavior; COLS],
     ) -> [f32; COLS] {
         // RESET:
         // Part 1:
@@ -331,7 +337,7 @@ impl<const COLS: usize> ColumnWidths<COLS> {
     fn on_drag_move(
         &mut self,
         drag_event: &DragMoveEvent<DraggedColumn>,
-        resize_behavior: &[ResizeBehavior; COLS],
+        resize_behavior: &[TableResizeBehavior; COLS],
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
@@ -376,7 +382,7 @@ impl<const COLS: usize> ColumnWidths<COLS> {
         diff: f32,
         col_idx: usize,
         widths: &mut [f32; COLS],
-        resize_behavior: &[ResizeBehavior; COLS],
+        resize_behavior: &[TableResizeBehavior; COLS],
     ) {
         // if diff > 0.0 then go right
         if diff > 0.0 {
@@ -390,7 +396,7 @@ impl<const COLS: usize> ColumnWidths<COLS> {
         diff: f32,
         col_idx: usize,
         widths: &mut [f32; COLS],
-        resize_behavior: &[ResizeBehavior; COLS],
+        resize_behavior: &[TableResizeBehavior; COLS],
         direction: i8,
     ) -> f32 {
         let mut diff_remaining = diff;
@@ -446,8 +452,8 @@ impl<const COLS: usize> ColumnWidths<COLS> {
 
 pub struct TableWidths<const COLS: usize> {
     initial: [DefiniteLength; COLS],
-    current: Option<Entity<ColumnWidths<COLS>>>,
-    resizable: [ResizeBehavior; COLS],
+    current: Option<Entity<TableColumnWidths<COLS>>>,
+    resizable: [TableResizeBehavior; COLS],
 }
 
 impl<const COLS: usize> TableWidths<COLS> {
@@ -457,7 +463,7 @@ impl<const COLS: usize> TableWidths<COLS> {
         TableWidths {
             initial: widths,
             current: None,
-            resizable: [ResizeBehavior::None; COLS],
+            resizable: [TableResizeBehavior::None; COLS],
         }
     }
 
@@ -563,8 +569,8 @@ impl<const COLS: usize> Table<COLS> {
 
     pub fn resizable_columns(
         mut self,
-        resizable: [ResizeBehavior; COLS],
-        column_widths: &Entity<ColumnWidths<COLS>>,
+        resizable: [TableResizeBehavior; COLS],
+        column_widths: &Entity<TableColumnWidths<COLS>>,
         cx: &mut App,
     ) -> Self {
         if let Some(table_widths) = self.col_widths.as_mut() {
@@ -616,7 +622,7 @@ fn base_cell_style_text(width: Option<Length>, cx: &App) -> Div {
     base_cell_style(width).text_ui(cx)
 }
 
-pub fn render_row<const COLS: usize>(
+pub fn render_table_row<const COLS: usize>(
     row_index: usize,
     items: [impl IntoElement; COLS],
     table_context: TableRenderContext<COLS>,
@@ -663,12 +669,12 @@ pub fn render_row<const COLS: usize>(
     div().size_full().child(row).into_any_element()
 }
 
-pub fn render_header<const COLS: usize>(
+pub fn render_table_header<const COLS: usize>(
     headers: [impl IntoElement; COLS],
     table_context: TableRenderContext<COLS>,
     columns_widths: Option<(
-        WeakEntity<ColumnWidths<COLS>>,
-        [ResizeBehavior; COLS],
+        WeakEntity<TableColumnWidths<COLS>>,
+        [TableResizeBehavior; COLS],
         [DefiniteLength; COLS],
     )>,
     entity_id: Option<EntityId>,
@@ -771,7 +777,7 @@ impl<const COLS: usize> RenderOnce for Table<COLS> {
             .h_full()
             .v_flex()
             .when_some(self.headers.take(), |this, headers| {
-                this.child(render_header(
+                this.child(render_table_header(
                     headers,
                     table_context.clone(),
                     current_widths_with_initial_sizes,
@@ -822,7 +828,13 @@ impl<const COLS: usize> RenderOnce for Table<COLS> {
                     .map(|parent| match self.rows {
                         TableContents::Vec(items) => {
                             parent.children(items.into_iter().enumerate().map(|(index, row)| {
-                                render_row(index, row, table_context.clone(), window, cx)
+                                div().child(render_table_row(
+                                    index,
+                                    row,
+                                    table_context.clone(),
+                                    window,
+                                    cx,
+                                ))
                             }))
                         }
                         TableContents::UniformList(uniform_list_data) => parent.child(
@@ -837,7 +849,7 @@ impl<const COLS: usize> RenderOnce for Table<COLS> {
                                             .into_iter()
                                             .zip(range)
                                             .map(|(row, row_index)| {
-                                                render_row(
+                                                render_table_row(
                                                     row_index,
                                                     row,
                                                     table_context.clone(),
@@ -888,10 +900,14 @@ impl<const COLS: usize> RenderOnce for Table<COLS> {
                     );
 
                 if let Some(state) = interaction_state.as_ref() {
+                    let scrollbars = state
+                        .read(cx)
+                        .custom_scrollbar
+                        .clone()
+                        .unwrap_or_else(|| Scrollbars::new(super::ScrollAxes::Both));
                     content
                         .custom_scrollbars(
-                            Scrollbars::for_settings::<EditorSettings>()
-                                .tracked_scroll_handle(state.read(cx).scroll_handle.clone()),
+                            scrollbars.tracked_scroll_handle(state.read(cx).scroll_handle.clone()),
                             window,
                             cx,
                         )
@@ -1055,14 +1071,15 @@ mod test {
     fn parse_resize_behavior<const COLS: usize>(
         input: &str,
         total_size: f32,
-    ) -> [ResizeBehavior; COLS] {
-        let mut resize_behavior = [ResizeBehavior::None; COLS];
+    ) -> [TableResizeBehavior; COLS] {
+        let mut resize_behavior = [TableResizeBehavior::None; COLS];
         let mut max_index = 0;
         for (index, col) in input.split('|').enumerate() {
             if col.starts_with('X') || col.is_empty() {
-                resize_behavior[index] = ResizeBehavior::None;
+                resize_behavior[index] = TableResizeBehavior::None;
             } else if col.starts_with('*') {
-                resize_behavior[index] = ResizeBehavior::MinSize(col.len() as f32 / total_size);
+                resize_behavior[index] =
+                    TableResizeBehavior::MinSize(col.len() as f32 / total_size);
             } else {
                 panic!("invalid test input: unrecognized resize behavior: {}", col);
             }
@@ -1123,7 +1140,7 @@ mod test {
                 "invalid test input: total width not the same"
             );
             let resize_behavior = parse_resize_behavior::<COLS>(resize_behavior, total_1);
-            let result = ColumnWidths::reset_to_initial_size(
+            let result = TableColumnWidths::reset_to_initial_size(
                 column_index,
                 widths,
                 initial_sizes,
@@ -1301,7 +1318,7 @@ mod test {
 
             let distance = distance as f32 / total_1;
 
-            let result = ColumnWidths::drag_column_handle(
+            let result = TableColumnWidths::drag_column_handle(
                 distance,
                 column_index,
                 &mut widths,

crates/ui/src/components/scrollbar.rs 🔗

@@ -1,4 +1,4 @@
-use std::{any::Any, fmt::Debug, marker::PhantomData, ops::Not, time::Duration};
+use std::{any::Any, fmt::Debug, ops::Not, time::Duration};
 
 use gpui::{
     Along, App, AppContext as _, Axis as ScrollbarAxis, BorderStyle, Bounds, ContentMask, Context,
@@ -68,22 +68,10 @@ pub mod scrollbars {
         }
     }
 
-    impl GlobalSetting for ShowScrollbar {
-        fn get_value(_cx: &App) -> &Self {
-            &ShowScrollbar::Always
-        }
-    }
-
     pub trait ScrollbarVisibility: GlobalSetting + 'static {
         fn visibility(&self, cx: &App) -> ShowScrollbar;
     }
 
-    impl ScrollbarVisibility for ShowScrollbar {
-        fn visibility(&self, cx: &App) -> ShowScrollbar {
-            *ShowScrollbar::get_value(cx)
-        }
-    }
-
     #[derive(Default)]
     pub struct ScrollbarAutoHide(pub bool);
 
@@ -96,14 +84,13 @@ pub mod scrollbars {
     impl Global for ScrollbarAutoHide {}
 }
 
-fn get_scrollbar_state<S, T>(
-    mut config: Scrollbars<S, T>,
+fn get_scrollbar_state<T>(
+    mut config: Scrollbars<T>,
     caller_location: &'static std::panic::Location,
     window: &mut Window,
     cx: &mut App,
-) -> Entity<ScrollbarStateWrapper<S, T>>
+) -> Entity<ScrollbarStateWrapper<T>>
 where
-    S: ScrollbarVisibility,
     T: ScrollableHandle,
 {
     let element_id = config.id.take().unwrap_or_else(|| caller_location.into());
@@ -119,14 +106,13 @@ where
 pub trait WithScrollbar: Sized {
     type Output;
 
-    fn custom_scrollbars<S, T>(
+    fn custom_scrollbars<T>(
         self,
-        config: Scrollbars<S, T>,
+        config: Scrollbars<T>,
         window: &mut Window,
         cx: &mut App,
     ) -> Self::Output
     where
-        S: ScrollbarVisibility,
         T: ScrollableHandle;
 
     #[track_caller]
@@ -168,14 +154,13 @@ impl WithScrollbar for Stateful<Div> {
     type Output = Self;
 
     #[track_caller]
-    fn custom_scrollbars<S, T>(
+    fn custom_scrollbars<T>(
         self,
-        config: Scrollbars<S, T>,
+        config: Scrollbars<T>,
         window: &mut Window,
         cx: &mut App,
     ) -> Self::Output
     where
-        S: ScrollbarVisibility,
         T: ScrollableHandle,
     {
         render_scrollbar(
@@ -190,14 +175,13 @@ impl WithScrollbar for Div {
     type Output = Stateful<Div>;
 
     #[track_caller]
-    fn custom_scrollbars<S, T>(
+    fn custom_scrollbars<T>(
         self,
-        config: Scrollbars<S, T>,
+        config: Scrollbars<T>,
         window: &mut Window,
         cx: &mut App,
     ) -> Self::Output
     where
-        S: ScrollbarVisibility,
         T: ScrollableHandle,
     {
         let scrollbar = get_scrollbar_state(config, std::panic::Location::caller(), window, cx);
@@ -213,13 +197,12 @@ impl WithScrollbar for Div {
     }
 }
 
-fn render_scrollbar<S, T>(
-    scrollbar: Entity<ScrollbarStateWrapper<S, T>>,
+fn render_scrollbar<T>(
+    scrollbar: Entity<ScrollbarStateWrapper<T>>,
     div: Stateful<Div>,
     cx: &App,
 ) -> Stateful<Div>
 where
-    S: ScrollbarVisibility,
     T: ScrollableHandle,
 {
     let state = &scrollbar.read(cx).0;
@@ -247,9 +230,7 @@ where
     .child(state.clone())
 }
 
-impl<S: ScrollbarVisibility, T: ScrollableHandle> UniformListDecoration
-    for ScrollbarStateWrapper<S, T>
-{
+impl<T: ScrollableHandle> UniformListDecoration for ScrollbarStateWrapper<T> {
     fn compute(
         &self,
         _visible_range: Range<usize>,
@@ -334,7 +315,7 @@ impl ReservedSpace {
     }
 }
 
-#[derive(Debug, Default)]
+#[derive(Debug, Default, Clone, Copy)]
 enum ScrollbarWidth {
     #[default]
     Normal,
@@ -352,11 +333,12 @@ impl ScrollbarWidth {
     }
 }
 
-pub struct Scrollbars<S: ScrollbarVisibility = ShowScrollbar, T: ScrollableHandle = ScrollHandle> {
+#[derive(Clone)]
+pub struct Scrollbars<T: ScrollableHandle = ScrollHandle> {
     id: Option<ElementId>,
-    tracked_setting: PhantomData<S>,
+    get_visibility: fn(&App) -> ShowScrollbar,
     tracked_entity: Option<Option<EntityId>>,
-    scrollable_handle: Box<dyn FnOnce() -> T>,
+    scrollable_handle: T,
     handle_was_added: bool,
     visibility: Point<ReservedSpace>,
     scrollbar_width: ScrollbarWidth,
@@ -364,21 +346,21 @@ pub struct Scrollbars<S: ScrollbarVisibility = ShowScrollbar, T: ScrollableHandl
 
 impl Scrollbars {
     pub fn new(show_along: ScrollAxes) -> Self {
-        Self::new_with_setting(show_along)
+        Self::new_with_setting(show_along, |_| ShowScrollbar::Always)
     }
 
-    pub fn for_settings<S: ScrollbarVisibility>() -> Scrollbars<S> {
-        Scrollbars::<S>::new_with_setting(ScrollAxes::Both)
+    pub fn for_settings<S: ScrollbarVisibility>() -> Scrollbars {
+        Scrollbars::new_with_setting(ScrollAxes::Both, |cx| S::get_value(cx).visibility(cx))
     }
 }
 
-impl<S: ScrollbarVisibility> Scrollbars<S> {
-    fn new_with_setting(show_along: ScrollAxes) -> Self {
+impl Scrollbars {
+    fn new_with_setting(show_along: ScrollAxes, get_visibility: fn(&App) -> ShowScrollbar) -> Self {
         Self {
             id: None,
-            tracked_setting: PhantomData,
+            get_visibility,
             handle_was_added: false,
-            scrollable_handle: Box::new(ScrollHandle::new),
+            scrollable_handle: ScrollHandle::new(),
             tracked_entity: None,
             visibility: show_along.apply_to(Default::default(), ReservedSpace::Thumb),
             scrollbar_width: ScrollbarWidth::Normal,
@@ -386,9 +368,7 @@ impl<S: ScrollbarVisibility> Scrollbars<S> {
     }
 }
 
-impl<Setting: ScrollbarVisibility, ScrollHandle: ScrollableHandle>
-    Scrollbars<Setting, ScrollHandle>
-{
+impl<ScrollHandle: ScrollableHandle> Scrollbars<ScrollHandle> {
     pub fn id(mut self, id: impl Into<ElementId>) -> Self {
         self.id = Some(id.into());
         self
@@ -416,24 +396,24 @@ impl<Setting: ScrollbarVisibility, ScrollHandle: ScrollableHandle>
     pub fn tracked_scroll_handle<TrackedHandle: ScrollableHandle>(
         self,
         tracked_scroll_handle: TrackedHandle,
-    ) -> Scrollbars<Setting, TrackedHandle> {
+    ) -> Scrollbars<TrackedHandle> {
         let Self {
             id,
-            tracked_setting,
             tracked_entity: tracked_entity_id,
             scrollbar_width,
             visibility,
+            get_visibility,
             ..
         } = self;
 
         Scrollbars {
             handle_was_added: true,
-            scrollable_handle: Box::new(|| tracked_scroll_handle),
+            scrollable_handle: tracked_scroll_handle,
             id,
-            tracked_setting,
             tracked_entity: tracked_entity_id,
             visibility,
             scrollbar_width,
+            get_visibility,
         }
     }
 
@@ -491,19 +471,17 @@ enum ParentHovered {
 
 /// This is used to ensure notifies within the state do not notify the parent
 /// unintentionally.
-struct ScrollbarStateWrapper<S: ScrollbarVisibility, T: ScrollableHandle>(
-    Entity<ScrollbarState<S, T>>,
-);
+struct ScrollbarStateWrapper<T: ScrollableHandle>(Entity<ScrollbarState<T>>);
 
 /// A scrollbar state that should be persisted across frames.
-struct ScrollbarState<S: ScrollbarVisibility, T: ScrollableHandle = ScrollHandle> {
+struct ScrollbarState<T: ScrollableHandle = ScrollHandle> {
     thumb_state: ThumbState,
     notify_id: Option<EntityId>,
     manually_added: bool,
     scroll_handle: T,
     width: ScrollbarWidth,
-    tracked_setting: PhantomData<S>,
     show_setting: ShowScrollbar,
+    get_visibility: fn(&App) -> ShowScrollbar,
     visibility: Point<ReservedSpace>,
     show_state: VisibilityState,
     mouse_in_parent: bool,
@@ -511,9 +489,9 @@ struct ScrollbarState<S: ScrollbarVisibility, T: ScrollableHandle = ScrollHandle
     _auto_hide_task: Option<Task<()>>,
 }
 
-impl<S: ScrollbarVisibility, T: ScrollableHandle> ScrollbarState<S, T> {
+impl<T: ScrollableHandle> ScrollbarState<T> {
     fn new_from_config(
-        config: Scrollbars<S, T>,
+        config: Scrollbars<T>,
         parent_id: EntityId,
         window: &mut Window,
         cx: &mut Context<Self>,
@@ -521,16 +499,16 @@ impl<S: ScrollbarVisibility, T: ScrollableHandle> ScrollbarState<S, T> {
         cx.observe_global_in::<SettingsStore>(window, Self::settings_changed)
             .detach();
 
-        let show_setting = S::get_value(cx).visibility(cx);
+        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)(),
+            scroll_handle: config.scrollable_handle,
             width: config.scrollbar_width,
             visibility: config.visibility,
-            tracked_setting: PhantomData,
             show_setting,
+            get_visibility: config.get_visibility,
             show_state: VisibilityState::from_show_setting(show_setting),
             mouse_in_parent: true,
             last_prepaint_state: None,
@@ -539,7 +517,7 @@ impl<S: ScrollbarVisibility, T: ScrollableHandle> ScrollbarState<S, T> {
     }
 
     fn settings_changed(&mut self, window: &mut Window, cx: &mut Context<Self>) {
-        self.set_show_scrollbar(S::get_value(cx).visibility(cx), window, cx);
+        self.set_show_scrollbar((self.get_visibility)(cx), window, cx);
     }
 
     /// Schedules a scrollbar auto hide if no auto hide is currently in progress yet.
@@ -754,7 +732,7 @@ impl<S: ScrollbarVisibility, T: ScrollableHandle> ScrollbarState<S, T> {
     }
 }
 
-impl<S: ScrollbarVisibility, T: ScrollableHandle> Render for ScrollbarState<S, T> {
+impl<T: ScrollableHandle> Render for ScrollbarState<T> {
     fn render(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
         ScrollbarElement {
             state: cx.entity(),
@@ -763,9 +741,9 @@ impl<S: ScrollbarVisibility, T: ScrollableHandle> Render for ScrollbarState<S, T
     }
 }
 
-struct ScrollbarElement<S: ScrollbarVisibility, T: ScrollableHandle> {
+struct ScrollbarElement<T: ScrollableHandle> {
     origin: Point<Pixels>,
-    state: Entity<ScrollbarState<S, T>>,
+    state: Entity<ScrollbarState<T>>,
 }
 
 #[derive(Default, Debug, PartialEq, Eq)]
@@ -945,7 +923,7 @@ impl PartialEq for ScrollbarPrepaintState {
     }
 }
 
-impl<S: ScrollbarVisibility, T: ScrollableHandle> Element for ScrollbarElement<S, T> {
+impl<T: ScrollableHandle> Element for ScrollbarElement<T> {
     type RequestLayoutState = ();
     type PrepaintState = Option<ScrollbarPrepaintState>;
 
@@ -1266,7 +1244,7 @@ impl<S: ScrollbarVisibility, T: ScrollableHandle> Element for ScrollbarElement<S
     }
 }
 
-impl<S: ScrollbarVisibility, T: ScrollableHandle> IntoElement for ScrollbarElement<S, T> {
+impl<T: ScrollableHandle> IntoElement for ScrollbarElement<T> {
     type Element = Self;
 
     fn into_element(self) -> Self::Element {