From 604529e929989b2b5d00e32092d0a37280e2560d Mon Sep 17 00:00:00 2001
From: Hazel Granados <hazel.granados@protonmail.com>
Date: Tue, 20 Oct 2026 10:28:33 -0500
Subject: [PATCH] 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.
---
 ROADMAP.md                                   |  2 ++
 crates/ui-egui/src/canvas.rs                 |  3 ++-
 crates/ui-egui/tests/ui_group/text_select.rs | 21 ++++++++++++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/ROADMAP.md b/ROADMAP.md
index b1283e775..cc88faa6b 100644
--- a/ROADMAP.md
+++ b/ROADMAP.md
@@ -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.
diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs
index eb9244a28..d83708aa9 100644
--- a/crates/ui-egui/src/canvas.rs
+++ b/crates/ui-egui/src/canvas.rs
@@ -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() {
diff --git a/crates/ui-egui/tests/ui_group/text_select.rs b/crates/ui-egui/tests/ui_group/text_select.rs
index 9ed4e9d5d..d3d40da8d 100644
--- a/crates/ui-egui/tests/ui_group/text_select.rs
+++ b/crates/ui-egui/tests/ui_group/text_select.rs
@@ -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(|_| {});