From f9b35b894420edb242f9f0e352de75e7cd3a2f70 Mon Sep 17 00:00:00 2001 From: kirillDevPro <113171057+kirillDevPro@users.noreply.github.com> Date: Tue, 8 Sep 2026 23:39:43 +0200 Subject: [PATCH] fix(menu): open submenus on hover and fit them to the viewport --- .../src/moon/dropdown/popup.rs | 105 +++++++--- .../src/moon/dropdown/popup/submenu.rs | 100 +++++++++ .../src/moon/dropdown/popup/tests.rs | 190 ++++++++++++++++++ 3 files changed, 369 insertions(+), 26 deletions(-) create mode 100644 crates/moon-ui-components/src/moon/dropdown/popup/submenu.rs create mode 100644 crates/moon-ui-components/src/moon/dropdown/popup/tests.rs diff --git a/crates/moon-ui-components/src/moon/dropdown/popup.rs b/crates/moon-ui-components/src/moon/dropdown/popup.rs index a33a051..026a164 100644 --- a/crates/moon-ui-components/src/moon/dropdown/popup.rs +++ b/crates/moon-ui-components/src/moon/dropdown/popup.rs @@ -2,6 +2,9 @@ use super::*; +mod submenu; +use submenu::SubmenuPlacement; + /// Shared immutable inputs used to render every row in one menu level. struct MenuLevelRenderContext { menu_id: SharedString, @@ -14,6 +17,7 @@ struct MenuLevelRenderContext { palette: MoonPalette, tokens: MoonThemeTokens, dropdown_selection: Option>, + submenu_state: Option>>>, } /// Retained variable-height list state for one large popup-menu level. @@ -119,6 +123,7 @@ pub struct MoonPopupMenu { max_height: Option, mono: bool, dropdown_selection: Option>, + submenu_state: Option>>>, } #[derive(IntoElement)] @@ -149,6 +154,7 @@ impl MoonPopupMenu { max_height: None, mono: true, dropdown_selection: None, + submenu_state: None, } } @@ -399,9 +405,6 @@ impl MoonPopupMenu { matches!(self.width, MoonMenuWidth::Rendered(_)), "scaled or fitted menu widths require RenderOnce with an App context" ); - if !menu_level_is_virtualized(self.items.len()) { - return self.render_with_theme(p, MoonThemeTokens::default(), None, None); - } MoonPopupMenuResolvedTheme { menu: self, palette: p, @@ -512,8 +515,7 @@ impl MoonPopupMenu { } } }; - if self.width.is_measured() - && let Some(max_width) = self.rendered_max_width + if let Some(max_width) = self.rendered_max_width && width > max_width { width = max_width.max(1.0); @@ -579,6 +581,7 @@ impl MoonPopupMenu { palette: p, tokens, dropdown_selection: self.dropdown_selection, + submenu_state: self.submenu_state, }); if let Some(list_state) = virtual_list_state { let list_height = @@ -673,7 +676,16 @@ impl MoonPopupMenu { ); } - match item.kind { + let hovered_branch = context + .submenu_state + .as_ref() + .and_then(|state| cx.and_then(|cx| *state.read(cx))); + let open = !item.disabled + && !item.submenu.items.is_empty() + && hovered_branch.map_or(item.selected, |branch| branch == Some(ix)); + let hover_state = context.submenu_state.clone(); + let hover_branch = (!item.disabled && !item.submenu.items.is_empty()).then_some(ix); + let row = match item.kind { MoonMenuItemKind::Separator => div() .id(ElementId::from(row_id.clone())) .debug_selector(move || row_id.to_string()) @@ -746,7 +758,7 @@ impl MoonPopupMenu { } MoonMenuItemKind::Item => { let disabled = item.disabled; - let selected = item.selected; + let selected = item.selected || open; let checked = item.checked; let on_click = menu_item_click_handler(&item, context.dropdown_selection.as_ref()); let submenu = item.submenu; @@ -842,30 +854,61 @@ impl MoonPopupMenu { } } - if selected && has_submenu { + if open { + let row_bounds = std::rc::Rc::new(std::cell::Cell::new(Bounds::default())); + let capture = row_bounds.clone(); + row = row.child( + canvas(move |bounds, _, _| capture.set(bounds), |_, _, _, _| {}) + .absolute() + .inset_0() + .size_full(), + ); row = row.child( - deferred( - div() - .absolute() - .left_full() - .ml(px(tokens.ui(SUBMENU_OFFSET_X))) - .top(px(-tokens.ui(MENU_PADDING))) - .child(MoonPopupMenuResolvedTheme { - menu: MoonPopupMenu::new(format!("{menu_id}:submenu:{ix}")) - .shared_level(submenu) - .width_policy(menu_width_policy) - .size(menu_size), - palette: p, - tokens: tokens.clone(), - }), - ) + deferred(SubmenuPlacement { + row_bounds, + gap: tokens.ui(SUBMENU_OFFSET_X), + top_overlap: tokens.ui(MENU_PADDING), + child: MoonPopupMenuResolvedTheme { + menu: MoonPopupMenu::new(format!("{menu_id}:submenu:{ix}")) + .shared_level(submenu) + .width_policy(menu_width_policy) + .size(menu_size), + palette: p, + tokens: tokens.clone(), + } + .into_any_element(), + }) .with_priority(1), ); } row.into_any_element() } - } + }; + div() + .id(SharedString::from(format!("{menu_id}:hover:{ix}"))) + .on_hover(move |hovered, window, cx| { + if *hovered && let Some(state) = &hover_state { + state.update(cx, |state, cx| { + if *state != Some(hover_branch) { + *state = Some(hover_branch); + cx.notify(); + window.refresh(); + } + }); + } + }) + .child(row) + .into_any_element() + } + + /// Retain one hovered branch per menu level; leaving a row keeps its child reachable. + fn retain_submenu_state(&mut self, window: &mut Window, cx: &mut App) { + self.submenu_state = Some(window.use_keyed_state( + ElementId::from(SharedString::from(format!("{}:hovered-branch", self.id))), + cx, + |_, _| None, + )); } /// Return unscaled row metrics for the configured menu size. @@ -886,7 +929,8 @@ impl RenderOnce for MoonPopupMenu { /// /// Returns: /// The rendered menu. - fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { + fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement { + self.retain_submenu_state(window, cx); let tokens = MoonTheme::active_tokens(cx); let virtual_list_state = self.retained_virtual_list_state(&tokens, window, cx); self.render_with_theme( @@ -907,7 +951,13 @@ impl RenderOnce for MoonPopupMenuResolvedTheme { /// /// Returns: /// The popup rendered with the inherited palette and theme tokens. - fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { + fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement { + self.menu.retain_submenu_state(window, cx); + let viewport = window.viewport_size(); + self.menu.rendered_max_width = Some((f32::from(viewport.width) - 12.0).max(1.0)); + let viewport_height = (f32::from(viewport.height) - 12.0).max(1.0); + let cap = resolve_menu_outer_max(self.menu.max_height, &self.tokens, false); + self.menu.max_height = Some(MoonMenuMaxHeight::Rendered(cap.min(viewport_height))); let virtual_list_state = self .menu .retained_virtual_list_state(&self.tokens, window, cx); @@ -915,3 +965,6 @@ impl RenderOnce for MoonPopupMenuResolvedTheme { .render_with_theme(self.palette, self.tokens, Some(cx), virtual_list_state) } } + +#[cfg(test)] +mod tests; diff --git a/crates/moon-ui-components/src/moon/dropdown/popup/submenu.rs b/crates/moon-ui-components/src/moon/dropdown/popup/submenu.rs new file mode 100644 index 0000000..3a9c556 --- /dev/null +++ b/crates/moon-ui-components/src/moon/dropdown/popup/submenu.rs @@ -0,0 +1,100 @@ +//! Place deferred submenu content beside its measured parent row within the viewport. + +use gpui::*; +use std::{cell::Cell, rc::Rc}; + +/// A deferred portal whose parent-row bounds are captured before its prepaint pass. +pub(super) struct SubmenuPlacement { + pub row_bounds: Rc>>, + pub gap: f32, + pub top_overlap: f32, + pub child: AnyElement, +} + +impl IntoElement for SubmenuPlacement { + type Element = Self; + + /// Preserve the portal as a layout element. + fn into_element(self) -> Self { + self + } +} + +impl Element for SubmenuPlacement { + type RequestLayoutState = LayoutId; + type PrepaintState = (); + + /// The surrounding menu owns retained identity. + fn id(&self) -> Option { + None + } + + /// Internal geometry has no separate inspector source. + fn source_location(&self) -> Option<&'static std::panic::Location<'static>> { + None + } + + /// Measure the menu outside normal row flow before choosing its opening side. + fn request_layout( + &mut self, + _: Option<&GlobalElementId>, + _: Option<&InspectorElementId>, + window: &mut Window, + cx: &mut App, + ) -> (LayoutId, LayoutId) { + let child = self.child.request_layout(window, cx); + let layout = window.request_layout( + Style { + position: Position::Absolute, + ..Style::default() + }, + [child], + cx, + ); + (layout, child) + } + + /// Prefer the row's right side, flip to its left, then clamp both axes to the viewport. + fn prepaint( + &mut self, + _: Option<&GlobalElementId>, + _: Option<&InspectorElementId>, + bounds: Bounds, + child: &mut LayoutId, + window: &mut Window, + cx: &mut App, + ) { + let row = self.row_bounds.get(); + let size = window.layout_bounds(*child).size; + let viewport = window.viewport_size(); + let margin = px(6.0); + let right = row.right() + px(self.gap); + let left = row.left() - px(self.gap) - size.width; + let x = if right + size.width <= viewport.width - margin { + right + } else { + left + }; + let x = x.min(viewport.width - size.width - margin).max(margin); + let y = (row.top() - px(self.top_overlap)) + .min(viewport.height - size.height - margin) + .max(px(0.0)); + window.with_element_offset(point(x, y) - bounds.origin, |window| { + self.child.prepaint(window, cx) + }); + } + + /// Paint at the position retained by the child's prepaint pass. + fn paint( + &mut self, + _: Option<&GlobalElementId>, + _: Option<&InspectorElementId>, + _: Bounds, + _: &mut LayoutId, + _: &mut (), + window: &mut Window, + cx: &mut App, + ) { + self.child.paint(window, cx); + } +} diff --git a/crates/moon-ui-components/src/moon/dropdown/popup/tests.rs b/crates/moon-ui-components/src/moon/dropdown/popup/tests.rs new file mode 100644 index 0000000..de52f4a --- /dev/null +++ b/crates/moon-ui-components/src/moon/dropdown/popup/tests.rs @@ -0,0 +1,190 @@ +//! Input and rendered-geometry regressions for cascading menus. + +use super::{MoonMenuItem, MoonPopupMenu}; +use crate::moon::{MoonScale, MoonTheme, ThemeMode}; +use gpui::{ParentElement as _, Styled as _}; +use std::{cell::Cell, rc::Rc}; + +/// Place two branches and a disabled branch near the requested viewport corner. +struct CascadeHarness { + edge: bool, + count: usize, + parent_count: usize, + calls: Rc>, +} + +impl gpui::Render for CascadeHarness { + /// Build real nested rows whose callbacks expose accidental hover activation. + fn render( + &mut self, + window: &mut gpui::Window, + _: &mut gpui::Context, + ) -> impl gpui::IntoElement { + let calls = self.calls.clone(); + let pos = if self.edge { + gpui::point( + window.viewport_size().width - gpui::px(205.0), + window.viewport_size().height - gpui::px(145.0), + ) + } else { + gpui::point(gpui::px(30.0), gpui::px(30.0)) + }; + gpui::div().size_full().child( + gpui::div().absolute().left(pos.x).top(pos.y).child( + MoonPopupMenu::new("cascade") + .width(180.0) + .items([ + MoonMenuItem::new("First").submenu((0..self.count).map(|ix| { + let calls = calls.clone(); + MoonMenuItem::new(format!("Action {ix}")) + .on_click(move |_, _, _| calls.set(calls.get() + 1)) + })), + MoonMenuItem::new("Second") + .submenu([MoonMenuItem::new("Nested") + .submenu([MoonMenuItem::new("Deep action")])]), + MoonMenuItem::new("Disabled") + .disabled(true) + .submenu([MoonMenuItem::new("Never")]), + MoonMenuItem::new("Ordinary"), + ]) + .items( + (4..self.parent_count).map(|ix| MoonMenuItem::new(format!("Parent {ix}"))), + ), + ), + ) + } +} + +/// Catches restoring selected-only expansion or clearing a branch on row leave: hovering must open +/// without invoking actions, crossing into its child must preserve it, and siblings must replace it. +#[gpui::test] +fn submenu_hover_switches_without_clicking_and_preserves_child_access( + cx: &mut gpui::TestAppContext, +) { + cx.update(crate::init); + for (theme, parent_count) in [ + (ThemeMode::Dark, 4), + (ThemeMode::Light, 4), + (ThemeMode::Dark, 100), + (ThemeMode::Light, 100), + ] { + cx.update(|cx| MoonTheme::set_mode(theme, cx)); + let calls = Rc::new(Cell::new(0)); + let sink = calls.clone(); + let window = cx.add_window(move |_, _| CascadeHarness { + edge: false, + count: 3, + parent_count, + calls: sink, + }); + let mut view = gpui::VisualTestContext::from_window(window.into(), cx); + view.run_until_parked(); + assert!(view.debug_bounds("cascade:submenu:0").is_none()); + let first = view.debug_bounds("cascade:item:0").unwrap(); + view.simulate_mouse_move(first.center(), None, gpui::Modifiers::none()); + view.run_until_parked(); + let child = view + .debug_bounds("cascade:submenu:0:item:0") + .expect("hover must open the first submenu"); + assert_eq!(calls.get(), 0, "hover must not execute a leaf action"); + view.simulate_mouse_move(child.center(), None, gpui::Modifiers::none()); + view.run_until_parked(); + assert!( + view.debug_bounds("cascade:submenu:0:item:0").is_some(), + "moving into the child must retain the branch" + ); + view.simulate_click(child.center(), gpui::Modifiers::none()); + assert_eq!( + calls.get(), + 1, + "the child outside the parent must remain clickable exactly once" + ); + for ix in [1, 2, 0, 3] { + let row = view + .debug_bounds( + [ + "cascade:item:0", + "cascade:item:1", + "cascade:item:2", + "cascade:item:3", + ][ix], + ) + .unwrap(); + view.simulate_mouse_move(row.center(), None, gpui::Modifiers::none()); + view.run_until_parked(); + assert_eq!( + view.debug_bounds("cascade:submenu:0").is_some(), + ix == 0, + "old branch must close on sibling hover" + ); + assert_eq!( + view.debug_bounds("cascade:submenu:1").is_some(), + ix == 1, + "enabled sibling must open automatically" + ); + assert!( + view.debug_bounds("cascade:submenu:2").is_none(), + "disabled branches must stay closed" + ); + } + } +} + +/// Catches removing the side flip, viewport height cap, or prepaint translation: eager and virtual +/// submenus must fit near both edges at enlarged UI scale, including a third menu level. +#[gpui::test] +fn submenu_flips_left_and_caps_height_at_viewport_edges(cx: &mut gpui::TestAppContext) { + cx.update(crate::init); + for theme in [ThemeMode::Dark, ThemeMode::Light] { + for count in [12, 100] { + cx.update(|cx| { + MoonTheme::set_mode(theme, cx); + MoonTheme::global_mut(cx).scale = MoonScale { + ui: 1.25, + font: 1.0, + font_delta: 2.0, + }; + }); + let window = cx.add_window(move |_, _| CascadeHarness { + edge: true, + count, + parent_count: 4, + calls: Rc::new(Cell::new(0)), + }); + let mut view = gpui::VisualTestContext::from_window(window.into(), cx); + view.run_until_parked(); + let viewport = view.update(|window, _| window.viewport_size()); + let first = view.debug_bounds("cascade:item:0").unwrap(); + view.simulate_mouse_move(first.center(), None, gpui::Modifiers::none()); + view.run_until_parked(); + let child = view + .debug_bounds("cascade:submenu:0") + .expect("edge submenu must open on hover"); + assert!( + child.right() <= first.left(), + "submenu must flip to the parent's left at the right edge: row={first:?} child={child:?} viewport={viewport:?}" + ); + assert!( + child.left() >= gpui::px(6.0) && child.right() <= viewport.width - gpui::px(6.0), + "submenu must fit horizontally" + ); + assert!( + child.top() >= gpui::px(6.0) && child.bottom() <= viewport.height - gpui::px(6.0), + "submenu must fit vertically" + ); + let second = view.debug_bounds("cascade:item:1").unwrap(); + view.simulate_mouse_move(second.center(), None, gpui::Modifiers::none()); + view.run_until_parked(); + let nested = view.debug_bounds("cascade:submenu:1:item:0").unwrap(); + view.simulate_mouse_move(nested.center(), None, gpui::Modifiers::none()); + view.run_until_parked(); + let deep = view + .debug_bounds("cascade:submenu:1:submenu:0") + .expect("third level must also open by hover"); + assert!(deep.left() >= gpui::px(6.0) && deep.right() <= viewport.width - gpui::px(6.0)); + assert!( + deep.top() >= gpui::px(6.0) && deep.bottom() <= viewport.height - gpui::px(6.0) + ); + } + } +}