From 63bd4ac999c3b3abf1f201e23b101294d32a35e0 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Tue, 28 Nov 2023 10:33:13 -0500 Subject: [PATCH 1/3] Allow `render_match` to return an `Option` to represent no matches --- .../command_palette2/src/command_palette.rs | 42 ++++++++++--------- crates/file_finder2/src/file_finder.rs | 30 ++++++------- crates/picker2/src/picker2.rs | 4 +- crates/storybook2/src/stories/picker.rs | 28 +++++++------ 4 files changed, 55 insertions(+), 49 deletions(-) diff --git a/crates/command_palette2/src/command_palette.rs b/crates/command_palette2/src/command_palette.rs index 07b819d3a10c054ea20cddac787d28e792d3b187..73dca81b9327c25ec4b2755c50516c40b9f93362 100644 --- a/crates/command_palette2/src/command_palette.rs +++ b/crates/command_palette2/src/command_palette.rs @@ -294,32 +294,34 @@ impl PickerDelegate for CommandPaletteDelegate { ix: usize, selected: bool, cx: &mut ViewContext>, - ) -> Self::ListItem { + ) -> Option { let colors = cx.theme().colors(); let Some(r#match) = self.matches.get(ix) else { - return div(); + return None; }; let Some(command) = self.commands.get(r#match.candidate_id) else { - return div(); + return None; }; - div() - .px_1() - .text_color(colors.text) - .text_ui() - .bg(colors.ghost_element_background) - .rounded_md() - .when(selected, |this| this.bg(colors.ghost_element_selected)) - .hover(|this| this.bg(colors.ghost_element_hover)) - .child( - h_stack() - .justify_between() - .child(HighlightedLabel::new( - command.name.clone(), - r#match.positions.clone(), - )) - .children(KeyBinding::for_action(&*command.action, cx)), - ) + Some( + div() + .px_1() + .text_color(colors.text) + .text_ui() + .bg(colors.ghost_element_background) + .rounded_md() + .when(selected, |this| this.bg(colors.ghost_element_selected)) + .hover(|this| this.bg(colors.ghost_element_hover)) + .child( + h_stack() + .justify_between() + .child(HighlightedLabel::new( + command.name.clone(), + r#match.positions.clone(), + )) + .children(KeyBinding::for_action(&*command.action, cx)), + ), + ) } } diff --git a/crates/file_finder2/src/file_finder.rs b/crates/file_finder2/src/file_finder.rs index ea578fbb0ebb5703f3f31ed142f5589a0345c814..c7d8e9eae0c1ed3d3c721af8e5a4dfb706637ddb 100644 --- a/crates/file_finder2/src/file_finder.rs +++ b/crates/file_finder2/src/file_finder.rs @@ -711,7 +711,7 @@ impl PickerDelegate for FileFinderDelegate { ix: usize, selected: bool, cx: &mut ViewContext>, - ) -> Self::ListItem { + ) -> Option { let path_match = self .matches .get(ix) @@ -722,19 +722,21 @@ impl PickerDelegate for FileFinderDelegate { let (file_name, file_name_positions, full_path, full_path_positions) = self.labels_for_match(path_match, cx, ix); - div() - .px_1() - .text_color(colors.text) - .text_ui() - .bg(colors.ghost_element_background) - .rounded_md() - .when(selected, |this| this.bg(colors.ghost_element_selected)) - .hover(|this| this.bg(colors.ghost_element_hover)) - .child( - v_stack() - .child(HighlightedLabel::new(file_name, file_name_positions)) - .child(HighlightedLabel::new(full_path, full_path_positions)), - ) + Some( + div() + .px_1() + .text_color(colors.text) + .text_ui() + .bg(colors.ghost_element_background) + .rounded_md() + .when(selected, |this| this.bg(colors.ghost_element_selected)) + .hover(|this| this.bg(colors.ghost_element_hover)) + .child( + v_stack() + .child(HighlightedLabel::new(file_name, file_name_positions)) + .child(HighlightedLabel::new(full_path, full_path_positions)), + ), + ) } } diff --git a/crates/picker2/src/picker2.rs b/crates/picker2/src/picker2.rs index dc6b77c7c7b21db00cc3cb1b6ae23588e07d6c36..ea31419201748ade52647504026d3d23e5ac27ea 100644 --- a/crates/picker2/src/picker2.rs +++ b/crates/picker2/src/picker2.rs @@ -32,7 +32,7 @@ pub trait PickerDelegate: Sized + 'static { ix: usize, selected: bool, cx: &mut ViewContext>, - ) -> Self::ListItem; + ) -> Option; } impl FocusableView for Picker { @@ -230,7 +230,7 @@ impl Render for Picker { ) }), ) - .child(picker.delegate.render_match( + .children(picker.delegate.render_match( ix, ix == selected_index, cx, diff --git a/crates/storybook2/src/stories/picker.rs b/crates/storybook2/src/stories/picker.rs index ae6a26161bd3a587c54aa7231336d40428dbbe69..f66fe7a93af28ddf5ba5274594fc03597299d915 100644 --- a/crates/storybook2/src/stories/picker.rs +++ b/crates/storybook2/src/stories/picker.rs @@ -51,25 +51,27 @@ impl PickerDelegate for Delegate { ix: usize, selected: bool, cx: &mut gpui::ViewContext>, - ) -> Self::ListItem { + ) -> Option { let colors = cx.theme().colors(); let Some(candidate_ix) = self.matches.get(ix) else { - return div(); + return None; }; // TASK: Make StringMatchCandidate::string a SharedString let candidate = SharedString::from(self.candidates[*candidate_ix].string.clone()); - div() - .text_color(colors.text) - .when(selected, |s| { - s.border_l_10().border_color(colors.terminal_ansi_yellow) - }) - .hover(|style| { - style - .bg(colors.element_active) - .text_color(colors.text_accent) - }) - .child(candidate) + Some( + div() + .text_color(colors.text) + .when(selected, |s| { + s.border_l_10().border_color(colors.terminal_ansi_yellow) + }) + .hover(|style| { + style + .bg(colors.element_active) + .text_color(colors.text_accent) + }) + .child(candidate), + ) } fn selected_index(&self) -> usize { From 1ee109cec723ae8e8816eae8961503fb052db0b3 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Tue, 28 Nov 2023 10:44:57 -0500 Subject: [PATCH 2/3] Use `ListItem` when rendering picker matches --- .../command_palette2/src/command_palette.rs | 37 +++++++------------ crates/file_finder2/src/file_finder.rs | 30 +++++---------- crates/ui2/src/components/list.rs | 30 ++++++--------- 3 files changed, 33 insertions(+), 64 deletions(-) diff --git a/crates/command_palette2/src/command_palette.rs b/crates/command_palette2/src/command_palette.rs index 73dca81b9327c25ec4b2755c50516c40b9f93362..5f3fb4a98595afc3b6930370bd552f0912af6344 100644 --- a/crates/command_palette2/src/command_palette.rs +++ b/crates/command_palette2/src/command_palette.rs @@ -1,17 +1,15 @@ use collections::{CommandPaletteFilter, HashMap}; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ - actions, div, prelude::*, Action, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, - FocusableView, Keystroke, ParentElement, Render, Styled, View, ViewContext, VisualContext, - WeakView, + actions, Action, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, FocusableView, + Keystroke, ParentElement, Render, Styled, View, ViewContext, VisualContext, WeakView, }; use picker::{Picker, PickerDelegate}; use std::{ cmp::{self, Reverse}, sync::Arc, }; -use theme::ActiveTheme; -use ui::{h_stack, v_stack, HighlightedLabel, KeyBinding, StyledExt}; +use ui::{h_stack, v_stack, HighlightedLabel, KeyBinding, ListItem}; use util::{ channel::{parse_zed_link, ReleaseChannel, RELEASE_CHANNEL}, ResultExt, @@ -141,7 +139,7 @@ impl CommandPaletteDelegate { } impl PickerDelegate for CommandPaletteDelegate { - type ListItem = Div; + type ListItem = ListItem; fn placeholder_text(&self) -> Arc { "Execute a command...".into() @@ -295,7 +293,6 @@ impl PickerDelegate for CommandPaletteDelegate { selected: bool, cx: &mut ViewContext>, ) -> Option { - let colors = cx.theme().colors(); let Some(r#match) = self.matches.get(ix) else { return None; }; @@ -304,23 +301,15 @@ impl PickerDelegate for CommandPaletteDelegate { }; Some( - div() - .px_1() - .text_color(colors.text) - .text_ui() - .bg(colors.ghost_element_background) - .rounded_md() - .when(selected, |this| this.bg(colors.ghost_element_selected)) - .hover(|this| this.bg(colors.ghost_element_hover)) - .child( - h_stack() - .justify_between() - .child(HighlightedLabel::new( - command.name.clone(), - r#match.positions.clone(), - )) - .children(KeyBinding::for_action(&*command.action, cx)), - ), + ListItem::new(ix).selected(selected).child( + h_stack() + .justify_between() + .child(HighlightedLabel::new( + command.name.clone(), + r#match.positions.clone(), + )) + .children(KeyBinding::for_action(&*command.action, cx)), + ), ) } } diff --git a/crates/file_finder2/src/file_finder.rs b/crates/file_finder2/src/file_finder.rs index c7d8e9eae0c1ed3d3c721af8e5a4dfb706637ddb..b54d28a400750b4caffbec99d163f561cf7f7784 100644 --- a/crates/file_finder2/src/file_finder.rs +++ b/crates/file_finder2/src/file_finder.rs @@ -2,9 +2,8 @@ use collections::HashMap; use editor::{scroll::autoscroll::Autoscroll, Bias, Editor}; use fuzzy::{CharBag, PathMatch, PathMatchCandidate}; use gpui::{ - actions, div, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, FocusableView, - InteractiveElement, IntoElement, Model, ParentElement, Render, Styled, Task, View, ViewContext, - VisualContext, WeakView, + actions, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, FocusableView, Model, + ParentElement, Render, Styled, Task, View, ViewContext, VisualContext, WeakView, }; use picker::{Picker, PickerDelegate}; use project::{PathMatchCandidateSet, Project, ProjectPath, WorktreeId}; @@ -16,8 +15,7 @@ use std::{ }, }; use text::Point; -use theme::ActiveTheme; -use ui::{v_stack, HighlightedLabel, StyledExt}; +use ui::{v_stack, HighlightedLabel, ListItem}; use util::{paths::PathLikeWithPosition, post_inc, ResultExt}; use workspace::Workspace; @@ -530,7 +528,7 @@ impl FileFinderDelegate { } impl PickerDelegate for FileFinderDelegate { - type ListItem = Div; + type ListItem = ListItem; fn placeholder_text(&self) -> Arc { "Search project files...".into() @@ -716,26 +714,16 @@ impl PickerDelegate for FileFinderDelegate { .matches .get(ix) .expect("Invalid matches state: no element for index {ix}"); - let theme = cx.theme(); - let colors = theme.colors(); let (file_name, file_name_positions, full_path, full_path_positions) = self.labels_for_match(path_match, cx, ix); Some( - div() - .px_1() - .text_color(colors.text) - .text_ui() - .bg(colors.ghost_element_background) - .rounded_md() - .when(selected, |this| this.bg(colors.ghost_element_selected)) - .hover(|this| this.bg(colors.ghost_element_hover)) - .child( - v_stack() - .child(HighlightedLabel::new(file_name, file_name_positions)) - .child(HighlightedLabel::new(full_path, full_path_positions)), - ), + ListItem::new(ix).selected(selected).child( + v_stack() + .child(HighlightedLabel::new(file_name, file_name_positions)) + .child(HighlightedLabel::new(full_path, full_path_positions)), + ), ) } } diff --git a/crates/ui2/src/components/list.rs b/crates/ui2/src/components/list.rs index 875ab6d97e2032ae8c6be9bb9e0445d99897d099..642903f09b39ddbbe0ca0c71ec9b43b8bd9f93da 100644 --- a/crates/ui2/src/components/list.rs +++ b/crates/ui2/src/components/list.rs @@ -231,23 +231,16 @@ impl RenderOnce for ListSubHeader { } } -#[derive(Default, PartialEq, Copy, Clone)] -pub enum ListEntrySize { - #[default] - Small, - Medium, -} - #[derive(IntoElement)] pub struct ListItem { id: ElementId, disabled: bool, + selected: bool, // TODO: Reintroduce this // disclosure_control_style: DisclosureControlVisibility, indent_level: u32, left_slot: Option, overflow: OverflowStyle, - size: ListEntrySize, toggle: Toggle, variant: ListItemVariant, on_click: Option>, @@ -259,10 +252,10 @@ impl ListItem { Self { id: id.into(), disabled: false, + selected: false, indent_level: 0, left_slot: None, overflow: OverflowStyle::Hidden, - size: ListEntrySize::default(), toggle: Toggle::NotToggleable, variant: ListItemVariant::default(), on_click: Default::default(), @@ -290,6 +283,11 @@ impl ListItem { self } + pub fn selected(mut self, selected: bool) -> Self { + self.selected = selected; + self + } + pub fn left_content(mut self, left_content: GraphicSlot) -> Self { self.left_slot = Some(left_content); self @@ -304,11 +302,6 @@ impl ListItem { self.left_slot = Some(GraphicSlot::Avatar(left_avatar.into())); self } - - pub fn size(mut self, size: ListEntrySize) -> Self { - self.size = size; - self - } } impl RenderOnce for ListItem { @@ -328,10 +321,6 @@ impl RenderOnce for ListItem { None => None, }; - let sized_item = match self.size { - ListEntrySize::Small => div().h_6(), - ListEntrySize::Medium => div().h_7(), - }; div() .id(self.id) .relative() @@ -354,8 +343,11 @@ impl RenderOnce for ListItem { // }) .hover(|style| style.bg(cx.theme().colors().ghost_element_hover)) .active(|style| style.bg(cx.theme().colors().ghost_element_active)) + .when(self.selected, |this| { + this.bg(cx.theme().colors().ghost_element_selected) + }) .child( - sized_item + div() .when(self.variant == ListItemVariant::Inset, |this| this.px_2()) // .ml(rems(0.75 * self.indent_level as f32)) .children((0..self.indent_level).map(|_| { From ecb3bd7f5943d2bd00e9dfd3f64843399f2749d9 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Tue, 28 Nov 2023 10:52:17 -0500 Subject: [PATCH 3/3] Use `ListItem`s in `Picker` story --- crates/storybook2/src/stories/picker.rs | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/crates/storybook2/src/stories/picker.rs b/crates/storybook2/src/stories/picker.rs index f66fe7a93af28ddf5ba5274594fc03597299d915..8bcfb8923dbaef403ec65916821ae2341367f136 100644 --- a/crates/storybook2/src/stories/picker.rs +++ b/crates/storybook2/src/stories/picker.rs @@ -5,6 +5,7 @@ use gpui::{ use picker::{Picker, PickerDelegate}; use std::sync::Arc; use theme2::ActiveTheme; +use ui::{Label, ListItem}; pub struct PickerStory { picker: View>, @@ -36,7 +37,7 @@ impl Delegate { } impl PickerDelegate for Delegate { - type ListItem = Div; + type ListItem = ListItem; fn match_count(&self) -> usize { self.candidates.len() @@ -52,7 +53,6 @@ impl PickerDelegate for Delegate { selected: bool, cx: &mut gpui::ViewContext>, ) -> Option { - let colors = cx.theme().colors(); let Some(candidate_ix) = self.matches.get(ix) else { return None; }; @@ -60,17 +60,9 @@ impl PickerDelegate for Delegate { let candidate = SharedString::from(self.candidates[*candidate_ix].string.clone()); Some( - div() - .text_color(colors.text) - .when(selected, |s| { - s.border_l_10().border_color(colors.terminal_ansi_yellow) - }) - .hover(|style| { - style - .bg(colors.element_active) - .text_color(colors.text_accent) - }) - .child(candidate), + ListItem::new(ix) + .selected(selected) + .child(Label::new(candidate)), ) }