From 1090c47a90c586b397dc3fdd0cd09b530f37e5a0 Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Sun, 14 Sep 2025 14:53:07 -0500 Subject: [PATCH] Move Keymap Editor Table component to UI crate (#38157) 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 ... --- crates/keymap_editor/src/keymap_editor.rs | 29 ++--- crates/keymap_editor/src/ui_components/mod.rs | 1 - crates/ui/src/components.rs | 2 + .../src/components/data_table.rs} | 101 +++++++++------- crates/ui/src/components/scrollbar.rs | 110 +++++++----------- 5 files changed, 121 insertions(+), 122 deletions(-) rename crates/{keymap_editor/src/ui_components/table.rs => ui/src/components/data_table.rs} (94%) diff --git a/crates/keymap_editor/src/keymap_editor.rs b/crates/keymap_editor/src/keymap_editor.rs index 51524f92eacbf6227877f1203050bb6f964033b3..8a1e494d09aec024c57fb8acf480e8aed7efff82 100644 --- a/crates/keymap_editor/src/keymap_editor.rs +++ b/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, Point, Subscription)>, previous_edit: Option, humanized_action_names: HumanizedActionNameCache, - current_widths: Entity>, + current_widths: Entity>, 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, window: &mut Window, cx: &mut Context) -> Self { let _keymap_subscription = cx.observe_global_in::(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::()) + }); 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, diff --git a/crates/keymap_editor/src/ui_components/mod.rs b/crates/keymap_editor/src/ui_components/mod.rs index 5d6463a61a21afd5208b75af0362f6f7956f5e56..c093bab554d9e4f3f2f74818d5016a0480053903 100644 --- a/crates/keymap_editor/src/ui_components/mod.rs +++ b/crates/keymap_editor/src/ui_components/mod.rs @@ -1,2 +1 @@ pub mod keystroke_input; -pub mod table; diff --git a/crates/ui/src/components.rs b/crates/ui/src/components.rs index 486673e73354b488753cead1f187a5f7ce7687cc..0b7061581518a16d49c677de10dd62ece65b24b4 100644 --- a/crates/ui/src/components.rs +++ b/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::*; diff --git a/crates/keymap_editor/src/ui_components/table.rs b/crates/ui/src/components/data_table.rs similarity index 94% rename from crates/keymap_editor/src/ui_components/table.rs rename to crates/ui/src/components/data_table.rs index cb0332c868032724d555f3fc5c57e33eaeda624b..4a1f4939cca2eb85bb7a549d06af1e9ea8cf04d0 100644 --- a/crates/keymap_editor/src/ui_components/table.rs +++ b/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 TableContents { pub struct TableInteractionState { pub focus_handle: FocusHandle, pub scroll_handle: UniformListScrollHandle, + pub custom_scrollbar: Option, } impl TableInteractionState { - pub fn new(cx: &mut App) -> Entity { - 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 { @@ -87,9 +93,9 @@ impl TableInteractionState { fn render_resize_handles( &self, column_widths: &[Length; COLS], - resizable_columns: &[ResizeBehavior; COLS], + resizable_columns: &[TableResizeBehavior; COLS], initial_sizes: [DefiniteLength; COLS], - columns: Option>>, + columns: Option>>, 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 { 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 { +pub struct TableColumnWidths { widths: [DefiniteLength; COLS], visible_widths: [DefiniteLength; COLS], cached_bounds_width: Pixels, initialized: bool, } -impl ColumnWidths { +impl TableColumnWidths { pub fn new(_: &mut App) -> Self { Self { widths: [DefiniteLength::default(); COLS], @@ -220,7 +226,7 @@ impl ColumnWidths { &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 ColumnWidths { 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 ColumnWidths { fn on_drag_move( &mut self, drag_event: &DragMoveEvent, - resize_behavior: &[ResizeBehavior; COLS], + resize_behavior: &[TableResizeBehavior; COLS], window: &mut Window, cx: &mut Context, ) { @@ -376,7 +382,7 @@ impl ColumnWidths { 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 ColumnWidths { 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 ColumnWidths { pub struct TableWidths { initial: [DefiniteLength; COLS], - current: Option>>, - resizable: [ResizeBehavior; COLS], + current: Option>>, + resizable: [TableResizeBehavior; COLS], } impl TableWidths { @@ -457,7 +463,7 @@ impl TableWidths { TableWidths { initial: widths, current: None, - resizable: [ResizeBehavior::None; COLS], + resizable: [TableResizeBehavior::None; COLS], } } @@ -563,8 +569,8 @@ impl Table { pub fn resizable_columns( mut self, - resizable: [ResizeBehavior; COLS], - column_widths: &Entity>, + resizable: [TableResizeBehavior; COLS], + column_widths: &Entity>, 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, cx: &App) -> Div { base_cell_style(width).text_ui(cx) } -pub fn render_row( +pub fn render_table_row( row_index: usize, items: [impl IntoElement; COLS], table_context: TableRenderContext, @@ -663,12 +669,12 @@ pub fn render_row( div().size_full().child(row).into_any_element() } -pub fn render_header( +pub fn render_table_header( headers: [impl IntoElement; COLS], table_context: TableRenderContext, columns_widths: Option<( - WeakEntity>, - [ResizeBehavior; COLS], + WeakEntity>, + [TableResizeBehavior; COLS], [DefiniteLength; COLS], )>, entity_id: Option, @@ -771,7 +777,7 @@ impl RenderOnce for Table { .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 RenderOnce for Table { .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 RenderOnce for Table { .into_iter() .zip(range) .map(|(row, row_index)| { - render_row( + render_table_row( row_index, row, table_context.clone(), @@ -888,10 +900,14 @@ impl RenderOnce for Table { ); 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::() - .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( 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::(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, diff --git a/crates/ui/src/components/scrollbar.rs b/crates/ui/src/components/scrollbar.rs index e4c2937be775eed4d9ae673da763e1eda5a65419..f3cc37d2f55356f05af3f1644dc4292a6add2660 100644 --- a/crates/ui/src/components/scrollbar.rs +++ b/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( - mut config: Scrollbars, +fn get_scrollbar_state( + mut config: Scrollbars, caller_location: &'static std::panic::Location, window: &mut Window, cx: &mut App, -) -> Entity> +) -> Entity> 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( + fn custom_scrollbars( self, - config: Scrollbars, + config: Scrollbars, window: &mut Window, cx: &mut App, ) -> Self::Output where - S: ScrollbarVisibility, T: ScrollableHandle; #[track_caller] @@ -168,14 +154,13 @@ impl WithScrollbar for Stateful
{ type Output = Self; #[track_caller] - fn custom_scrollbars( + fn custom_scrollbars( self, - config: Scrollbars, + config: Scrollbars, 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
; #[track_caller] - fn custom_scrollbars( + fn custom_scrollbars( self, - config: Scrollbars, + config: Scrollbars, 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( - scrollbar: Entity>, +fn render_scrollbar( + scrollbar: Entity>, div: Stateful
, cx: &App, ) -> Stateful
where - S: ScrollbarVisibility, T: ScrollableHandle, { let state = &scrollbar.read(cx).0; @@ -247,9 +230,7 @@ where .child(state.clone()) } -impl UniformListDecoration - for ScrollbarStateWrapper -{ +impl UniformListDecoration for ScrollbarStateWrapper { fn compute( &self, _visible_range: Range, @@ -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 { +#[derive(Clone)] +pub struct Scrollbars { id: Option, - tracked_setting: PhantomData, + get_visibility: fn(&App) -> ShowScrollbar, tracked_entity: Option>, - scrollable_handle: Box T>, + scrollable_handle: T, handle_was_added: bool, visibility: Point, scrollbar_width: ScrollbarWidth, @@ -364,21 +346,21 @@ pub struct Scrollbars Self { - Self::new_with_setting(show_along) + Self::new_with_setting(show_along, |_| ShowScrollbar::Always) } - pub fn for_settings() -> Scrollbars { - Scrollbars::::new_with_setting(ScrollAxes::Both) + pub fn for_settings() -> Scrollbars { + Scrollbars::new_with_setting(ScrollAxes::Both, |cx| S::get_value(cx).visibility(cx)) } } -impl Scrollbars { - 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 Scrollbars { } } -impl - Scrollbars -{ +impl Scrollbars { pub fn id(mut self, id: impl Into) -> Self { self.id = Some(id.into()); self @@ -416,24 +396,24 @@ impl pub fn tracked_scroll_handle( self, tracked_scroll_handle: TrackedHandle, - ) -> Scrollbars { + ) -> Scrollbars { 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( - Entity>, -); +struct ScrollbarStateWrapper(Entity>); /// A scrollbar state that should be persisted across frames. -struct ScrollbarState { +struct ScrollbarState { thumb_state: ThumbState, notify_id: Option, manually_added: bool, scroll_handle: T, width: ScrollbarWidth, - tracked_setting: PhantomData, show_setting: ShowScrollbar, + get_visibility: fn(&App) -> ShowScrollbar, visibility: Point, show_state: VisibilityState, mouse_in_parent: bool, @@ -511,9 +489,9 @@ struct ScrollbarState>, } -impl ScrollbarState { +impl ScrollbarState { fn new_from_config( - config: Scrollbars, + config: Scrollbars, parent_id: EntityId, window: &mut Window, cx: &mut Context, @@ -521,16 +499,16 @@ impl ScrollbarState { cx.observe_global_in::(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 ScrollbarState { } fn settings_changed(&mut self, window: &mut Window, cx: &mut Context) { - 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 ScrollbarState { } } -impl Render for ScrollbarState { +impl Render for ScrollbarState { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { ScrollbarElement { state: cx.entity(), @@ -763,9 +741,9 @@ impl Render for ScrollbarState { +struct ScrollbarElement { origin: Point, - state: Entity>, + state: Entity>, } #[derive(Default, Debug, PartialEq, Eq)] @@ -945,7 +923,7 @@ impl PartialEq for ScrollbarPrepaintState { } } -impl Element for ScrollbarElement { +impl Element for ScrollbarElement { type RequestLayoutState = (); type PrepaintState = Option; @@ -1266,7 +1244,7 @@ impl Element for ScrollbarElement IntoElement for ScrollbarElement { +impl IntoElement for ScrollbarElement { type Element = Self; fn into_element(self) -> Self::Element {