Render `Picker` matches using `ListItem`s (#3418)

Marshall Bowers created

This PR updates the `PickerDelegate` implementations to render their
matches using the `ListItem` component so that they can have a
consistent style.

At some point it might make sense to move the `ListItem` rendering up
into the `Picker` implementation itself, and just have the delegate
responsible for giving us the inner content of the `ListItem`.

Release Notes:

- N/A

Change summary

crates/command_palette2/src/command_palette.rs | 31 +++++++------------
crates/file_finder2/src/file_finder.rs         | 28 +++++------------
crates/picker2/src/picker2.rs                  |  4 +-
crates/storybook2/src/stories/picker.rs        | 24 +++++---------
crates/ui2/src/components/list.rs              | 30 +++++++------------
5 files changed, 42 insertions(+), 75 deletions(-)

Detailed changes

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<str> {
         "Execute a command...".into()
@@ -294,24 +292,16 @@ impl PickerDelegate for CommandPaletteDelegate {
         ix: usize,
         selected: bool,
         cx: &mut ViewContext<Picker<Self>>,
-    ) -> Self::ListItem {
-        let colors = cx.theme().colors();
+    ) -> Option<Self::ListItem> {
         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(
+        Some(
+            ListItem::new(ix).selected(selected).child(
                 h_stack()
                     .justify_between()
                     .child(HighlightedLabel::new(
@@ -319,7 +309,8 @@ impl PickerDelegate for CommandPaletteDelegate {
                         r#match.positions.clone(),
                     ))
                     .children(KeyBinding::for_action(&*command.action, cx)),
-            )
+            ),
+        )
     }
 }
 

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<str> {
         "Search project files...".into()
@@ -711,30 +709,22 @@ impl PickerDelegate for FileFinderDelegate {
         ix: usize,
         selected: bool,
         cx: &mut ViewContext<Picker<Self>>,
-    ) -> Self::ListItem {
+    ) -> Option<Self::ListItem> {
         let path_match = self
             .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);
 
-        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(
+        Some(
+            ListItem::new(ix).selected(selected).child(
                 v_stack()
                     .child(HighlightedLabel::new(file_name, file_name_positions))
                     .child(HighlightedLabel::new(full_path, full_path_positions)),
-            )
+            ),
+        )
     }
 }
 

crates/picker2/src/picker2.rs 🔗

@@ -32,7 +32,7 @@ pub trait PickerDelegate: Sized + 'static {
         ix: usize,
         selected: bool,
         cx: &mut ViewContext<Picker<Self>>,
-    ) -> Self::ListItem;
+    ) -> Option<Self::ListItem>;
 }
 
 impl<D: PickerDelegate> FocusableView for Picker<D> {
@@ -230,7 +230,7 @@ impl<D: PickerDelegate> Render for Picker<D> {
                                                             )
                                                         }),
                                                     )
-                                                    .child(picker.delegate.render_match(
+                                                    .children(picker.delegate.render_match(
                                                         ix,
                                                         ix == selected_index,
                                                         cx,

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<Picker<Delegate>>,
@@ -36,7 +37,7 @@ impl Delegate {
 }
 
 impl PickerDelegate for Delegate {
-    type ListItem = Div;
+    type ListItem = ListItem;
 
     fn match_count(&self) -> usize {
         self.candidates.len()
@@ -51,25 +52,18 @@ impl PickerDelegate for Delegate {
         ix: usize,
         selected: bool,
         cx: &mut gpui::ViewContext<Picker<Self>>,
-    ) -> Self::ListItem {
-        let colors = cx.theme().colors();
+    ) -> Option<Self::ListItem> {
         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(
+            ListItem::new(ix)
+                .selected(selected)
+                .child(Label::new(candidate)),
+        )
     }
 
     fn selected_index(&self) -> usize {

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<GraphicSlot>,
     overflow: OverflowStyle,
-    size: ListEntrySize,
     toggle: Toggle,
     variant: ListItemVariant,
     on_click: Option<Rc<dyn Fn(&ClickEvent, &mut WindowContext) + 'static>>,
@@ -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(|_| {