From 1fce2e1ac4bd5033b3fffe52e6e44cdb6c0a9f01 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Thu, 9 Oct 2025 10:52:33 +0800 Subject: [PATCH] menu: Improve MenuItem paddings and fix submenu support. (#1339) Fix #1332 was broken submenu selection. --- crates/ui/src/menu/menu_item.rs | 33 ++++++-------------- crates/ui/src/menu/popup_menu.rs | 52 +++++++++++++++++--------------- 2 files changed, 37 insertions(+), 48 deletions(-) diff --git a/crates/ui/src/menu/menu_item.rs b/crates/ui/src/menu/menu_item.rs index 05badfef..8a24bd8b 100644 --- a/crates/ui/src/menu/menu_item.rs +++ b/crates/ui/src/menu/menu_item.rs @@ -1,32 +1,32 @@ -use crate::{h_flex, ActiveTheme, Disableable, Selectable, StyledExt}; +use crate::{h_flex, ActiveTheme, Disableable, StyledExt}; use gpui::{ prelude::FluentBuilder as _, AnyElement, App, ClickEvent, ElementId, InteractiveElement, - IntoElement, MouseButton, ParentElement, RenderOnce, StatefulInteractiveElement as _, - StyleRefinement, Styled, Window, + IntoElement, MouseButton, ParentElement, RenderOnce, SharedString, + StatefulInteractiveElement as _, StyleRefinement, Styled, Window, }; use smallvec::SmallVec; #[derive(IntoElement)] pub struct MenuItem { id: ElementId, + group_name: SharedString, style: StyleRefinement, disabled: bool, selected: bool, - hovered: bool, on_click: Option>, on_hover: Option>, children: SmallVec<[AnyElement; 2]>, } impl MenuItem { - pub fn new(id: impl Into) -> Self { + pub fn new(id: impl Into, group_name: impl Into) -> Self { let id: ElementId = id.into(); Self { id: id.clone(), + group_name: group_name.into(), style: StyleRefinement::default(), disabled: false, selected: false, - hovered: false, on_click: None, on_hover: None, children: SmallVec::new(), @@ -44,11 +44,6 @@ impl MenuItem { self } - pub fn hovered(mut self, hovered: bool) -> Self { - self.hovered = hovered; - self - } - pub fn on_click( mut self, handler: impl Fn(&ClickEvent, &mut Window, &mut App) + 'static, @@ -72,17 +67,6 @@ impl Disableable for MenuItem { } } -impl Selectable for MenuItem { - fn selected(mut self, selected: bool) -> Self { - self.selected = selected; - self - } - - fn is_selected(&self) -> bool { - self.selected - } -} - impl Styled for MenuItem { fn style(&mut self) -> &mut gpui::StyleRefinement { &mut self.style @@ -99,6 +83,7 @@ impl RenderOnce for MenuItem { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { h_flex() .id(self.id) + .group(&self.group_name) .gap_x_1() .py_1() .px_2() @@ -112,11 +97,11 @@ impl RenderOnce for MenuItem { this.on_hover(move |hovered, window, cx| (on_hover)(hovered, window, cx)) }) .when(!self.disabled, |this| { - this.when(self.hovered, |this| { + this.group_hover(self.group_name, |this| { this.bg(cx.theme().accent) .text_color(cx.theme().accent_foreground) }) - .hover(|this| { + .when(self.selected, |this| { this.bg(cx.theme().accent) .text_color(cx.theme().accent_foreground) }) diff --git a/crates/ui/src/menu/popup_menu.rs b/crates/ui/src/menu/popup_menu.rs index 7c3684c4..d813eb45 100644 --- a/crates/ui/src/menu/popup_menu.rs +++ b/crates/ui/src/menu/popup_menu.rs @@ -792,9 +792,9 @@ impl PopupMenu { let bounds = self.bounds; let max_width = self.max_width(); let (anchor, left) = if max_width + bounds.origin.x > window.bounds().size.width { - (Corner::TopRight, -px(14.)) + (Corner::TopRight, -px(16.)) } else { - (Corner::TopLeft, bounds.size.width) + (Corner::TopLeft, bounds.size.width - px(8.)) }; let is_bottom_pos = bounds.origin.y + bounds.size.height > window.bounds().size.height; @@ -815,29 +815,31 @@ impl PopupMenu { ) -> impl IntoElement { let has_icon = self.has_icon; let selected = self.selected_index == Some(ix); - const EDGE_PADDING: Pixels = px(8.); - const INNER_PADDING: Pixels = px(4.); + const EDGE_PADDING: Pixels = px(4.); + const INNER_PADDING: Pixels = px(8.); + + let is_submenu = matches!(item, PopupMenuItem::Submenu { .. }); + let group_name = format!("popup-menu-item-{}", ix); let (item_height, radius) = match self.size { Size::Small => (px(20.), state.radius.half()), _ => (px(26.), state.radius), }; - let this = MenuItem::new(ix) + let this = MenuItem::new(ix, &group_name) .relative() .text_sm() .py_0() .px(INNER_PADDING) .rounded(radius) .items_center() - .hovered(selected) + .selected(selected) .on_hover(cx.listener(move |this, hovered, _, cx| { if *hovered { this.selected_index = Some(ix); - } else { - if this.selected_index == Some(ix) { - this.selected_index = None; - } + } else if !is_submenu && this.selected_index == Some(ix) { + // TODO: Better handle the submenu unselection when hover out + this.selected_index = None; } cx.notify(); @@ -951,23 +953,25 @@ impl PopupMenu { .child(IconName::ChevronRight), ), ) - .when(selected, |this| { + .child({ let (anchor, left) = self.child_menu_anchor(window); let is_bottom_pos = matches!(anchor, Corner::BottomLeft | Corner::BottomRight); - this.child( - anchored() - .anchor(anchor) - .child( - div() - .occlude() - .when(is_bottom_pos, |this| this.bottom_0()) - .when(!is_bottom_pos, |this| this.top_neg_1()) - .left(left) - .child(menu.clone()), - ) - .snap_to_window_with_margin(Edges::all(EDGE_PADDING)), - ) + anchored() + .anchor(anchor) + .child( + div() + .id("submenu") + .group(&group_name) + .when(!selected, |this| this.invisible()) + .group_hover(&group_name, |this| this.visible()) + .occlude() + .when(is_bottom_pos, |this| this.bottom_0()) + .when(!is_bottom_pos, |this| this.top_neg_1()) + .left(left) + .child(menu.clone()), + ) + .snap_to_window_with_margin(Edges::all(EDGE_PADDING)) }), } }