From af2177c6c82cc5df60ad7493abbc97a40d66eef0 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Tue, 10 Nov 2026 09:32:51 +0700 Subject: [PATCH] feat: add sub menu and panel min width to menus --- crates/core/src/dropdown_menu.rs | 52 ++- crates/core/src/menu.rs | 712 ++++++++++++++++++++++++------- crates/core/tests/menu.rs | 381 +++++++++++++++-- 3 files changed, 934 insertions(+), 211 deletions(-) diff --git a/crates/core/src/dropdown_menu.rs b/crates/core/src/dropdown_menu.rs index f4557b5..58a2cbe 100644 --- a/crates/core/src/dropdown_menu.rs +++ b/crates/core/src/dropdown_menu.rs @@ -5,9 +5,9 @@ use std::rc::Rc; use gpui::prelude::FluentBuilder as _; use gpui::{ - App, ClickEvent, DispatchPhase, ElementId, InteractiveElement as _, IntoElement, KeyDownEvent, - MouseButton, MouseDownEvent, ParentElement as _, RenderOnce, Role, SharedString, - StatefulInteractiveElement as _, Styled as _, Window, canvas, div, relative, + App, ClickEvent, DefiniteLength, DispatchPhase, ElementId, InteractiveElement as _, + IntoElement, KeyDownEvent, MouseButton, MouseDownEvent, ParentElement as _, RenderOnce, Role, + SharedString, StatefulInteractiveElement as _, Styled as _, Window, canvas, div, relative, }; use gpui_native_icons::{Icon, IconRole, icon}; use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, Size}; @@ -15,7 +15,9 @@ use gpui_native_utils::{focus_handle, focus_ring}; use crate::button::ButtonVariant; use crate::common::{Sizable, h_flex}; -use crate::menu::{ChangeHandler, MenuChrome, MenuItem, TriggerBoundsCell, anchored_menu_panel}; +use crate::menu::{ + ChangeHandler, MenuChrome, MenuItem, MenuPanelSpec, TriggerBoundsCell, anchored_menu_panel, +}; use crate::spinner::Spinner; /// The main zone's activation handler, shared by pointer, Enter, and Space. @@ -38,6 +40,7 @@ pub struct DropdownMenu { on_click: Option, on_open_change: Option, items: Option, + min_width: Option, } impl DropdownMenu { @@ -60,6 +63,7 @@ impl DropdownMenu { on_click: None, on_open_change: None, items: None, + min_width: None, } } @@ -146,6 +150,15 @@ impl DropdownMenu { self.items = Some(Box::new(items)); self } + + /// Sets the panel's minimum width, defaults to the standard menu width. + pub fn min_width(mut self, width: W) -> Self + where + W: Into, + { + self.min_width = Some(width.into()); + self + } } impl Sizable for DropdownMenu { @@ -374,6 +387,21 @@ impl RenderOnce for DropdownMenu { Rc::from([]) }; + let on_close = Some(on_open.clone()); + + let spec = MenuPanelSpec { + open: self.open, + items, + initial_selection: None, + preselect: *keyboard_open.read(cx), + min_width: self.min_width, + label, + position: None, + trigger_bounds: trigger_bounds.clone(), + on_dismiss: on_close.clone(), + on_activate: on_close, + }; + div() .relative() .child( @@ -431,21 +459,7 @@ impl RenderOnce for DropdownMenu { ) .child(trigger) .when_some( - anchored_menu_panel( - window, - cx, - &self.id, - self.open, - items, - None, - *keyboard_open.read(cx), - Some(on_open), - label, - None, - &trigger_bounds, - &panel_chrome, - enter_duration, - ), + anchored_menu_panel(window, cx, &self.id, spec, &panel_chrome, enter_duration), |wrapper, panel| wrapper.child(panel), ) } diff --git a/crates/core/src/menu.rs b/crates/core/src/menu.rs index c00ec24..026e5b6 100644 --- a/crates/core/src/menu.rs +++ b/crates/core/src/menu.rs @@ -1,16 +1,19 @@ +//! The floating menu panel shared by menus, dropdowns, and context menus. + use std::cell::RefCell; use std::rc::Rc; use gpui::prelude::FluentBuilder; use gpui::{ - Animation, AnimationExt as _, AnyElement, App, Bounds, BoxShadow, DispatchPhase, ElementId, - Entity, InteractiveElement as _, IntoElement, KeyDownEvent, MouseButton, MouseDownEvent, - ParentElement as _, Pixels, Point, RenderOnce, Role, ScrollHandle, SharedString, - StatefulInteractiveElement as _, Styled as _, Window, anchored, canvas, deferred, div, - ease_out_quint, point, px, relative, + Animation, AnimationExt as _, AnyElement, App, Bounds, BoxShadow, DefiniteLength, + DispatchPhase, ElementId, Entity, FocusHandle, InteractiveElement as _, IntoElement, + KeyDownEvent, MouseButton, MouseDownEvent, ParentElement as _, Pixels, Point, RenderOnce, Role, + ScrollHandle, SharedString, StatefulInteractiveElement as _, Styled as _, Window, anchored, + canvas, deferred, div, ease_out_quint, point, px, relative, }; use gpui_native_icons::{Icon, IconRole, icon as icon_glyph}; use gpui_native_theme::{ActiveTheme as _, POPUP_MAX_HEIGHT}; +use gpui_native_utils::focus_handle; use crate::common::v_flex; use crate::scroll_area::{ScrollArea, ScrollAxes}; @@ -25,7 +28,7 @@ pub(crate) type TriggerBoundsCell = Rc>>>; type TriggerFn = Box AnyElement>; type ItemsFn = Box Vec>; -/// One row of a [`Menu`]: an action, a checkable action, or a separator. +/// One row of a [`Menu`]: an action, a checkable action, a submenu, or a separator. #[derive(Clone)] pub struct MenuItem { kind: MenuItemKind, @@ -42,6 +45,7 @@ pub struct MenuItem { enum MenuItemKind { Action, Separator, + Submenu(Rc<[MenuItem]>), } impl MenuItem { @@ -121,9 +125,26 @@ impl MenuItem { self } + /// Opens a submenu with these rows instead of running an action. + pub fn submenu(mut self, items: I) -> Self + where + I: IntoIterator, + { + self.kind = MenuItemKind::Submenu(items.into_iter().collect::>().into()); + self + } + + /// The submenu's rows, when this item opens one. + fn submenu_items(&self) -> Option<&[MenuItem]> { + match &self.kind { + MenuItemKind::Submenu(items) => Some(items), + _ => None, + } + } + /// Whether keyboard navigation and activation may land on this item. fn selectable(&self) -> bool { - matches!(self.kind, MenuItemKind::Action) && !self.disabled + matches!(self.kind, MenuItemKind::Action | MenuItemKind::Submenu(_)) && !self.disabled } } @@ -165,6 +186,7 @@ pub struct Menu { on_change: Option, trigger: Option, items: Option, + min_width: Option, } impl MenuChrome { @@ -217,6 +239,7 @@ impl Menu { on_change: None, trigger: None, items: None, + min_width: None, } } @@ -279,12 +302,22 @@ impl Menu { self.a11y_label = Some(label.into()); self } + + /// Sets the panel's minimum width; defaults to the standard menu width. + pub fn min_width(mut self, width: W) -> Self + where + W: Into, + { + self.min_width = Some(width.into()); + self + } } impl RenderOnce for Menu { fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { let (chrome, enter_duration) = MenuChrome::read(cx); let trigger_bounds: Rc>>> = Rc::new(RefCell::new(None)); + let on_change = self.on_change.clone(); let fallback_label = self .a11y_label @@ -300,6 +333,19 @@ impl RenderOnce for Menu { Rc::from([]) }; + let spec = MenuPanelSpec { + open: self.open, + items, + initial_selection: self.initial_selection, + preselect: false, + min_width: self.min_width, + label: fallback_label, + position: self.open_at, + trigger_bounds: trigger_bounds.clone(), + on_dismiss: on_change.clone(), + on_activate: self.on_change, + }; + div() .relative() .child( @@ -309,7 +355,7 @@ impl RenderOnce for Menu { move |bounds, _window, _cx| *cell.borrow_mut() = Some(bounds) }, { - let on_change = self.on_change.clone(); + let on_change = on_change; let open = self.open; let trigger_button = self.trigger_button; let cell = trigger_bounds.clone(); @@ -349,65 +395,172 @@ impl RenderOnce for Menu { wrapper.child(trigger(window, cx)) }) .when_some( - anchored_menu_panel( - window, - cx, - &self.id, - self.open, - items, - self.initial_selection, - false, - self.on_change, - fallback_label, - self.open_at, - &trigger_bounds, - &chrome, - enter_duration, - ), + anchored_menu_panel(window, cx, &self.id, spec, &chrome, enter_duration), |wrapper, panel| wrapper.child(panel), ) } } -/// Assembles the floating menu panel. -#[allow(clippy::too_many_arguments)] +/// Everything the floating menu panel needs from its owner. +pub(crate) struct MenuPanelSpec { + pub open: bool, + pub items: Rc<[MenuItem]>, + pub initial_selection: Option, + pub preselect: bool, + pub min_width: Option, + pub label: SharedString, + pub position: Option>, + pub trigger_bounds: TriggerBoundsCell, + /// Closes this panel: outside clicks, `escape`, and `left` past a closed flyout. + pub on_dismiss: Option, + /// Closes the whole menu when a row inside activates. + pub on_activate: Option, +} + +/// Per-panel state that survives across frames while the panel stays mounted. +struct MenuPanelState { + focus: FocusHandle, + was_open: Entity, + scroll: Entity, + selection: Entity>, +} + +fn create_menu_panel_state(id: &ElementId, window: &mut Window, cx: &mut App) -> MenuPanelState { + MenuPanelState { + focus: focus_handle(&(id.clone(), "panel").into(), window, cx), + was_open: window.use_keyed_state((id.clone(), "menu-was-open"), cx, |_, _| false), + scroll: window.use_keyed_state((id.clone(), "menu-scroll"), cx, |_, _| ScrollHandle::new()), + selection: window.use_keyed_state((id.clone(), "menu-selection"), cx, |_, _| None), + } +} + +struct MenuPanelParams<'a> { + id: &'a ElementId, + items: Rc<[MenuItem]>, + initial_selection: Option, + preselect: bool, + min_width: Option, + label: SharedString, + /// Clicks here do not dismiss the panel: the trigger or the owning row. + dismiss_bounds: &'a TriggerBoundsCell, + /// Receives the panel's painted bounds, for a parent's dismissal checks. + bounds_sink: Option<&'a TriggerBoundsCell>, + /// This panel's flyout bounds are pushed here. + outer_stack: Option<&'a Rc>>>, + on_dismiss: Option, + on_activate: Option, + /// Flyout panels dismiss themselves on `left`; top-level panels ignore it. + is_flyout: bool, + chrome: &'a MenuChrome, + enter_duration: std::time::Duration, + debug_prefix: &'static str, +} + +/// Wiring a panel's rows need to open their submenus. +struct FlyoutContext<'a> { + state: &'a MenuPanelState, + open: &'a Entity>, + preselect: &'a Entity, + /// The open flyout's painted bounds; a parent ignores clicks inside them. + bounds: &'a TriggerBoundsCell, + /// Bounds of every open flyout below this panel, this level's included. + stack: &'a Rc>>, + /// Closes this level's flyout and returns focus to this panel. + dismiss: ChangeHandler, + on_activate: Option, + id: ElementId, + enter_duration: std::time::Duration, + min_width: Option, +} + +struct RowEnv<'a> { + chrome: &'a MenuChrome, + selection: &'a Entity>, + on_activate: Option, + flyout: &'a FlyoutContext<'a>, + debug_prefix: &'static str, +} + +/// Assembles the floating menu panel anchored to its trigger. pub(crate) fn anchored_menu_panel( window: &mut Window, cx: &mut App, id: &ElementId, - open: bool, - items: Rc<[MenuItem]>, - initial_selection: Option, - preselect: bool, - on_change: Option, - label: SharedString, - position: Option>, - trigger_bounds: &TriggerBoundsCell, + spec: MenuPanelSpec, chrome: &MenuChrome, enter_duration: std::time::Duration, ) -> Option { let was_open = window.use_keyed_state((id.clone(), "menu-was-open"), cx, |_, _| false); - if !open { + if !spec.open { was_open.update(cx, |was_open, _| *was_open = false); + // A closed menu drops any open flyout; fresh state on the next open. + window + .use_keyed_state::>((id.clone(), "submenu-open"), cx, |_, _| None) + .update(cx, |open, _| *open = None); + window + .use_keyed_state((id.clone(), "submenu-preselect"), cx, |_, _| false) + .update(cx, |preselect, _| *preselect = false); return None; } - let on_change = on_change.clone(); - // Keyboard focus rides the panel across frames - let focus = gpui_native_utils::focus_handle(&(id.clone(), "panel").into(), window, cx); - let scroll = - window.use_keyed_state((id.clone(), "menu-scroll"), cx, |_, _| ScrollHandle::new()); - let selection = window.use_keyed_state((id.clone(), "menu-selection"), cx, |_, _| None); + let state = create_menu_panel_state(id, window, cx); + let panel = menu_panel( + window, + cx, + &state, + MenuPanelParams { + id, + items: spec.items, + initial_selection: spec.initial_selection, + preselect: spec.preselect, + min_width: spec.min_width, + label: spec.label, + dismiss_bounds: &spec.trigger_bounds, + bounds_sink: None, + outer_stack: None, + on_dismiss: spec.on_dismiss, + on_activate: spec.on_activate, + is_flyout: false, + chrome, + enter_duration, + debug_prefix: "menu", + }, + ); + + let anchored = match spec.position { + Some(position) => anchored().position(position), + None => anchored(), + } + .offset(point(px(0.), chrome.spacing_unit)); + + Some(deferred(anchored.child(panel)).into_any_element()) +} + +/// Builds one menu panel: chrome, rows, focus, navigation, and dismissals. +fn menu_panel( + window: &mut Window, + cx: &mut App, + state: &MenuPanelState, + params: MenuPanelParams, +) -> AnyElement { + let MenuPanelState { + focus, + was_open, + scroll, + selection, + } = state; let selected = if *was_open.read(cx) { *selection.read(cx) } else { - let requested = initial_selection - .map(|index| index.min(items.len().saturating_sub(1))) + let requested = params + .initial_selection + .map(|index| index.min(params.items.len().saturating_sub(1))) .or_else(|| { - preselect - .then(|| items.iter().position(|item| item.selectable())) + params + .preselect + .then(|| params.items.iter().position(MenuItem::selectable)) .flatten() }); selection.update(cx, |selection, _| *selection = requested); @@ -415,59 +568,151 @@ pub(crate) fn anchored_menu_panel( }; was_open.update(cx, |was_open, _| *was_open = true); - let rows: Vec = items + // Submenu plumbing for this level; a flyout panel recurses with its own. + let flyout_id: ElementId = (params.id.clone(), "submenu").into(); + let flyout_state = create_menu_panel_state(&flyout_id, window, cx); + let flyout_open = window.use_keyed_state((params.id.clone(), "submenu-open"), cx, |_, _| None); + let flyout_preselect = + window.use_keyed_state((params.id.clone(), "submenu-preselect"), cx, |_, _| false); + let flyout_last = + window.use_keyed_state((params.id.clone(), "submenu-last-open"), cx, |_, _| None); + let flyout_bounds: TriggerBoundsCell = Rc::new(RefCell::new(None)); + let flyout_stack: Rc>> = Rc::new(RefCell::new(Vec::new())); + flyout_stack.borrow_mut().push(flyout_bounds.clone()); + + if let Some(outer_stack) = params.outer_stack { + outer_stack.borrow_mut().push(flyout_bounds.clone()); + } + + // A switched or reopened flyout starts from a clean selection. + let open_flyout = *flyout_open.read(cx); + if *flyout_last.read(cx) != open_flyout { + flyout_state + .was_open + .update(cx, |was_open, _| *was_open = false); + flyout_state + .selection + .update(cx, |selection, _| *selection = None); + flyout_last.update(cx, |last, _| *last = open_flyout); + } + + let dismiss_flyout: ChangeHandler = { + let flyout_open = flyout_open.clone(); + let flyout_preselect = flyout_preselect.clone(); + let focus = focus.clone(); + Rc::new(move |_, window, cx| { + flyout_open.update(cx, |open, _| *open = None); + flyout_preselect.update(cx, |preselect, _| *preselect = false); + window.focus(&focus, cx); + }) + }; + + let flyout = FlyoutContext { + state: &flyout_state, + open: &flyout_open, + preselect: &flyout_preselect, + bounds: &flyout_bounds, + stack: &flyout_stack, + dismiss: dismiss_flyout, + on_activate: params.on_activate.clone(), + id: flyout_id, + enter_duration: params.enter_duration, + min_width: params.min_width, + }; + + let rows: Vec = params + .items .iter() .enumerate() .map(|(index, item)| { render_item( + window, + cx, item, index, selected == Some(index), - &selection, - &on_change, - chrome, + &RowEnv { + chrome: params.chrome, + selection, + on_activate: params.on_activate.clone(), + flyout: &flyout, + debug_prefix: params.debug_prefix, + }, ) }) .collect(); - let panel = v_flex() + v_flex() .id("menu-panel") - .debug_selector(|| "menu-panel".to_string()) + .debug_selector(move || format!("{}-panel", params.debug_prefix)) .occlude() - .track_focus(&focus) + .track_focus(focus) .role(Role::Menu) - .aria_label(label) - .min_w_56() - .p(chrome.spacing_unit) - .bg(chrome.surface) - .text_color(chrome.foreground) - .rounded(chrome.radius_overlay) + .aria_label(params.label) + .when_some(params.min_width, |panel, width| panel.min_w(width)) + .when(params.min_width.is_none(), |panel| panel.min_w_56()) + .p(params.chrome.spacing_unit) + .bg(params.chrome.surface) + .text_color(params.chrome.foreground) + .rounded(params.chrome.radius_overlay) .border_1() - .border_color(chrome.panel_border) - .shadow(chrome.shadow.clone()) + // A flyout overlaps its parent, so it always wears a visible border. + .border_color(if params.is_flyout { + params.chrome.border + } else { + params.chrome.panel_border + }) + .shadow(params.chrome.shadow.clone()) .overflow_hidden() .on_mouse_down_out({ - let on_change = on_change.clone(); - let trigger_bounds = trigger_bounds.clone(); + let on_dismiss = params.on_dismiss.clone(); + let dismiss_bounds = params.dismiss_bounds.clone(); + let flyout_stack = flyout_stack.clone(); move |event, window, cx| { - if trigger_bounds + if dismiss_bounds .borrow() .is_some_and(|bounds| bounds.contains(&event.position)) { return; } - if let Some(on_change) = &on_change { - on_change(false, window, cx); + + // Clicks inside any open flyout belong to that flyout. + if flyout_stack.borrow().iter().any(|cell| { + cell.borrow() + .is_some_and(|bounds| bounds.contains(&event.position)) + }) { + return; + } + + if let Some(on_dismiss) = &on_dismiss { + on_dismiss(false, window, cx); } } }) .on_key_down({ - let items = items.clone(); - let on_change = on_change.clone(); + let items = params.items.clone(); let selection = selection.clone(); + let flyout_open = flyout_open.clone(); + let flyout_preselect = flyout_preselect.clone(); + let on_dismiss = params.on_dismiss.clone(); + let on_activate = params.on_activate.clone(); + let left_dismisses = params.is_flyout; move |event: &KeyDownEvent, window, cx| { - handle_menu_key(event, &items, &selection, &on_change, window, cx); + handle_menu_key( + event, + MenuKeyEnv { + items: &items, + selection: &selection, + flyout_open: &flyout_open, + flyout_preselect: &flyout_preselect, + on_dismiss: &on_dismiss, + on_activate: &on_activate, + left_dismisses, + }, + window, + cx, + ); } }) .child( @@ -478,110 +723,81 @@ pub(crate) fn anchored_menu_panel( .child(v_flex().gap_0p5().children(rows)), ) .child( - canvas(|_, _, _| {}, { - let focus = focus.clone(); - move |_, _, window, cx| { - if !focus.contains_focused(window, cx) { - window.focus(&focus, cx); + canvas( + { + let bounds_sink = params.bounds_sink.cloned(); + move |bounds, _window, _cx| { + if let Some(cell) = &bounds_sink { + *cell.borrow_mut() = Some(bounds); + } } - } - }) + }, + { + let focus = focus.clone(); + let flyout_open = flyout_open.clone(); + move |_, _, window, cx| { + if flyout_open.read(cx).is_none() && !focus.contains_focused(window, cx) { + window.focus(&focus, cx); + } + } + }, + ) .absolute() .inset_0(), - ); - - let anchored = match position { - Some(position) => anchored().position(position), - None => anchored(), - } - .offset(point(px(0.), chrome.spacing_unit)); - - Some( - deferred(anchored.child(panel.with_animation( + ) + .with_animation( "menu-enter", - Animation::new(enter_duration).with_easing(ease_out_quint()), + Animation::new(params.enter_duration).with_easing(ease_out_quint()), |panel, delta| panel.opacity(delta), - ))) - .into_any_element(), - ) + ) + .into_any_element() } /// Renders one menu row. fn render_item( + window: &mut Window, + cx: &mut App, item: &MenuItem, index: usize, selected: bool, - selection: &Entity>, - on_change: &Option, - chrome: &MenuChrome, + env: &RowEnv, ) -> AnyElement { - match item.kind { + match &item.kind { MenuItemKind::Separator => div() .h(px(1.)) - .debug_selector(move || format!("menu-separator-{index}")) - .mx(chrome.spacing_unit * 2.0) - .my(chrome.spacing_unit) - .bg(chrome.border) + .debug_selector(move || format!("{}-separator-{index}", env.debug_prefix)) + .mx(env.chrome.spacing_unit * 2.0) + .my(env.chrome.spacing_unit) + .bg(env.chrome.border) .into_any_element(), MenuItemKind::Action => { - let label_color = row_color(item, selected, chrome); + let label_color = row_color(item, selected, false, env.chrome); let disabled = item.disabled; - let selectable = item.selectable(); let destructive = item.destructive; let checked = item.checked; let shortcut = item.shortcut.clone(); let icon = item.icon.clone(); - div() - .id(("menu-item", index)) - .debug_selector(move || format!("menu-item-{index}")) - .flex() - .items_center() - .gap_2() - .min_h(chrome.menu_item_height) - .px(chrome.spacing_unit * 2.0) - .text_size(chrome.text_size) - .line_height(relative(1.0)) - .rounded(chrome.radius_sm) - .map(|row| { - if disabled { - row - } else if selected { - let highlight = if destructive { - chrome.destructive - } else { - chrome.primary - }; - row.bg(highlight) - } else { - row.hover(|row| row.bg(chrome.muted_hover)) - } - }) - .on_mouse_move({ - let selection = selection.clone(); - move |_, _window, cx| { - if selectable && *selection.read(cx) != Some(index) { - selection.update(cx, |selection, _| *selection = Some(index)); - } - } - }) + row_shell(cx, item, index, selected, env) .on_click({ let handler = item.handler.clone(); - let on_change = on_change.clone(); + let on_activate = env.on_activate.clone(); move |_, window, cx| { if disabled { return; } - if let Some(on_change) = &on_change { - on_change(false, window, cx); + + if let Some(on_activate) = &on_activate { + on_activate(false, window, cx); } + if let Some(handler) = &handler { handler(window, cx); } } }) .when_some(icon, |row, icon| { - row.child(icon.size(chrome.icon_size).color(label_color)) + row.child(icon.size(env.chrome.icon_size).color(label_color)) }) .child( div() @@ -593,9 +809,9 @@ fn render_item( row.child( div() .text_color(if selected && !destructive && !disabled { - chrome.primary_foreground + env.chrome.primary_foreground } else { - chrome.muted_foreground + env.chrome.muted_foreground }) .child(shortcut), ) @@ -603,51 +819,201 @@ fn render_item( .when(checked, |row| { row.child( icon_glyph(IconRole::Check) - .size(chrome.dense_icon_size) + .size(env.chrome.dense_icon_size) .color(label_color), ) }) .into_any_element() } + MenuItemKind::Submenu(items) => { + let chrome = env.chrome; + let flyout = env.flyout; + let disabled = item.disabled; + let open = *flyout.open.read(cx) == Some(index); + let label_color = row_color(item, selected, open, chrome); + let row_bounds: TriggerBoundsCell = Rc::new(RefCell::new(None)); + + row_shell(cx, item, index, selected, env) + .on_click({ + let flyout_open = flyout.open.clone(); + let flyout_preselect = flyout.preselect.clone(); + move |_, _window, cx| { + if disabled { + return; + } + // Hover already opens the flyout; a click never closes it. + if *flyout_open.read(cx) != Some(index) { + flyout_open.update(cx, |open, _| *open = Some(index)); + flyout_preselect.update(cx, |preselect, _| *preselect = false); + } + } + }) + .child( + div() + .flex_1() + .text_color(label_color) + .child(item.label.clone()), + ) + .child( + icon_glyph(IconRole::ChevronRight) + .size(chrome.dense_icon_size) + .color(label_color), + ) + .aria_expanded(open) + .when(open, |row| { + row.child( + canvas( + { + let cell = row_bounds.clone(); + move |bounds, _window, _cx| *cell.borrow_mut() = Some(bounds) + }, + |_, _, _, _| {}, + ) + .absolute() + .inset_0(), + ) + .child(deferred( + div().absolute().left_full().top_0().child(menu_panel( + window, + cx, + flyout.state, + MenuPanelParams { + id: &flyout.id, + items: items.clone(), + initial_selection: None, + preselect: *flyout.preselect.read(cx), + min_width: flyout.min_width, + label: item.label.clone(), + dismiss_bounds: &row_bounds, + bounds_sink: Some(flyout.bounds), + outer_stack: Some(flyout.stack), + on_dismiss: Some(flyout.dismiss.clone()), + on_activate: flyout.on_activate.clone(), + is_flyout: true, + chrome, + enter_duration: flyout.enter_duration, + debug_prefix: "submenu", + }, + )), + )) + }) + .into_any_element() + } } } +/// The shared row shell: metrics, highlight, and hover-selection. +fn row_shell( + cx: &App, + item: &MenuItem, + index: usize, + selected: bool, + env: &RowEnv, +) -> gpui::Stateful { + let chrome = env.chrome; + let selectable = item.selectable(); + let opens_submenu = item.submenu_items().is_some(); + let flyout_open = *env.flyout.open.read(cx) == Some(index); + + div() + .id(("menu-item", index)) + .debug_selector(move || format!("{}-item-{index}", env.debug_prefix)) + .flex() + .items_center() + .gap_2() + .min_h(chrome.menu_item_height) + .px(chrome.spacing_unit * 2.0) + .text_size(chrome.text_size) + .line_height(relative(1.0)) + .rounded(chrome.radius_sm) + .map(|row| { + if item.disabled { + row + } else if selected && flyout_open { + row.bg(chrome.muted_hover) + } else if selected { + let highlight = if item.destructive { + chrome.destructive + } else { + chrome.primary + }; + row.bg(highlight) + } else { + row.hover(|row| row.bg(chrome.muted_hover)) + } + }) + .on_mouse_move({ + let selection = env.selection.clone(); + let flyout_open = env.flyout.open.clone(); + let flyout_preselect = env.flyout.preselect.clone(); + let dismiss = env.flyout.dismiss.clone(); + move |_, window, cx| { + if !selectable { + return; + } + + if *selection.read(cx) != Some(index) { + selection.update(cx, |selection, _| *selection = Some(index)); + } + + if opens_submenu { + if *flyout_open.read(cx) != Some(index) { + flyout_open.update(cx, |open, _| *open = Some(index)); + flyout_preselect.update(cx, |preselect, _| *preselect = false); + } + } else if flyout_open.read(cx).is_some() { + dismiss(false, window, cx); + } + } + }) +} + /// The row's label/icon color. -fn row_color(item: &MenuItem, selected: bool, chrome: &MenuChrome) -> gpui::Hsla { +fn row_color( + item: &MenuItem, + selected: bool, + flyout_open: bool, + chrome: &MenuChrome, +) -> gpui::Hsla { if item.disabled { return chrome.muted_foreground; } if item.destructive { - return if selected { + return if selected && !flyout_open { chrome.destructive_foreground } else { chrome.destructive }; } - if selected { + if selected && !flyout_open { chrome.primary_foreground } else { chrome.foreground } } +/// Everything key navigation needs from the panel. +struct MenuKeyEnv<'a> { + items: &'a [MenuItem], + selection: &'a Entity>, + flyout_open: &'a Entity>, + flyout_preselect: &'a Entity, + on_dismiss: &'a Option, + on_activate: &'a Option, + left_dismisses: bool, +} + /// Handles the panel's navigation keys. -fn handle_menu_key( - event: &KeyDownEvent, - items: &[MenuItem], - selection: &Entity>, - on_change: &Option, - window: &mut Window, - cx: &mut App, -) { - let selectable: Vec = items +fn handle_menu_key(event: &KeyDownEvent, env: MenuKeyEnv, window: &mut Window, cx: &mut App) { + let selectable: Vec = env + .items .iter() .enumerate() .filter(|(_, item)| item.selectable()) .map(|(index, _)| index) .collect(); - let current = *selection.read(cx); + let current = *env.selection.read(cx); let position = current.and_then(|current| selectable.iter().position(|index| *index == current)); @@ -668,24 +1034,50 @@ fn handle_menu_key( "home" => 0, _ => selectable.len() - 1, }; - selection.update(cx, |selection, _| *selection = Some(selectable[next])); + env.selection + .update(cx, |selection, _| *selection = Some(selectable[next])); + } + "right" => { + let opens = current + .and_then(|current| env.items.get(current)) + .is_some_and(|item| item.selectable() && item.submenu_items().is_some()); + if !opens { + return; + } + env.flyout_open.update(cx, |open, _| *open = current); + env.flyout_preselect + .update(cx, |preselect, _| *preselect = true); } "enter" | "space" => { if let Some(current) = current - && let Some(item) = items.get(current) + && let Some(item) = env.items.get(current) && item.selectable() { - if let Some(on_change) = &on_change { - on_change(false, window, cx); - } - if let Some(handler) = &item.handler { - handler(window, cx); + if item.submenu_items().is_some() { + env.flyout_open.update(cx, |open, _| *open = Some(current)); + env.flyout_preselect + .update(cx, |preselect, _| *preselect = true); + } else { + if let Some(on_activate) = env.on_activate { + on_activate(false, window, cx); + } + if let Some(handler) = &item.handler { + handler(window, cx); + } } } } "escape" => { - if let Some(on_change) = &on_change { - on_change(false, window, cx); + if let Some(on_dismiss) = env.on_dismiss { + on_dismiss(false, window, cx); + } + } + "left" => { + if !env.left_dismisses { + return; + } + if let Some(on_dismiss) = env.on_dismiss { + on_dismiss(false, window, cx); } } _ => return, diff --git a/crates/core/tests/menu.rs b/crates/core/tests/menu.rs index e9e6520..0de191c 100644 --- a/crates/core/tests/menu.rs +++ b/crates/core/tests/menu.rs @@ -1,14 +1,15 @@ use std::{cell::RefCell, rc::Rc}; use gpui::{ - Context, IntoElement, Modifiers, MouseButton, ParentElement as _, Render, Styled as _, - TestAppContext, VisualTestContext, Window, WindowHandle, div, point, px, + Context, DefiniteLength, IntoElement, Modifiers, MouseButton, ParentElement as _, Render, + Styled as _, TestAppContext, VisualTestContext, Window, WindowHandle, div, point, px, }; use gpui_native_core::menu::{Menu, MenuItem}; use gpui_native_theme::Theme; struct Harness { open: bool, + min_width: Option, changes: Rc>>, activated: Rc>>, } @@ -18,36 +19,38 @@ impl Render for Harness { let changes = self.changes.clone(); let view = cx.entity(); let activated = self.activated.clone(); - div().size_full().child( - Menu::new("test-menu") - .open(self.open) - .on_change(move |next, _, cx| { - changes.borrow_mut().push(next); - view.update(cx, |harness, cx| { - harness.open = next; - cx.notify(); - }); - }) - .trigger(|_, _| { - div() - .w(px(100.)) - .h(px(32.)) - .bg(gpui::black()) - .into_any_element() - }) - .items(move |_, _| { - vec![ - cut_item(activated.clone()), - MenuItem::separator(), - copy_item(activated.clone()), - MenuItem::new("Unavailable").disabled(true).handler({ - let activated = activated.clone(); - move |_, _| record(&activated, "unavailable") - }), - delete_item(activated.clone()), - ] - }), - ) + let mut menu = Menu::new("test-menu") + .open(self.open) + .on_change(move |next, _, cx| { + changes.borrow_mut().push(next); + view.update(cx, |harness, cx| { + harness.open = next; + cx.notify(); + }); + }) + .trigger(|_, _| { + div() + .w(px(100.)) + .h(px(32.)) + .bg(gpui::black()) + .into_any_element() + }) + .items(move |_, _| { + vec![ + cut_item(activated.clone()), + MenuItem::separator(), + copy_item(activated.clone()), + MenuItem::new("Unavailable").disabled(true).handler({ + let activated = activated.clone(); + move |_, _| record(&activated, "unavailable") + }), + delete_item(activated.clone()), + ] + }); + if let Some(width) = self.min_width { + menu = menu.min_width(width); + } + div().size_full().child(menu) } } @@ -67,6 +70,61 @@ fn delete_item(activated: Rc>>) -> MenuItem { .handler(move |_, _| record(&activated, "delete")) } +fn action_item( + label: &'static str, + key: &'static str, + activated: Rc>>, +) -> MenuItem { + MenuItem::new(label).handler(move |_, _| record(&activated, key)) +} + +fn submenu_item(activated: Rc>>) -> MenuItem { + MenuItem::new("Export").submenu(vec![ + action_item("PNG", "png", activated.clone()), + action_item("PDF", "pdf", activated.clone()), + ]) +} + +/// A menu with an Export row whose flyout holds PNG and PDF actions. +struct SubmenuHarness { + open: bool, + changes: Rc>>, + activated: Rc>>, +} + +impl Render for SubmenuHarness { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let changes = self.changes.clone(); + let view = cx.entity(); + let activated = self.activated.clone(); + div().size_full().child( + Menu::new("test-submenu") + .open(self.open) + .on_change(move |next, _, cx| { + changes.borrow_mut().push(next); + view.update(cx, |harness, cx| { + harness.open = next; + cx.notify(); + }); + }) + .trigger(|_, _| { + div() + .w(px(100.)) + .h(px(32.)) + .bg(gpui::black()) + .into_any_element() + }) + .items(move |_, _| { + vec![ + action_item("Cut", "cut", activated.clone()), + submenu_item(activated.clone()), + action_item("Delete", "delete", activated.clone()), + ] + }), + ) + } +} + fn record(sink: &Rc>>, what: &'static str) { sink.borrow_mut().push(what); } @@ -82,6 +140,19 @@ fn menu_window( WindowHandle, Changes, Activated, +) { + menu_window_with(open, None) +} + +fn menu_window_with( + open: bool, + min_width: Option, +) -> ( + TestAppContext, + VisualTestContext, + WindowHandle, + Changes, + Activated, ) { let mut cx = TestAppContext::single(); cx.update(Theme::init); @@ -91,6 +162,7 @@ fn menu_window( let harness_activated = activated.clone(); let window = cx.add_window(move |_, _| Harness { open, + min_width, changes: harness_changes, activated: harness_activated, }); @@ -101,6 +173,25 @@ fn menu_window( (cx, visual, window, changes, activated) } +fn submenu_window(open: bool) -> (TestAppContext, VisualTestContext, Changes, Activated) { + let mut cx = TestAppContext::single(); + cx.update(Theme::init); + let changes = Rc::new(RefCell::new(Vec::new())); + let activated = Rc::new(RefCell::new(Vec::new())); + let harness_changes = changes.clone(); + let harness_activated = activated.clone(); + let window = cx.add_window(move |_, _| SubmenuHarness { + open, + changes: harness_changes, + activated: harness_activated, + }); + let mut visual = VisualTestContext::from_window(window.into(), &cx); + visual.update(|window, cx| { + window.draw(cx).clear(cx); + }); + (cx, visual, changes, activated) +} + fn draw(visual: &mut VisualTestContext) { visual.update(|window, cx| { window.draw(cx).clear(cx); @@ -112,6 +203,17 @@ fn click(visual: &mut VisualTestContext, position: gpui::Point) { visual.simulate_mouse_up(position, MouseButton::Left, Modifiers::default()); } +fn hover(visual: &mut VisualTestContext, position: gpui::Point) { + visual.simulate_mouse_move(position, None, Modifiers::default()); +} + +fn center(bounds: gpui::Bounds) -> gpui::Point { + point( + bounds.origin.x + bounds.size.width / 2., + bounds.origin.y + bounds.size.height / 2., + ) +} + /// Every action row renders at the platform's stated menu row height, /// whatever its content — the metric is the whole row, not a floor under /// padding. Item 1 of the harness menu is a separator, so it is skipped. @@ -204,3 +306,218 @@ fn clicking_an_item_activates_and_closes() { assert_eq!(*activated.borrow(), vec!["cut"]); assert_eq!(*changes.borrow(), vec![false]); } + +#[test] +fn right_arrow_opens_the_submenu_and_enter_within_activates_through() { + let (_cx, mut visual, changes, activated) = submenu_window(true); + + // down arms Cut, the next down reaches Export, right opens its flyout + // with the first row armed. + visual.simulate_keystrokes("down down right"); + draw(&mut visual); + assert!(visual.debug_bounds("submenu-item-0").is_some()); + assert!(changes.borrow().is_empty(), "the menu stays open"); + assert!(activated.borrow().is_empty()); + + // Enter inside the flyout fires PNG and closes the whole menu. + visual.simulate_keystrokes("enter"); + draw(&mut visual); + assert_eq!(*activated.borrow(), vec!["png"]); + assert_eq!(*changes.borrow(), vec![false]); + assert!(visual.debug_bounds("submenu-item-0").is_none()); +} + +#[test] +fn enter_on_a_submenu_row_opens_it_and_navigates_within() { + let (_cx, mut visual, changes, activated) = submenu_window(true); + + visual.simulate_keystrokes("down down enter"); + draw(&mut visual); + assert!(visual.debug_bounds("submenu-item-0").is_some()); + assert!( + changes.borrow().is_empty(), + "enter opens, it does not close" + ); + + // The flyout armed PNG; down reaches PDF and enter fires it. + visual.simulate_keystrokes("down enter"); + draw(&mut visual); + assert_eq!(*activated.borrow(), vec!["pdf"]); + assert_eq!(*changes.borrow(), vec![false]); +} + +#[test] +fn left_closes_the_submenu_and_returns_to_the_menu() { + let (_cx, mut visual, changes, activated) = submenu_window(true); + + visual.simulate_keystrokes("down down right"); + draw(&mut visual); + visual.simulate_keystrokes("left"); + draw(&mut visual); + assert!(visual.debug_bounds("submenu-item-0").is_none()); + assert!(changes.borrow().is_empty(), "the menu stays open"); + assert!(activated.borrow().is_empty()); + + // Navigation resumes in the parent: down reaches Delete, enter fires it. + visual.simulate_keystrokes("down enter"); + draw(&mut visual); + assert_eq!(*activated.borrow(), vec!["delete"]); + assert_eq!(*changes.borrow(), vec![false]); +} + +#[test] +fn escape_closes_the_submenu_then_the_menu() { + let (_cx, mut visual, changes, activated) = submenu_window(true); + + visual.simulate_keystrokes("down down right"); + draw(&mut visual); + visual.simulate_keystrokes("escape"); + draw(&mut visual); + assert!(visual.debug_bounds("submenu-item-0").is_none()); + assert!( + changes.borrow().is_empty(), + "the first escape only closes the flyout" + ); + + visual.simulate_keystrokes("escape"); + draw(&mut visual); + assert_eq!(*changes.borrow(), vec![false]); + assert!(activated.borrow().is_empty()); +} + +#[test] +fn clicking_a_submenu_row_opens_the_flyout_and_a_flyout_action_closes_the_menu() { + let (_cx, mut visual, changes, activated) = submenu_window(true); + + let row = visual + .debug_bounds("menu-item-1") + .unwrap_or_else(|| panic!("the Export row should have been laid out")); + click( + &mut visual, + point( + row.origin.x + row.size.width / 2., + row.origin.y + row.size.height / 2., + ), + ); + draw(&mut visual); + assert!(visual.debug_bounds("submenu-item-0").is_some()); + assert!(changes.borrow().is_empty()); + + let png = visual + .debug_bounds("submenu-item-0") + .unwrap_or_else(|| panic!("the PNG row should have been laid out")); + click( + &mut visual, + point( + png.origin.x + png.size.width / 2., + png.origin.y + png.size.height / 2., + ), + ); + draw(&mut visual); + assert_eq!(*activated.borrow(), vec!["png"]); + assert_eq!(*changes.borrow(), vec![false]); +} + +#[test] +fn a_click_inside_the_flyout_padding_does_not_dismiss_the_menu() { + let (_cx, mut visual, changes, _activated) = submenu_window(true); + + let row = visual.debug_bounds("menu-item-1").unwrap(); + click( + &mut visual, + point( + row.origin.x + row.size.width / 2., + row.origin.y + row.size.height / 2., + ), + ); + draw(&mut visual); + + // The flyout's own padding belongs to the flyout, not to the dismissal + // listener of the panel behind it. + let panel = visual.debug_bounds("submenu-panel").unwrap(); + click( + &mut visual, + point( + panel.origin.x + px(4.), + panel.size.height + panel.origin.y - px(2.), + ), + ); + draw(&mut visual); + assert!(changes.borrow().is_empty(), "the flyout stays open"); + assert!(visual.debug_bounds("submenu-item-0").is_some()); + + // A click outside everything still closes the whole menu. + click(&mut visual, point(px(600.), px(600.))); + assert_eq!(*changes.borrow(), vec![false]); +} + +#[test] +fn hovering_a_submenu_row_opens_its_flyout_and_an_action_row_closes_it() { + let (_cx, mut visual, changes, _activated) = submenu_window(true); + + let export = visual.debug_bounds("menu-item-1").unwrap(); + hover(&mut visual, center(export)); + draw(&mut visual); + assert!( + visual.debug_bounds("submenu-item-0").is_some(), + "hover opens the flyout" + ); + assert!(changes.borrow().is_empty(), "the menu stays open"); + + let cut = visual.debug_bounds("menu-item-0").unwrap(); + hover(&mut visual, center(cut)); + draw(&mut visual); + assert!( + visual.debug_bounds("submenu-item-0").is_none(), + "hovering an action row closes the flyout" + ); + assert!(changes.borrow().is_empty(), "the menu stays open"); +} + +#[test] +fn the_flyout_overlaps_the_parent_from_the_rows_right_edge() { + let (_cx, mut visual, _changes, _activated) = submenu_window(true); + + let export = visual.debug_bounds("menu-item-1").unwrap(); + hover(&mut visual, center(export)); + draw(&mut visual); + + let parent = visual.debug_bounds("menu-panel").unwrap(); + let flyout = visual.debug_bounds("submenu-panel").unwrap(); + let parent_right = parent.origin.x + parent.size.width; + let row_right = export.origin.x + export.size.width; + // macOS style: the flyout covers the parent's trailing padding, starting + // at the row's right edge. + assert!( + flyout.origin.x < parent_right - px(1.), + "the flyout starts at {:?}, inside the parent edge at {:?}", + flyout.origin.x, + parent_right + ); + assert!( + flyout.origin.x >= row_right - px(1.), + "the flyout starts at {:?}, at or after the row's right edge {:?}", + flyout.origin.x, + row_right + ); +} + +#[test] +fn a_custom_min_width_overrides_the_default_panel_width() { + let (_cx, mut visual, _window, _changes, _activated) = menu_window(true); + let default_width = visual.debug_bounds("menu-panel").unwrap().size.width; + + let (_cx, mut visual, _window, _changes, _activated) = + menu_window_with(true, Some(px(320.).into())); + let width = visual.debug_bounds("menu-panel").unwrap().size.width; + // Painted bounds are snapped to device pixels; allow half a pixel. + assert!( + (width - px(320.)).abs() <= px(0.5), + "the panel is {width:?} wide, expected the 320px minimum" + ); + assert!( + width > default_width, + "the default panel is {:?} wide and should be narrower", + default_width + ); +}