Merge PR #772

crates/ui-egui/src/lib.rs

@@ -44,6 +44,8 @@ mod tests_album_picker;
 #[cfg(test)]
 mod tests_crop_rotate;
 #[cfg(test)]
+mod tests_crop_tool;
+#[cfg(test)]
 mod tests_curve;
 #[cfg(test)]
 mod tests_date_picker;
@@ -232,6 +234,9 @@ pub struct LightcraftApp {
     /// The keyboard shortcuts editor is waiting for a key press for this command: no shortcut
     /// fires (the native menu bar drops its accelerators too) until it gets one or is cancelled.
     pub recording_shortcut: Option<String>,
+    /// A text field had the keyboard at the end of the last frame. egui drops the focus as Esc
+    /// arrives, before the shortcuts are read, so this is how Esc there stays the field's own.
+    pub text_focus: bool,
     /// The host is [`headless::Headless`] (it answers viewport screenshot commands itself).
     pub headless_host: bool,
     /// Warnings to show one at a time (damaged settings files…, issue #103).
@@ -344,6 +349,7 @@ impl LightcraftApp {
             native_menu: false,
             native_shortcuts: Default::default(),
             recording_shortcut: None,
+            text_focus: false,
             headless_host: false,
             notices: vec![],
             quit_prompt: None,
@@ -1076,6 +1082,7 @@ impl LightcraftApp {
         panels::keyword_list::drag_feedback(self, &ctx);
         panels::toast(self, &ctx);
         self.widgets = widgets::take_registry(&ctx);
+        self.text_focus = ctx.egui_wants_keyboard_input();
         self.end_frame(&ctx, t0);
     }
 }

crates/ui-egui/src/menus.rs

@@ -406,6 +406,9 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option<Re
                 app.ui.slideshow = None;
             } else if !app.ui.tool.is_empty() {
                 app.ui.tool.clear();
+            } else if app.ui.right == RightPanel::Crop && app.ui.view == ViewMode::Detail {
+                // Esc leaves the crop tool (the crop stays), as C does; the next Esc goes back
+                panel(app, &ctx, RightPanel::Crop, "Crop, Rotate, Geometry");
             } else if app.ui.view == ViewMode::People && app.ui.person_page.is_some() {
                 app.ui.person_page = None;
             } else if matches!(app.ui.view, ViewMode::Compare | ViewMode::Survey) {

crates/ui-egui/src/shortcuts.rs

@@ -344,6 +344,8 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) {
     // an open popup (a menu, a date picker's calendar) closes on Esc itself: Esc's command (Back,
     // which also closes dialogs) waits until nothing is open
     let popup_open = egui::Popup::is_any_open(ctx);
+    // Esc that just took a text field's focus away (it gives back the field's text) does nothing else
+    let popup_open = popup_open || app.text_focus;
     ctx.input(|i| {
         for b in bindable() {
             if let Some(sc) = binding(keymap, b.id, b.default)

crates/ui-egui/src/tests_crop_tool.rs

@@ -0,0 +1,64 @@
+//! Headless tests of leaving the crop tool.
+
+use std::time::Duration;
+
+use serde_json::json;
+
+use crate::headless::Headless;
+use crate::state::{RightPanel, ViewMode};
+use crate::{LightcraftApp, Services};
+
+const T: Duration = Duration::from_secs(20);
+const SETTLE: Duration = Duration::from_secs(120);
+
+/// Esc in the crop tool closes the tool and keeps the photo in Detail (and the crop); only the
+/// next Esc goes back to the grid. It used to jump straight to the grid.
+#[test]
+fn escape_leaves_the_crop_tool_before_the_view() {
+    let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() });
+    let mut h = Headless::new(app, [1400.0, 900.0], 1.0);
+    let id = h.app.session.visible_cloned()[1];
+    h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [id.0], "active": id.0}}), T);
+    h.app.ui.view = ViewMode::Detail;
+    let r = h.request("engine.execute", json!({"command": "panel.crop"}), T);
+    assert_eq!(r["ok"], true, "{r}");
+    let r = h.request("engine.execute", json!({"command": "crop.set", "params": {"rect": [0.1, 0.1, 0.8, 0.9]}}), T);
+    assert_eq!(r["ok"], true, "{r}");
+    h.settle(SETTLE);
+    assert_eq!(h.app.ui.right, RightPanel::Crop);
+    let crop = h.app.session.catalog.photo(id).map(|p| p.develop.crop.geometry.rect);
+    h.request("ui.key", json!({"key": "Escape"}), T);
+    h.settle(SETTLE);
+    assert_ne!(h.app.ui.right, RightPanel::Crop, "Esc closes the crop tool");
+    assert_eq!(h.app.ui.view, ViewMode::Detail, "and stays on the photo");
+    assert_eq!(h.app.session.catalog.photo(id).map(|p| p.develop.crop.geometry.rect), crop, "the crop is kept");
+    h.request("ui.key", json!({"key": "Escape"}), T);
+    h.settle(SETTLE);
+    assert_eq!(h.app.ui.view, ViewMode::PhotoGrid, "the next Esc goes back to the grid");
+}
+
+/// Esc in the crop tool's Angle field only leaves the field: egui drops the field's focus as the
+/// key arrives, so Esc used to run Back as well (to the grid; with the crop tool's own Esc step,
+/// it would close the tool).
+#[test]
+fn escape_in_the_angle_field_only_leaves_the_field() {
+    let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() });
+    let mut h = Headless::new(app, [1400.0, 900.0], 1.0);
+    let id = h.app.session.visible_cloned()[1];
+    h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [id.0], "active": id.0}}), T);
+    h.app.ui.view = ViewMode::Detail;
+    h.request("engine.execute", json!({"command": "panel.crop"}), T);
+    h.settle(SETTLE);
+    let r = h.request("ui.clickWidget", json!({"id": "cropAngleField"}), T);
+    assert_eq!(r["ok"], true, "{r}");
+    h.settle(SETTLE);
+    h.request("ui.text", json!({"text": "7"}), T);
+    h.settle(SETTLE);
+    h.request("ui.key", json!({"key": "Escape"}), T);
+    h.settle(SETTLE);
+    assert_eq!((h.app.ui.view, h.app.ui.right), (ViewMode::Detail, RightPanel::Crop), "Esc only left the field");
+    // the field has let go: the next Esc is the crop tool's
+    h.request("ui.key", json!({"key": "Escape"}), T);
+    h.settle(SETTLE);
+    assert_eq!((h.app.ui.view, h.app.ui.right == RightPanel::Crop), (ViewMode::Detail, false));
+}

