Rework `ListItem` to use `children` (#3387)

Marshall Bowers created

This PR reworks the `ListItem` component to accept `children` rather
than just a `Label`.

This is a step towards making the `ListItem` component more open.

As part of this the `ContextMenu` was simplified to only construct the
various list components in `render` rather than holding them as part of
its state.

Release Notes:

- N/A

Change summary

crates/project_panel2/src/project_panel.rs        |  2 
crates/terminal_view2/src/terminal_view.rs        |  9 +---
crates/ui2/src/components/context_menu.rs         | 33 ++++++++--------
crates/ui2/src/components/list.rs                 | 31 +++++----------
crates/ui2/src/components/stories/context_menu.rs | 20 ++++------
crates/workspace2/src/dock.rs                     |  9 +---
6 files changed, 41 insertions(+), 63 deletions(-)

Detailed changes

crates/project_panel2/src/project_panel.rs 🔗

@@ -371,7 +371,7 @@ impl ProjectPanel {
         _entry_id: ProjectEntryId,
         _cx: &mut ViewContext<Self>,
     ) {
-        todo!()
+        // todo!()
         //     let project = self.project.read(cx);
 
         //     let worktree_id = if let Some(id) = project.worktree_id_for_entry(entry_id, cx) {

crates/terminal_view2/src/terminal_view.rs 🔗

@@ -31,7 +31,7 @@ use workspace::{
     notifications::NotifyResultExt,
     register_deserializable_item,
     searchable::{SearchEvent, SearchOptions, SearchableItem},
-    ui::{ContextMenu, Icon, IconElement, Label, ListItem},
+    ui::{ContextMenu, Icon, IconElement, Label},
     CloseActiveItem, NewCenterTerminal, Pane, ToolbarItemLocation, Workspace, WorkspaceId,
 };
 
@@ -299,11 +299,8 @@ impl TerminalView {
         cx: &mut ViewContext<Self>,
     ) {
         self.context_menu = Some(ContextMenu::build(cx, |menu, _| {
-            menu.action(ListItem::new("clear", Label::new("Clear")), Box::new(Clear))
-                .action(
-                    ListItem::new("close", Label::new("Close")),
-                    Box::new(CloseActiveItem { save_intent: None }),
-                )
+            menu.action("Clear", Box::new(Clear))
+                .action("Close", Box::new(CloseActiveItem { save_intent: None }))
         }));
         dbg!(&position);
         // todo!()

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

@@ -1,7 +1,7 @@
 use std::cell::RefCell;
 use std::rc::Rc;
 
-use crate::{prelude::*, v_stack, List};
+use crate::{prelude::*, v_stack, Label, List};
 use crate::{ListItem, ListSeparator, ListSubHeader};
 use gpui::{
     overlay, px, Action, AnchorCorner, AnyElement, AppContext, Bounds, ClickEvent, DispatchPhase,
@@ -10,9 +10,9 @@ use gpui::{
 };
 
 pub enum ContextMenuItem {
-    Separator(ListSeparator),
-    Header(ListSubHeader),
-    Entry(ListItem, Rc<dyn Fn(&ClickEvent, &mut WindowContext)>),
+    Separator,
+    Header(SharedString),
+    Entry(SharedString, Rc<dyn Fn(&ClickEvent, &mut WindowContext)>),
 }
 
 pub struct ContextMenu {
@@ -46,29 +46,30 @@ impl ContextMenu {
     }
 
     pub fn header(mut self, title: impl Into<SharedString>) -> Self {
-        self.items
-            .push(ContextMenuItem::Header(ListSubHeader::new(title)));
+        self.items.push(ContextMenuItem::Header(title.into()));
         self
     }
 
     pub fn separator(mut self) -> Self {
-        self.items.push(ContextMenuItem::Separator(ListSeparator));
+        self.items.push(ContextMenuItem::Separator);
         self
     }
 
     pub fn entry(
         mut self,
-        view: ListItem,
+        label: impl Into<SharedString>,
         on_click: impl Fn(&ClickEvent, &mut WindowContext) + 'static,
     ) -> Self {
         self.items
-            .push(ContextMenuItem::Entry(view, Rc::new(on_click)));
+            .push(ContextMenuItem::Entry(label.into(), Rc::new(on_click)));
         self
     }
 
-    pub fn action(self, view: ListItem, action: Box<dyn Action>) -> Self {
+    pub fn action(self, label: impl Into<SharedString>, action: Box<dyn Action>) -> Self {
         // todo: add the keybindings to the list entry
-        self.entry(view, move |_, cx| cx.dispatch_action(action.boxed_clone()))
+        self.entry(label.into(), move |_, cx| {
+            cx.dispatch_action(action.boxed_clone())
+        })
     }
 
     pub fn confirm(&mut self, _: &menu::Confirm, cx: &mut ViewContext<Self>) {
@@ -104,16 +105,16 @@ impl Render for ContextMenu {
                 // .border_color(cx.theme().colors().border)
                 .child(
                     List::new().children(self.items.iter().map(|item| match item {
-                        ContextMenuItem::Separator(separator) => {
-                            separator.clone().render_into_any()
+                        ContextMenuItem::Separator => ListSeparator::new().render_into_any(),
+                        ContextMenuItem::Header(header) => {
+                            ListSubHeader::new(header.clone()).render_into_any()
                         }
-                        ContextMenuItem::Header(header) => header.clone().render_into_any(),
                         ContextMenuItem::Entry(entry, callback) => {
                             let callback = callback.clone();
                             let dismiss = cx.listener(|_, _, cx| cx.emit(Manager::Dismiss));
 
-                            entry
-                                .clone()
+                            ListItem::new(entry.clone())
+                                .child(Label::new(entry.clone()))
                                 .on_click(move |event, cx| {
                                     callback(event, cx);
                                     dismiss(event, cx)

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

@@ -245,45 +245,28 @@ pub struct ListItem {
     // TODO: Reintroduce this
     // disclosure_control_style: DisclosureControlVisibility,
     indent_level: u32,
-    label: Label,
     left_slot: Option<GraphicSlot>,
     overflow: OverflowStyle,
     size: ListEntrySize,
     toggle: Toggle,
     variant: ListItemVariant,
     on_click: Option<Rc<dyn Fn(&ClickEvent, &mut WindowContext) + 'static>>,
-}
-
-impl Clone for ListItem {
-    fn clone(&self) -> Self {
-        Self {
-            id: self.id.clone(),
-            disabled: self.disabled,
-            indent_level: self.indent_level,
-            label: self.label.clone(),
-            left_slot: self.left_slot.clone(),
-            overflow: self.overflow,
-            size: self.size,
-            toggle: self.toggle,
-            variant: self.variant,
-            on_click: self.on_click.clone(),
-        }
-    }
+    children: SmallVec<[AnyElement; 2]>,
 }
 
 impl ListItem {
-    pub fn new(id: impl Into<ElementId>, label: Label) -> Self {
+    pub fn new(id: impl Into<ElementId>) -> Self {
         Self {
             id: id.into(),
             disabled: false,
             indent_level: 0,
-            label,
             left_slot: None,
             overflow: OverflowStyle::Hidden,
             size: ListEntrySize::default(),
             toggle: Toggle::NotToggleable,
             variant: ListItemVariant::default(),
             on_click: Default::default(),
+            children: SmallVec::new(),
         }
     }
 
@@ -394,11 +377,17 @@ impl Component for ListItem {
                     .relative()
                     .child(disclosure_control(self.toggle))
                     .children(left_content)
-                    .child(self.label),
+                    .children(self.children),
             )
     }
 }
 
+impl ParentElement for ListItem {
+    fn children_mut(&mut self) -> &mut SmallVec<[AnyElement; 2]> {
+        &mut self.children
+    }
+}
+
 #[derive(RenderOnce, Clone)]
 pub struct ListSeparator;
 

crates/ui2/src/components/stories/context_menu.rs 🔗

@@ -2,7 +2,7 @@ use gpui::{actions, Action, AnchorCorner, Div, Render, View};
 use story::Story;
 
 use crate::prelude::*;
-use crate::{menu_handle, ContextMenu, Label, ListItem};
+use crate::{menu_handle, ContextMenu, Label};
 
 actions!(PrintCurrentDate, PrintBestFood);
 
@@ -10,17 +10,13 @@ fn build_menu(cx: &mut WindowContext, header: impl Into<SharedString>) -> View<C
     ContextMenu::build(cx, |menu, _| {
         menu.header(header)
             .separator()
-            .entry(
-                ListItem::new("Print current time", Label::new("Print current time")),
-                |v, cx| {
-                    println!("dispatching PrintCurrentTime action");
-                    cx.dispatch_action(PrintCurrentDate.boxed_clone())
-                },
-            )
-            .entry(
-                ListItem::new("Print best food", Label::new("Print best food")),
-                |v, cx| cx.dispatch_action(PrintBestFood.boxed_clone()),
-            )
+            .entry("Print current time", |v, cx| {
+                println!("dispatching PrintCurrentTime action");
+                cx.dispatch_action(PrintCurrentDate.boxed_clone())
+            })
+            .entry("Print best foot", |v, cx| {
+                cx.dispatch_action(PrintBestFood.boxed_clone())
+            })
     })
 }
 

crates/workspace2/src/dock.rs 🔗

@@ -8,9 +8,7 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::sync::Arc;
 use theme2::ActiveTheme;
-use ui::{
-    h_stack, menu_handle, ContextMenu, IconButton, InteractionState, Label, ListItem, Tooltip,
-};
+use ui::{h_stack, menu_handle, ContextMenu, IconButton, InteractionState, Tooltip};
 
 pub enum PanelEvent {
     ChangePosition,
@@ -720,10 +718,7 @@ impl Render for PanelButtons {
                                     {
                                         let panel = panel.clone();
                                         menu = menu.entry(
-                                            ListItem::new(
-                                                panel.entity_id(),
-                                                Label::new(format!("Dock {}", position.to_label())),
-                                            ),
+                                            format!("Dock {}", position.to_label()),
                                             move |_, cx| {
                                                 panel.set_position(position, cx);
                                             },