Add inset variant to `ListItem` (#3422)

Marshall Bowers created

This PR adds an inset variant to the `ListItem` component.

We're now using this inset variant for the `ListItem`s we render in
pickers.

Release Notes:

- N/A

Change summary

crates/command_palette2/src/command_palette.rs |   2 
crates/file_finder2/src/file_finder.rs         |   2 
crates/storybook2/src/stories/picker.rs        |   1 
crates/ui2/src/components/list.rs              | 104 +++++++++----------
4 files changed, 54 insertions(+), 55 deletions(-)

Detailed changes

crates/command_palette2/src/command_palette.rs 🔗

@@ -301,7 +301,7 @@ impl PickerDelegate for CommandPaletteDelegate {
         };
 
         Some(
-            ListItem::new(ix).selected(selected).child(
+            ListItem::new(ix).inset(true).selected(selected).child(
                 h_stack()
                     .justify_between()
                     .child(HighlightedLabel::new(

crates/file_finder2/src/file_finder.rs 🔗

@@ -719,7 +719,7 @@ impl PickerDelegate for FileFinderDelegate {
             self.labels_for_match(path_match, cx, ix);
 
         Some(
-            ListItem::new(ix).selected(selected).child(
+            ListItem::new(ix).inset(true).selected(selected).child(
                 v_stack()
                     .child(HighlightedLabel::new(file_name, file_name_positions))
                     .child(HighlightedLabel::new(full_path, full_path_positions)),

crates/ui2/src/components/list.rs 🔗

@@ -12,14 +12,6 @@ use crate::{
 };
 use crate::{prelude::*, GraphicSlot};
 
-#[derive(Clone, Copy, Default, Debug, PartialEq)]
-pub enum ListItemVariant {
-    /// The list item extends to the far left and right of the list.
-    FullWidth,
-    #[default]
-    Inset,
-}
-
 pub enum ListHeaderMeta {
     Tools(Vec<IconButton>),
     // TODO: This should be a button
@@ -32,8 +24,39 @@ pub struct ListHeader {
     label: SharedString,
     left_icon: Option<Icon>,
     meta: Option<ListHeaderMeta>,
-    variant: ListItemVariant,
     toggle: Toggle,
+    inset: bool,
+}
+
+impl ListHeader {
+    pub fn new(label: impl Into<SharedString>) -> Self {
+        Self {
+            label: label.into(),
+            left_icon: None,
+            meta: None,
+            inset: false,
+            toggle: Toggle::NotToggleable,
+        }
+    }
+
+    pub fn toggle(mut self, toggle: Toggle) -> Self {
+        self.toggle = toggle;
+        self
+    }
+
+    pub fn left_icon(mut self, left_icon: Option<Icon>) -> Self {
+        self.left_icon = left_icon;
+        self
+    }
+
+    pub fn right_button(self, button: IconButton) -> Self {
+        self.meta(Some(ListHeaderMeta::Tools(vec![button])))
+    }
+
+    pub fn meta(mut self, meta: Option<ListHeaderMeta>) -> Self {
+        self.meta = meta;
+        self
+    }
 }
 
 impl RenderOnce for ListHeader {
@@ -61,7 +84,7 @@ impl RenderOnce for ListHeader {
             .child(
                 div()
                     .h_5()
-                    .when(self.variant == ListItemVariant::Inset, |this| this.px_2())
+                    .when(self.inset, |this| this.px_2())
                     .flex()
                     .flex_1()
                     .items_center()
@@ -90,42 +113,11 @@ impl RenderOnce for ListHeader {
     }
 }
 
-impl ListHeader {
-    pub fn new(label: impl Into<SharedString>) -> Self {
-        Self {
-            label: label.into(),
-            left_icon: None,
-            meta: None,
-            variant: ListItemVariant::default(),
-            toggle: Toggle::NotToggleable,
-        }
-    }
-
-    pub fn toggle(mut self, toggle: Toggle) -> Self {
-        self.toggle = toggle;
-        self
-    }
-
-    pub fn left_icon(mut self, left_icon: Option<Icon>) -> Self {
-        self.left_icon = left_icon;
-        self
-    }
-
-    pub fn right_button(self, button: IconButton) -> Self {
-        self.meta(Some(ListHeaderMeta::Tools(vec![button])))
-    }
-
-    pub fn meta(mut self, meta: Option<ListHeaderMeta>) -> Self {
-        self.meta = meta;
-        self
-    }
-}
-
 #[derive(IntoElement, Clone)]
 pub struct ListSubHeader {
     label: SharedString,
     left_icon: Option<Icon>,
-    variant: ListItemVariant,
+    inset: bool,
 }
 
 impl ListSubHeader {
@@ -133,7 +125,7 @@ impl ListSubHeader {
         Self {
             label: label.into(),
             left_icon: None,
-            variant: ListItemVariant::default(),
+            inset: false,
         }
     }
 
@@ -150,7 +142,7 @@ impl RenderOnce for ListSubHeader {
         h_stack().flex_1().w_full().relative().py_1().child(
             div()
                 .h_6()
-                .when(self.variant == ListItemVariant::Inset, |this| this.px_2())
+                .when(self.inset, |this| this.px_2())
                 .flex()
                 .flex_1()
                 .w_full()
@@ -185,7 +177,7 @@ pub struct ListItem {
     left_slot: Option<GraphicSlot>,
     overflow: OverflowStyle,
     toggle: Toggle,
-    variant: ListItemVariant,
+    inset: bool,
     on_click: Option<Rc<dyn Fn(&ClickEvent, &mut WindowContext) + 'static>>,
     on_secondary_mouse_down: Option<Rc<dyn Fn(&MouseDownEvent, &mut WindowContext) + 'static>>,
     children: SmallVec<[AnyElement; 2]>,
@@ -202,7 +194,7 @@ impl ListItem {
             left_slot: None,
             overflow: OverflowStyle::Hidden,
             toggle: Toggle::NotToggleable,
-            variant: ListItemVariant::default(),
+            inset: false,
             on_click: None,
             on_secondary_mouse_down: None,
             children: SmallVec::new(),
@@ -222,8 +214,8 @@ impl ListItem {
         self
     }
 
-    pub fn variant(mut self, variant: ListItemVariant) -> Self {
-        self.variant = variant;
+    pub fn inset(mut self, inset: bool) -> Self {
+        self.inset = inset;
         self
     }
 
@@ -283,20 +275,26 @@ impl RenderOnce for ListItem {
         div()
             .id(self.id)
             .relative()
-            .hover(|mut style| {
-                style.background = Some(cx.theme().colors().editor_background.into());
-                style
-            })
             // TODO: Add focus state
             // .when(self.state == InteractionState::Focused, |this| {
             //     this.border()
             //         .border_color(cx.theme().colors().border_focused)
             // })
+            .when(self.inset, |this| this.rounded_md())
             .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)
             })
+            .when_some(self.on_click.clone(), |this, on_click| {
+                this.on_click(move |event, cx| {
+                    // HACK: GPUI currently fires `on_click` with any mouse button,
+                    // but we only care about the left button.
+                    if event.down.button == MouseButton::Left {
+                        (on_click)(event, cx)
+                    }
+                })
+            })
             .when_some(self.on_secondary_mouse_down, |this, on_mouse_down| {
                 this.on_mouse_down(MouseButton::Right, move |event, cx| {
                     (on_mouse_down)(event, cx)
@@ -304,7 +302,7 @@ impl RenderOnce for ListItem {
             })
             .child(
                 div()
-                    .when(self.variant == ListItemVariant::Inset, |this| this.px_2())
+                    .when(self.inset, |this| this.px_2())
                     .ml(self.indent_level as f32 * self.indent_step_size)
                     .flex()
                     .gap_1()