mirror of
https://github.com/emilk/egui.git
synced 2026-06-26 22:53:14 -04:00
Fix menu keyboard toggle for open submenus (#7957)
Summary This PR fixes submenu keyboard parity: pressing Enter/Space on an already-open submenu button now collapses that submenu (matching top-level menu button behavior). What changed Updated submenu interaction logic to distinguish pointer primary clicks from keyboard/accessibility-triggered clicks. Kept pointer/touch behavior unchanged (submenu button clicks still don’t auto-close submenu). Added regression tests for: keyboard open of nested submenu, keyboard close (second Enter) of nested submenu, pointer clicks on submenu button keeping submenu open. Validation cargo test -p egui_kittest --test regression_tests Breaking changes None. Behavior change is limited to keyboard/accessibility activation of already-open submenu buttons. * Closes <https://github.com/emilk/egui/issues/7926> * [ X ] I have followed the instructions in the PR template --------- Co-authored-by: Lucas Meurer <hi@lucasmerlin.me>
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user