Skip to content

Commit 6f60e63

Browse files
committed
menu: wire parent_menu at build time and fix ESC dismiss
Three related fixes for submenu dismiss behavior in PopupMenu: 1. Wire parent_menu in PopupMenu::build (one-shot at construction). Submenus added via PopupMenuItem::submenu(label, entity) — the data constructor — can't reach the parent's entity at build time, so they land with parent_menu = None. After the user closure populates the menu, iterate menu_items once and wire any orphan submenus to point at the menu being built. Zero per-frame cost, handles arbitrary nesting because every submenu also goes through PopupMenu::build. This was previously done in Render::render which iterated and notified every frame — replaced with the one-shot approach. 2. Fix ESC not closing menus when a submenu is open. When a submenu is opened by hover, the parent menu retains focus. Pressing ESC hit the parent's dismiss(), which bailed out early because active_submenu() was Some. Split dismiss into dismiss (Cancel handler — clears the active submenu first) and dismiss_menu (internal — propagates up the chain). 3. Guard handle_dismiss for submenu clicks. handle_dismiss (click-outside) now returns early if a submenu is active. Without this, clicking a submenu item triggers the parent's on_mouse_down_out, tearing down the submenu before the item's on_click fires. Fixes #2229
1 parent 9e252d8 commit 6f60e63

1 file changed

Lines changed: 39 additions & 8 deletions

File tree

crates/ui/src/menu/popup_menu.rs

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,24 @@ impl PopupMenu {
321321
cx: &mut App,
322322
f: impl FnOnce(Self, &mut Window, &mut Context<PopupMenu>) -> Self,
323323
) -> Entity<Self> {
324-
cx.new(|cx| f(Self::new(cx), window, cx))
324+
cx.new(|cx| {
325+
let menu = f(Self::new(cx), window, cx);
326+
// Wire parent_menu on any submenu children that were added via
327+
// the PopupMenuItem::submenu(label, entity) data constructor —
328+
// unlike PopupMenu::submenu() (the builder), it can't reach the
329+
// parent's entity at construction time. Runs once per menu build.
330+
let parent = cx.entity().downgrade();
331+
for item in &menu.menu_items {
332+
if let PopupMenuItem::Submenu { menu: sub, .. } = item {
333+
sub.update(cx, |sub, _| {
334+
if sub.parent_menu.is_none() {
335+
sub.parent_menu = Some(parent.clone());
336+
}
337+
});
338+
}
339+
}
340+
menu
341+
})
325342
}
326343

327344
/// Set the focus handle of Entity to handle actions.
@@ -753,7 +770,7 @@ impl PopupMenu {
753770
self.dispatch_confirm_action(action, window, cx);
754771
}
755772

756-
self.dismiss(&Cancel, window, cx)
773+
self.dismiss_menu(window, cx)
757774
}
758775
Some(PopupMenuItem::ElementItem {
759776
handler, action, ..
@@ -763,7 +780,7 @@ impl PopupMenu {
763780
} else if let Some(action) = action.as_ref() {
764781
self.dispatch_confirm_action(action, window, cx);
765782
}
766-
self.dismiss(&Cancel, window, cx)
783+
self.dismiss_menu(window, cx)
767784
}
768785
_ => {}
769786
}
@@ -932,11 +949,17 @@ impl PopupMenu {
932949
}
933950
}
934951

952+
/// Cancel/ESC action handler. Closes any active submenu first, then
953+
/// dismisses the entire menu chain. This ensures ESC works even when a
954+
/// hover-opened submenu is visible and the parent retains focus.
935955
fn dismiss(&mut self, _: &Cancel, window: &mut Window, cx: &mut Context<Self>) {
936-
if self.active_submenu().is_some() {
937-
return;
938-
}
956+
self.selected_index = None;
957+
self.dismiss_menu(window, cx);
958+
}
939959

960+
/// Internal dismiss: emits DismissEvent and propagates up the parent chain.
961+
/// Used by `confirm` (item click) and `handle_dismiss` (click-outside).
962+
fn dismiss_menu(&mut self, window: &mut Window, cx: &mut Context<Self>) {
940963
cx.emit(DismissEvent);
941964

942965
// Focus back to the previous focused handle.
@@ -951,7 +974,7 @@ impl PopupMenu {
951974
// Dismiss parent menu, when this menu is dismissed
952975
_ = parent_menu.update(cx, |view, cx| {
953976
view.selected_index = None;
954-
view.dismiss(&Cancel, window, cx);
977+
view.dismiss_menu(window, cx);
955978
});
956979
}
957980

@@ -970,7 +993,15 @@ impl PopupMenu {
970993
}
971994
}
972995

973-
self.dismiss(&Cancel, window, cx);
996+
// Do not dismiss if a submenu is active — the submenu handles its
997+
// own mouse-down-out. Without this guard, clicking a submenu item
998+
// would trigger the parent's on_mouse_down_out and tear down the
999+
// submenu before the item's on_click fires.
1000+
if self.active_submenu().is_some() {
1001+
return;
1002+
}
1003+
1004+
self.dismiss_menu(window, cx);
9741005
}
9751006

9761007
fn on_mouse_down_out(

0 commit comments

Comments
 (0)