Skip to content

Commit 3f1dfd1

Browse files
committed
menu: auto-wire parent_menu on submenu items and fix ESC dismiss
Two fixes for submenu dismiss propagation: 1. Auto-wire parent_menu at the start of PopupMenu::render(). Submenus added via PopupMenuItem::submenu() (the data constructor) don't get parent_menu set, breaking the dismiss chain. This ensures proper dismiss propagation regardless of how the submenu was constructed — including submenus built by table delegates which operate in Context<TableState>, not Context<PopupMenu>. 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 would hit the parent's dismiss(), which bailed out early because active_submenu() was Some. Now dismiss() clears selected_index first (closing the submenu) and proceeds with full dismissal. Fixes #2229
1 parent b63a12e commit 3f1dfd1

1 file changed

Lines changed: 36 additions & 7 deletions

File tree

crates/ui/src/menu/popup_menu.rs

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,7 @@ impl PopupMenu {
752752
self.dispatch_confirm_action(action, window, cx);
753753
}
754754

755-
self.dismiss(&Cancel, window, cx)
755+
self.dismiss_menu(window, cx)
756756
}
757757
Some(PopupMenuItem::ElementItem {
758758
handler, action, ..
@@ -762,7 +762,7 @@ impl PopupMenu {
762762
} else if let Some(action) = action.as_ref() {
763763
self.dispatch_confirm_action(action, window, cx);
764764
}
765-
self.dismiss(&Cancel, window, cx)
765+
self.dismiss_menu(window, cx)
766766
}
767767
_ => {}
768768
}
@@ -929,11 +929,17 @@ impl PopupMenu {
929929
}
930930
}
931931

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

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

939945
// Focus back to the previous focused handle.
@@ -948,7 +954,7 @@ impl PopupMenu {
948954
// Dismiss parent menu, when this menu is dismissed
949955
_ = parent_menu.update(cx, |view, cx| {
950956
view.selected_index = None;
951-
view.dismiss(&Cancel, window, cx);
957+
view.dismiss_menu(window, cx);
952958
});
953959
}
954960

@@ -967,7 +973,15 @@ impl PopupMenu {
967973
}
968974
}
969975

970-
self.dismiss(&Cancel, window, cx);
976+
// Do not dismiss if a submenu is active — the submenu handles its
977+
// own mouse-down-out. Without this guard, clicking a submenu item
978+
// would trigger the parent's on_mouse_down_out and tear down the
979+
// submenu before the item's on_click fires.
980+
if self.active_submenu().is_some() {
981+
return;
982+
}
983+
984+
self.dismiss_menu(window, cx);
971985
}
972986

973987
fn on_mouse_down_out(
@@ -1269,6 +1283,21 @@ struct RenderOptions {
12691283

12701284
impl Render for PopupMenu {
12711285
fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
1286+
// Auto-wire parent_menu on submenu children that were added via
1287+
// PopupMenuItem::submenu() (which doesn't set parent_menu, unlike
1288+
// PopupMenu::submenu() which does). This ensures the dismiss chain
1289+
// works regardless of how the submenu was constructed.
1290+
let parent = cx.entity().downgrade();
1291+
for item in &self.menu_items {
1292+
if let PopupMenuItem::Submenu { menu, .. } = item {
1293+
menu.update(cx, |sub, _| {
1294+
if sub.parent_menu.is_none() {
1295+
sub.parent_menu = Some(parent.clone());
1296+
}
1297+
});
1298+
}
1299+
}
1300+
12721301
self.update_submenu_menu_anchor(window);
12731302

12741303
let view = cx.entity().clone();

0 commit comments

Comments
 (0)