From 38a10fe05c7bbbf6016d0e2ec2a830a7bfb96b3a Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Tue, 28 Oct 2025 14:42:56 +0800 Subject: [PATCH] menu: Add `item` and export `PopupMenuItem` with builder methods to PopupMenu. (#1445) Now we can be easy to add a menu item without define `action`, just use `on_click` callback. ## Break Changes - Removed complex methods: `menu_element_with_check_and_disabled`, `menu_element_with_icon_and_disabled`, you can use `item` and `PopupMenuItem` instead. --- crates/story/src/menu_story.rs | 42 +++- crates/ui/src/menu/popup_menu.rs | 330 +++++++++++++++++++---------- docs/docs/components/popup_menu.md | 88 +++----- 3 files changed, 283 insertions(+), 177 deletions(-) diff --git a/crates/story/src/menu_story.rs b/crates/story/src/menu_story.rs index 15b0d5c8..634a1b3c 100644 --- a/crates/story/src/menu_story.rs +++ b/crates/story/src/menu_story.rs @@ -3,8 +3,12 @@ use gpui::{ ParentElement as _, Render, SharedString, Styled as _, Window, actions, div, px, }; use gpui_component::{ - ActiveTheme as _, IconName, button::Button, context_menu::ContextMenuExt, h_flex, - popup_menu::PopupMenuExt as _, v_flex, + ActiveTheme as _, IconName, + button::Button, + context_menu::ContextMenuExt, + h_flex, + popup_menu::{PopupMenuExt as _, PopupMenuItem}, + v_flex, }; use serde::Deserialize; @@ -104,6 +108,7 @@ impl MenuStory { impl Render for MenuStory { fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { let checked = self.checked; + let view = cx.entity(); v_flex() .key_context(CONTEXT) @@ -124,6 +129,14 @@ impl Render for MenuStory { .label("Edit") .popup_menu(move |this, window, cx| { this.link("About", "https://github.com/longbridge/gpui-component") + .separator() + .item(PopupMenuItem::new("Handle Click").on_click( + window.listener_for(&view, |this, _, _, cx| { + this.message = + "You have clicked Handle Click".to_string(); + cx.notify(); + }), + )) .separator() .menu("Copy", Box::new(Copy)) .menu("Cut", Box::new(Cut)) @@ -133,14 +146,23 @@ impl Render for MenuStory { .separator() .menu_with_icon("Search", IconName::Search, Box::new(SearchAll)) .separator() - .menu_element(Box::new(Info(0)), |_, cx| { - v_flex().child("Custom Element").child( - div() - .text_xs() - .text_color(cx.theme().muted_foreground) - .child("THis is sub-title"), - ) - }) + .item( + PopupMenuItem::element(|_, cx| { + v_flex().child("Custom Element").child( + div() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child("This is sub-title"), + ) + }) + .on_click( + window.listener_for(&view, |this, _, _, cx| { + this.message = "You have clicked on custom element" + .to_string(); + cx.notify(); + }), + ), + ) .menu_element_with_check(checked, Box::new(Info(0)), |_, cx| { h_flex().gap_1().child("Custom Element").child( div() diff --git a/crates/ui/src/menu/popup_menu.rs b/crates/ui/src/menu/popup_menu.rs index 9c89b9ea..bb9afc87 100644 --- a/crates/ui/src/menu/popup_menu.rs +++ b/crates/ui/src/menu/popup_menu.rs @@ -13,7 +13,7 @@ use gpui::{ InteractiveElement, IntoElement, KeyBinding, ParentElement, Pixels, Render, ScrollHandle, SharedString, StatefulInteractiveElement, Styled, WeakEntity, Window, }; -use gpui::{Half, MouseDownEvent, OwnedMenuItem, Subscription}; +use gpui::{ClickEvent, Half, MouseDownEvent, OwnedMenuItem, Subscription}; use std::rc::Rc; const CONTEXT: &str = "PopupMenu"; @@ -59,9 +59,12 @@ pub trait PopupMenuExt: Styled + Selectable + InteractiveElement + IntoElement + } impl PopupMenuExt for Button {} -pub(crate) enum PopupMenuItem { +/// An menu item in a popup menu. +pub enum PopupMenuItem { Separator, + /// A non-interactive label item. Label(SharedString), + /// A standard menu item. Item { icon: Option, label: SharedString, @@ -69,15 +72,19 @@ pub(crate) enum PopupMenuItem { is_link: bool, action: Option>, // For link item - handler: Option>, + handler: Option>, }, + /// A menu item with custom element render. ElementItem { icon: Option, disabled: bool, - action: Box, + action: Option>, render: Box AnyElement + 'static>, - handler: Option>, + handler: Option>, }, + /// A submenu item that opens another popup menu. + /// + /// NOTE: This is only supported when the parent menu is not `scrollable`. Submenu { icon: Option, label: SharedString, @@ -86,7 +93,147 @@ pub(crate) enum PopupMenuItem { }, } +impl FluentBuilder for PopupMenuItem {} impl PopupMenuItem { + /// Create a new menu item with the given label. + #[inline] + pub fn new(label: impl Into) -> Self { + PopupMenuItem::Item { + icon: None, + label: label.into(), + disabled: false, + action: None, + is_link: false, + handler: None, + } + } + + /// Create a new menu item with custom element render. + #[inline] + pub fn element(builder: F) -> Self + where + F: Fn(&mut Window, &mut App) -> E + 'static, + E: IntoElement, + { + PopupMenuItem::ElementItem { + icon: None, + disabled: false, + action: None, + render: Box::new(move |window, cx| builder(window, cx).into_any_element()), + handler: None, + } + } + + /// Create a new submenu item that opens another popup menu. + #[inline] + pub fn submenu(label: impl Into, menu: Entity) -> Self { + PopupMenuItem::Submenu { + icon: None, + label: label.into(), + disabled: false, + menu, + } + } + + /// Create a separator menu item. + #[inline] + pub fn separator() -> Self { + PopupMenuItem::Separator + } + + /// Creates a label menu item. + #[inline] + pub fn label(label: impl Into) -> Self { + PopupMenuItem::Label(label.into()) + } + + /// Set the icon for the menu item. + /// + /// Only works for [`PopupMenuItem::Item`], [`PopupMenuItem::ElementItem`] and [`PopupMenuItem::Submenu`]. + pub fn icon(mut self, icon: impl Into) -> Self { + match &mut self { + PopupMenuItem::Item { icon: i, .. } => { + *i = Some(icon.into()); + } + PopupMenuItem::ElementItem { icon: i, .. } => { + *i = Some(icon.into()); + } + PopupMenuItem::Submenu { icon: i, .. } => { + *i = Some(icon.into()); + } + _ => {} + } + self + } + + /// Set the action for the menu item. + /// + /// Only works for [`PopupMenuItem::Item`] and [`PopupMenuItem::ElementItem`]. + pub fn action(mut self, action: Box) -> Self { + match &mut self { + PopupMenuItem::Item { action: a, .. } => { + *a = Some(action); + } + PopupMenuItem::ElementItem { action: a, .. } => { + *a = Some(action); + } + _ => {} + } + self + } + + /// Set the disabled state for the menu item. + /// + /// Only works for [`PopupMenuItem::Item`], [`PopupMenuItem::ElementItem`] and [`PopupMenuItem::Submenu`]. + pub fn disabled(mut self, disabled: bool) -> Self { + match &mut self { + PopupMenuItem::Item { disabled: d, .. } => { + *d = disabled; + } + PopupMenuItem::ElementItem { disabled: d, .. } => { + *d = disabled; + } + PopupMenuItem::Submenu { disabled: d, .. } => { + *d = disabled; + } + _ => {} + } + self + } + + /// Add a click handler for the menu item. + /// + /// Only works for [`PopupMenuItem::Item`] and [`PopupMenuItem::ElementItem`]. + pub fn on_click(mut self, handler: F) -> Self + where + F: Fn(&ClickEvent, &mut Window, &mut App) + 'static, + { + match &mut self { + PopupMenuItem::Item { handler: h, .. } => { + *h = Some(Rc::new(handler)); + } + PopupMenuItem::ElementItem { handler: h, .. } => { + *h = Some(Rc::new(handler)); + } + _ => {} + } + self + } + + /// Create a link menu item. + #[inline] + pub fn link(label: impl Into, href: impl Into) -> Self { + let href = href.into(); + PopupMenuItem::Item { + icon: None, + label: label.into(), + disabled: false, + action: None, + is_link: true, + handler: Some(Rc::new(move |_, _, cx| cx.open_url(&href))), + } + } + #[inline] fn is_clickable(&self) -> bool { !matches!(self, PopupMenuItem::Separator) @@ -238,7 +385,7 @@ impl PopupMenu { /// Add label pub fn label(mut self, label: impl Into) -> Self { - self.menu_items.push(PopupMenuItem::Label(label.into())); + self.menu_items.push(PopupMenuItem::label(label.into())); self } @@ -255,14 +402,8 @@ impl PopupMenu { disabled: bool, ) -> Self { let href = href.into(); - self.menu_items.push(PopupMenuItem::Item { - icon: None, - label: label.into(), - disabled, - action: None, - is_link: true, - handler: Some(Rc::new(move |_, cx| cx.open_url(&href))), - }); + self.menu_items + .push(PopupMenuItem::link(label, href).disabled(disabled)); self } @@ -277,7 +418,7 @@ impl PopupMenu { } /// Add Menu to open link with icon and disabled state - pub fn link_with_icon_and_disabled( + fn link_with_icon_and_disabled( mut self, label: impl Into, icon: impl Into, @@ -285,14 +426,11 @@ impl PopupMenu { disabled: bool, ) -> Self { let href = href.into(); - self.menu_items.push(PopupMenuItem::Item { - icon: Some(icon.into()), - label: label.into(), - disabled, - action: None, - is_link: true, - handler: Some(Rc::new(move |_, cx| cx.open_url(&href))), - }); + self.menu_items.push( + PopupMenuItem::link(label, href) + .icon(icon) + .disabled(disabled), + ); self } @@ -382,29 +520,6 @@ impl PopupMenu { self.menu_element_with_icon_and_disabled(icon, action, false, builder) } - /// Add Menu Item with custom element render with icon and disabled state - pub fn menu_element_with_icon_and_disabled( - mut self, - icon: impl Into, - action: Box, - disabled: bool, - builder: F, - ) -> Self - where - F: Fn(&mut Window, &mut App) -> E + 'static, - E: IntoElement, - { - self.menu_items.push(PopupMenuItem::ElementItem { - render: Box::new(move |window, cx| builder(window, cx).into_any_element()), - action, - icon: Some(icon.into()), - disabled, - handler: None, - }); - self.has_icon = true; - self - } - /// Add Menu Item with custom element render with check state pub fn menu_element_with_check( self, @@ -419,8 +534,30 @@ impl PopupMenu { self.menu_element_with_check_and_disabled(checked, action, false, builder) } + /// Add Menu Item with custom element render with icon and disabled state + fn menu_element_with_icon_and_disabled( + mut self, + icon: impl Into, + action: Box, + disabled: bool, + builder: F, + ) -> Self + where + F: Fn(&mut Window, &mut App) -> E + 'static, + E: IntoElement, + { + self.menu_items.push( + PopupMenuItem::element(builder) + .action(action) + .icon(icon) + .disabled(disabled), + ); + self.has_icon = true; + self + } + /// Add Menu Item with custom element render with check state and disabled state - pub fn menu_element_with_check_and_disabled( + fn menu_element_with_check_and_disabled( mut self, checked: bool, action: Box, @@ -431,30 +568,13 @@ impl PopupMenu { F: Fn(&mut Window, &mut App) -> E + 'static, E: IntoElement, { - if checked { - self.menu_items.push(PopupMenuItem::ElementItem { - render: Box::new(move |window, cx| builder(window, cx).into_any_element()), - action, - handler: None, - icon: Some(IconName::Check.into()), - disabled, - }); - self.has_icon = true; - } else { - self.menu_items.push(PopupMenuItem::ElementItem { - render: Box::new(move |window, cx| builder(window, cx).into_any_element()), - action, - handler: None, - icon: None, - disabled, - }); - } - self - } - - /// Use small size, the menu item will have smaller height. - pub(crate) fn small(mut self) -> Self { - self.size = Size::Small; + self.menu_items.push( + PopupMenuItem::element(builder) + .action(action) + .when(checked, |item| item.icon(IconName::Check)) + .disabled(disabled), + ); + self.has_icon = self.has_icon || checked; self } @@ -468,7 +588,7 @@ impl PopupMenu { return self; } - self.menu_items.push(PopupMenuItem::Separator); + self.menu_items.push(PopupMenuItem::separator()); self } @@ -483,36 +603,11 @@ impl PopupMenu { self.submenu_with_icon(None, label, window, cx, f) } - /// Add a Submenu item with disabled state - pub fn submenu_with_disabled( - self, - label: impl Into, - disabled: bool, - window: &mut Window, - cx: &mut Context, - f: impl Fn(PopupMenu, &mut Window, &mut Context) -> PopupMenu + 'static, - ) -> Self { - self.submenu_with_icon_with_disabled(None, label, disabled, window, cx, f) - } - /// Add a Submenu item with icon pub fn submenu_with_icon( - self, - icon: Option, - label: impl Into, - window: &mut Window, - cx: &mut Context, - f: impl Fn(PopupMenu, &mut Window, &mut Context) -> PopupMenu + 'static, - ) -> Self { - self.submenu_with_icon_with_disabled(icon, label, false, window, cx, f) - } - - /// Add a Submenu item with icon and disabled state - pub fn submenu_with_icon_with_disabled( mut self, icon: Option, label: impl Into, - disabled: bool, window: &mut Window, cx: &mut Context, f: impl Fn(PopupMenu, &mut Window, &mut Context) -> PopupMenu + 'static, @@ -523,12 +618,21 @@ impl PopupMenu { view.parent_menu = Some(parent_menu); }); - self.menu_items.push(PopupMenuItem::Submenu { - icon, - label: label.into(), - menu: submenu, - disabled, - }); + self.menu_items.push( + PopupMenuItem::submenu(label, submenu).when_some(icon, |this, icon| this.icon(icon)), + ); + self + } + + /// Add menu item. + pub fn item(mut self, item: impl Into) -> Self { + self.menu_items.push(item.into()); + self + } + + /// Use small size, the menu item will have smaller height. + pub(crate) fn small(mut self) -> Self { + self.size = Size::Small; self } @@ -543,14 +647,12 @@ impl PopupMenu { self.has_icon = true; } - self.menu_items.push(PopupMenuItem::Item { - icon, - label: label.into(), - disabled, - action: Some(action.boxed_clone()), - is_link: false, - handler: None, - }); + self.menu_items.push( + PopupMenuItem::new(label) + .when_some(icon, |item, icon| item.icon(icon)) + .disabled(disabled) + .action(action), + ); self } @@ -627,7 +729,7 @@ impl PopupMenu { handler, action, .. }) => { if let Some(handler) = handler { - handler(window, cx); + handler(&ClickEvent::default(), window, cx); } else if let Some(action) = action.as_ref() { self.dispatch_confirm_action(action, window, cx); } @@ -638,8 +740,8 @@ impl PopupMenu { handler, action, .. }) => { if let Some(handler) = handler { - handler(window, cx); - } else { + handler(&ClickEvent::default(), window, cx); + } else if let Some(action) = action.as_ref() { self.dispatch_confirm_action(action, window, cx); } self.dismiss(&Cancel, window, cx) diff --git a/docs/docs/components/popup_menu.md b/docs/docs/components/popup_menu.md index 012954f0..6a9f32b8 100644 --- a/docs/docs/components/popup_menu.md +++ b/docs/docs/components/popup_menu.md @@ -244,6 +244,10 @@ menu.menu_element(Box::new(CustomAction), |window, cx| { ### Scrollable Menus +:::warning +When you have enabled `scrollable()` on a menu, avoid using submenus within it, as this can lead to usability issues. +::: + For menus with many items, enable scrolling: ```rust @@ -290,59 +294,9 @@ menu.action_context(focus_handle) ## API Reference -### PopupMenu - -| Method | Description | -| -------------------------- | --------------------------------------------- | -| `build(window, cx, f)` | Create a new popup menu with builder function | -| `action_context(handle)` | Set focus handle for action dispatch | -| `min_w(width)` | Set minimum width | -| `max_w(width)` | Set maximum width | -| `max_h(height)` | Set maximum height | -| `scrollable()` | Enable vertical scrolling | -| `external_link_icon(bool)` | Show/hide external link icons | - -### Menu Items - -| Method | Description | -| --------------------------------------------- | --------------------------------- | -| `menu(label, action)` | Add basic menu item | -| `menu_with_disabled(label, action, disabled)` | Add menu item with disabled state | -| `menu_with_icon(label, icon, action)` | Add menu item with icon | -| `menu_with_check(label, checked, action)` | Add checkable menu item | -| `label(text)` | Add non-interactive label | -| `separator()` | Add visual separator | -| `link(label, url)` | Add link menu item | -| `link_with_icon(label, icon, url)` | Add link with icon | - -### Custom Elements - -| Method | Description | -| --------------------------------------------------- | ----------------------------------- | -| `menu_element(action, builder)` | Add custom element menu item | -| `menu_element_with_icon(icon, action, builder)` | Add custom element with icon | -| `menu_element_with_check(checked, action, builder)` | Add custom element with check state | - -### Submenus - -| Method | Description | -| ------------------------------------------------------------- | ------------------------------- | -| `submenu(label, window, cx, builder)` | Add submenu | -| `submenu_with_icon(icon, label, window, cx, builder)` | Add submenu with icon | -| `submenu_with_disabled(label, disabled, window, cx, builder)` | Add submenu with disabled state | - -### PopupMenuExt Trait - -| Method | Description | -| ----------------------------------------- | ------------------------------------------ | -| `popup_menu(builder)` | Add popup menu to button (top-left anchor) | -| `popup_menu_with_anchor(corner, builder)` | Add popup menu with custom anchor | - -### ContextMenuExt Trait - -| Method | Description | -| ----------------------- | --------------------------- | -| `context_menu(builder)` | Add context menu to element | +- [PopupMenu] +- [context_menu] +- [PopupMenuItem] ## Examples @@ -369,6 +323,30 @@ div() }) ``` +### Add MenuItem without action + +Sometimes you may not like to define an action for a menu item, you just want add a `on_click` handler, in this case, the `item` and [PopupMenuItem] can help you: + +```rust +use gpui_component::{popup_menu::PopupMenuItem, Button}; + +Button::new("custom-item-menu") + .label("Options") + .popup_menu(|menu, window, cx| { + menu.item( + PopupMenuItem::new("Custom Action") + .disabled(false) + .icon(IconName::Star) + .on_click(|window, cx| { + // Custom click handler logic + println!("Custom Action Clicked!"); + }) + ) + .separator() + .menu("Standard Action", Box::new(StandardAction)) + }) +``` + ### Editor Menu with Shortcuts ```rust @@ -460,3 +438,7 @@ Button::new("settings") 6. **Progressive Disclosure**: Use submenus for complex hierarchies 7. **Clear Labels**: Use descriptive, action-oriented labels 8. **Reasonable Limits**: Use scrollable menus for more than 10-15 items + +[PopupMenu]: https://docs.rs/gpui-component/latest/gpui_component/menu/popup_menu/struct.PopupMenu.html +[PopupMenuItem]: https://docs.rs/gpui-component/latest/gpui_component/menu/popup_menu/struct.PopupMenuItem.html +[context_menu]: https://docs.rs/gpui-component/latest/gpui_component/menu/context_menu/trait.ContextMenuExt.html#method.context_menu