M3: let Escape close an open menu before it clears the selection (#3535)

The page's Escape handler runs in logic(), before the UI is drawn, and
ignored open popups, so one Escape closed the main menu and also cleared
the text selection. It now leaves Escape to an open popup, as
combine_ui.rs already does for its keys; the next Escape clears the
selection as before.
This commit is contained in:
Hazel Granados
2026-10-20 10:28:33 -05:00
parent 6fb6b7cf6f
commit 604529e929
3 changed files with 25 additions and 1 deletions
+2
View File
@@ -172,6 +172,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md
## Progress log
- **2026-10-20 (M3, #3535):** Escape with a menu open closes the menu and keeps the text selection; the next Escape clears it, as before. One keypress used to do both. A synthetic UI regression checks it. Estimates unchanged.
- **2026-10-19 (M14, #3373):** The main menu and its long submenus scroll in short windows, keeping their commands reachable. Estimates unchanged.
- **2026-10-19 (M3, #3387):** Save-prompt shortcuts yield to later consent and utility dialogs; a synthetic UI regression checks that an edited document stays open. Estimates unchanged.
+2 -1
View File
@@ -2285,7 +2285,8 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context, speed: crate::scroll_s
view.view_history(true);
}
let key = |k| ctx.input(|i| i.key_pressed(k));
if key(Key::Escape) {
// An open menu takes Escape first and closes; the selection stays for the next one (#3535).
if key(Key::Escape) && !egui::Popup::is_any_open(ctx) {
if view.camera.is_some() || view.camera_done.is_some() {
view.pending_action = Some(ViewAction::CancelCamera);
} else if view.selection.is_some() {
@@ -226,6 +226,27 @@ fn selected_text_copy_edit_and_context_menus() {
assert_eq!(h.state().views[0].selected_text().as_deref(), Some("quick"));
}
#[test]
fn escape_closes_an_open_menu_before_it_clears_the_selection() {
// #3535: one Escape closed the main menu and dropped the selection with it.
use egui_kittest::kittest::Queryable;
let (mut h, at) = harness(|_| {});
click(&mut h, at);
click(&mut h, at);
h.run_steps(2);
assert_eq!(h.state().views[0].selected_text().as_deref(), Some("quick"));
h.get_by_label("Menu").click();
h.run_steps(2);
assert!(egui::Popup::is_any_open(&h.ctx), "the menu is open");
h.key_press(egui::Key::Escape);
h.run_steps(2);
assert!(!egui::Popup::is_any_open(&h.ctx), "Escape closes the menu");
assert_eq!(h.state().views[0].selected_text().as_deref(), Some("quick"), "and keeps the selection");
h.key_press(egui::Key::Escape);
h.run_steps(2);
assert_eq!(h.state().views[0].selected_text(), None, "a second Escape clears it");
}
#[test]
fn selected_text_copy_command_requires_selection() {
let (mut h, at) = harness(|_| {});