docs/parity-checklist.md

@@ -660,7 +660,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained
 | KEY-SHORTCUTS | Shortcut list — ⌘/ | P1 | ✅ | `cmd:app.shortcuts` | |
 | KEY-HELP | Help — F1 | P2 | ✅ | `cmd:app.help` | |
 | KEY-VIDEO-PLAY | Play/pause video — Space | P1 | ⬜ | | |
-| KEY-ESC | Leave tool / view — Esc | P0 | ✅ | `cmd:view.back`, `crates/ui-egui/src/icons.rs` (`navigation_arrowheads_have_both_halves`) | top-bar navigation arrowheads render both halves at 1× and 2× scale |
+| KEY-ESC | Leave tool / view — Esc | P0 | ✅ | `cmd:view.back`, `crates/ui-egui/src/icons.rs` (`navigation_arrowheads_have_both_halves`), `crates/ui-egui/src/tests_crop_tool.rs` | top-bar navigation arrowheads render both halves at 1× and 2× scale; in the crop tool Esc closes the tool first (the crop stays), the next Esc goes back to the grid; Esc that ends an edit in a text field does nothing else |
 | KEY-COMMIT | Commit tool — Return | P1 | ✅ | `cmd:tool.done` | Return closes Crop / Remove / Red Eye / Masking back to Edit (edits apply live, as in Lightroom) |
 | KEY-DELETE-PIN | Delete selected pin — ⌫ | P0 | ✅ | `cmd:mask.delete`, `cmd:spot.delete` | ⌫ deletes the active mask (Masking) or the selected spot (Remove), never the photo while retouching |
 | KEY-HIDEPINS | Hide pins — H | P2 | ⬜ | | H = Remove panel |