diff --git a/crates/egui/src/containers/menu.rs b/crates/egui/src/containers/menu.rs index 1bd5954c8..cfdaac827 100644 --- a/crates/egui/src/containers/menu.rs +++ b/crates/egui/src/containers/menu.rs @@ -10,7 +10,7 @@ use crate::style::StyleModifier; use crate::{ - Button, Color32, Context, Frame, Id, InnerResponse, IntoAtoms, Layout, Popup, + Button, Color32, Context, Frame, Id, InnerResponse, IntoAtoms, Layout, PointerButton, Popup, PopupCloseBehavior, Response, Style, Ui, UiBuilder, UiKind, UiStack, UiStackInfo, Widget as _, }; use emath::{Align, RectAlign, Vec2, vec2}; @@ -458,6 +458,7 @@ impl SubMenu { let is_any_open = open_item.is_some(); let mut is_open = open_item == Some(id); + let was_open = is_open; let mut set_open = None; // We expand the button rect so there is no empty space where no menu is shown @@ -470,9 +471,21 @@ impl SubMenu { // But since we check if no other menu is open, nothing should be able to cover the button let is_hovered = hover_pos.is_some_and(|pos| button_rect.contains(pos)); + // `clicked` includes keyboard and accessibility click actions. + // We want Enter/Space to toggle an already open submenu, while pointer clicks should keep + // the submenu open (for touch and pointer interactions). + let clicked = button_response.clicked(); + let clicked_by_pointer = button_response.clicked_by(PointerButton::Primary); + let clicked_by_keyboard_or_access = clicked && !clicked_by_pointer; + + if ui.is_enabled() && is_open && clicked_by_keyboard_or_access { + set_open = Some(false); + is_open = false; + } + // The clicked handler is there for accessibility (keyboard navigation) let should_open = - ui.is_enabled() && (button_response.clicked() || (is_hovered && !is_any_open)); + ui.is_enabled() && ((!was_open && clicked) || (is_hovered && !is_any_open)); if should_open { set_open = Some(true); is_open = true; diff --git a/crates/egui_kittest/tests/regression_tests.rs b/crates/egui_kittest/tests/regression_tests.rs index 4289fe3a9..94617ff8b 100644 --- a/crates/egui_kittest/tests/regression_tests.rs +++ b/crates/egui_kittest/tests/regression_tests.rs @@ -262,3 +262,101 @@ pub fn menus_should_close_even_if_submenu_disappears() { ); } } + +fn keyboard_submenu_harness() -> Harness<'static, bool> { + Harness::builder() + .with_size(Vec2::new(400.0, 240.0)) + .build_ui_state( + |ui, checked| { + egui::Panel::top("menu_bar").show_inside(ui, |ui| { + egui::MenuBar::new().ui(ui, |ui| { + ui.menu_button("X", |ui| { + ui.menu_button("Y", |ui| { + ui.checkbox(checked, "Goal"); + }); + }); + }); + }); + }, + false, + ) +} + +#[test] +pub fn keyboard_should_open_nested_submenu() { + let mut harness = keyboard_submenu_harness(); + + harness.get_by_label("X").focus(); + harness.run(); + + harness.key_press(egui::Key::Enter); + harness.run(); + + harness.get_by_label_contains("Y").focus(); + harness.run(); + + harness.key_press(egui::Key::Enter); + harness.run(); + + assert!( + harness.query_by_label("Goal").is_some(), + "Expected nested submenu to open via keyboard" + ); +} + +#[test] +pub fn keyboard_should_close_nested_submenu_with_second_enter() { + let mut harness = keyboard_submenu_harness(); + + harness.get_by_label("X").focus(); + harness.run(); + + harness.key_press(egui::Key::Enter); + harness.run(); + + harness.get_by_label_contains("Y").focus(); + harness.run(); + + harness.key_press(egui::Key::Enter); + harness.run(); + + assert!( + harness.query_by_label("Goal").is_some(), + "Expected nested submenu to open before close attempt" + ); + + harness.get_by_label_contains("Y").focus(); + harness.run(); + + harness.key_press(egui::Key::Enter); + harness.run(); + + assert!( + harness.query_by_label("Goal").is_none(), + "Expected nested submenu to close when pressing Enter again" + ); +} + +#[test] +pub fn pointer_click_on_open_submenu_button_should_not_close_it() { + let mut harness = keyboard_submenu_harness(); + + harness.get_by_label("X").click(); + harness.run(); + + harness.get_by_label_contains("Y").click(); + harness.run(); + + assert!( + harness.query_by_label("Goal").is_some(), + "Expected submenu to remain open after pointer click on its button" + ); + + harness.get_by_label_contains("Y").click(); + harness.run(); + + assert!( + harness.query_by_label("Goal").is_some(), + "Expected submenu to remain open on repeated pointer click" + ); +}