Merge pull request #5568 from dotCooCoo/fix/held-escape-after-tools-cancel

Shortcuts: a held Escape that cancels a Tools or ruler move skips the tool

crates/ui-egui/src/shortcuts.rs

@@ -219,8 +219,9 @@ pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context) {
         presentation_keys(app, ctx, escape_pair);
         return;
     }
-    // Escape that cancels a ruler-origin drag or floating Tools move skips the tool and later Escape actions.
-    let escape_ended_gesture = crate::canvas::ruler_origin_dragging(ctx) || crate::tools_float::escape_cancelled_move(ctx);
+    // Escape that cancels a ruler-origin drag or floating Tools move skips the tool and later Escape actions,
+    // and so do its repeats while it is held.
+    let escape_ended_gesture = escape_ended_gesture(ctx);
     let escape_held_by_drag = crate::canvas::dragging(ctx) || escape_ended_gesture;
     let typing = ctx.egui_wants_keyboard_input();
     // Escape closes Edit Toolbar's drawer first (egui drops a drag from it on Escape already).
@@ -368,6 +369,33 @@ pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context) {
     single_key_shortcuts(app, ctx);
 }
 
+/// The key of the mark that the Escape held down ended a ruler-origin drag or a floating Tools move.
+fn escape_ended_gesture_id() -> egui::Id {
+    egui::Id::unique("escape-ended-gesture")
+}
+
+/// Does this frame's Escape end a ruler-origin drag or a floating Tools move? The repeats of an
+/// Escape that ended one do too, up to the frame that releases Escape or presses it again.
+fn escape_ended_gesture(ctx: &egui::Context) -> bool {
+    let (down, pressed, repeated) = ctx.input(|i| {
+        let escape =
+            |repeat: bool| i.events.iter().any(|e| matches!(e, egui::Event::Key { key: Key::Escape, pressed: true, repeat: r, .. } if *r == repeat));
+        (i.key_down(Key::Escape), escape(false), escape(true))
+    });
+    if crate::canvas::ruler_origin_dragging(ctx) || crate::tools_float::escape_cancelled_move(ctx) {
+        if down {
+            ctx.data_mut(|d| d.insert_temp(escape_ended_gesture_id(), true));
+        }
+        return true;
+    }
+    // A release in the same frame as a repeat still holds that repeat back.
+    let held = (down || repeated) && !pressed && ctx.data(|d| d.get_temp::<bool>(escape_ended_gesture_id())).is_some();
+    if !(held && down) {
+        ctx.data_mut(|d| d.remove::<bool>(escape_ended_gesture_id()));
+    }
+    held
+}
+
 fn remember_escape_pair(ctx: &egui::Context, key: Key, escape_pair: Option<bool>) {
     if key == Key::Escape && escape_pair.is_some() && !egui::Popup::is_any_open(ctx) {
         let now = ctx.input(|i| i.time);

crates/ui-egui/src/tests/screenmode.rs

@@ -416,6 +416,55 @@ fn cancelling_ruler_origin_drag_keeps_the_tools_escape() {
     }
 }
 
+/// An Escape event: a press (a repeat when `repeat`) or a release.
+fn escape_event(pressed: bool, repeat: bool) -> Vec<egui::Event> {
+    vec![egui::Event::Key { key: egui::Key::Escape, physical_key: None, pressed, repeat, modifiers: Default::default() }]
+}
+
+#[test]
+fn held_escape_that_cancels_tools_move_keeps_the_tool() {
+    for mode in [0, 1, 2] {
+        let ctx = egui::Context::default();
+        let mut app = in_screen_mode(&ctx, mode);
+        app.select_tool("intertwine");
+        let (original, to) = begin_tools_move(&mut app, &ctx);
+        frame(&mut app, &ctx, escape_event(true, false));
+        assert_eq!(app.ui.toolbar_pos, original, "move canceled in mode {mode}");
+        for _ in 0..3 {
+            frame(&mut app, &ctx, escape_event(true, true));
+        }
+        assert_eq!(app.session.tool_id(), "intertwine", "held Escape keeps the tool in mode {mode}");
+        release_drag(&mut app, &ctx, to);
+        frame(&mut app, &ctx, escape_event(true, true));
+        assert_eq!(app.session.tool_id(), "intertwine", "held Escape keeps the tool after the release in mode {mode}");
+        assert_eq!(app.ui.screen_mode, mode, "held Escape keeps the screen mode {mode}");
+        frame(&mut app, &ctx, [escape_event(true, true), escape_event(false, false)].concat());
+        assert_eq!(app.session.tool_id(), "intertwine", "a repeat with the release keeps the tool in mode {mode}");
+        frame(&mut app, &ctx, escape());
+        assert_eq!(app.session.tool_id(), "selection", "the next Escape reaches the tool in mode {mode}");
+    }
+}
+
+#[test]
+fn held_escape_that_cancels_ruler_origin_drag_keeps_the_tool() {
+    for mode in [0, 1, 2] {
+        let ctx = egui::Context::default();
+        let mut app = in_screen_mode(&ctx, mode);
+        app.select_tool("intertwine");
+        let to = begin_ruler_origin_drag(&mut app, &ctx);
+        frame(&mut app, &ctx, escape_event(true, false));
+        release_drag(&mut app, &ctx, to);
+        assert!(!crate::canvas::ruler_origin_dragging(&ctx));
+        for _ in 0..3 {
+            frame(&mut app, &ctx, escape_event(true, true));
+        }
+        assert_eq!(app.session.tool_id(), "intertwine", "held Escape keeps the tool in mode {mode}");
+        frame(&mut app, &ctx, escape_event(false, false));
+        frame(&mut app, &ctx, escape());
+        assert_eq!(app.session.tool_id(), "selection", "the next Escape reaches the tool in mode {mode}");
+    }
+}
+
 #[test]
 fn a_popup_takes_escape_before_full_screen_exits() {
     let ctx = egui::Context::default();