diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 04d43e0d3..2f1108bac 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -128,6 +128,9 @@ pub const UI_COMMANDS: &[UiCommand] = &[ // Overlay and the Remove panel list the modes. ("view.toolOverlay", "Cycle Tool Overlay", Some("Shift+H"), ""), ("view.visualizeSpots", "Visualize Spots", Some("A"), "View"), + // A while the Crop panel is open: the aspect lock as a first-class row. Visualize Spots keeps + // the key outside the panel; the two are a contextual pair in shortcuts::handle + ("crop.lockAspect", "Lock Aspect Ratio", Some("A"), ""), ("view.cropOverlay", "Cycle Crop Overlay", Some("Shift+O"), "View"), ("view.cropOverlayOrientation", "Cycle Crop Overlay Orientation", None, "View"), ("view.back", "Back to Grid", Some("Escape"), ""), @@ -1125,6 +1128,8 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option app.run("crop.aspect", json!({"aspect": "toggle"})), "view.cropOverlay" => { use crate::state::CropOverlay::*; app.ui.crop_overlay = match app.ui.crop_overlay { diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index e7b58a9b5..374e54f1c 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -68,8 +68,10 @@ pub fn shortcut_of<'a>(keymap: &'a Keymap, id: &str) -> Option<&'a str> { find_bindable(id).and_then(|b| binding(keymap, id, b.default)) } -/// Engine commands that intentionally share a key and are disambiguated by context in [`handle`]. -pub const CONTEXTUAL: &[(&str, &str)] = &[("photo.reject", "crop.rotateAspect")]; +/// Engine commands that intentionally share a key and are disambiguated by context in [`handle`]: +/// the X pair by the Crop panel, A by whether it is open (Visualize Spots everywhere, the aspect +/// lock under the Crop panel). +pub const CONTEXTUAL: &[(&str, &str)] = &[("photo.reject", "crop.rotateAspect"), ("view.visualizeSpots", "crop.lockAspect")]; /// Commands other than `id` whose shortcut is the key `sc` (contextual partners excepted). pub fn conflicts(keymap: &Keymap, id: &str, sc: &str) -> Vec<&'static str> { @@ -621,6 +623,13 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { if f == "crop.rotateAspect" && app.ui.right != crate::state::RightPanel::Crop { continue; } + // A is both Visualize Spots and the crop aspect lock: each key only in its own context + if f == "view.visualizeSpots" && app.ui.right == crate::state::RightPanel::Crop { + continue; + } + if f == "crop.lockAspect" && app.ui.right != crate::state::RightPanel::Crop { + continue; + } // / refreshes the selected spot's source in the Remove tool (the filmstrip elsewhere) if f == "view.filmstrip" && app.ui.right == crate::state::RightPanel::Remove && app.session.active_spot.is_some() { let _ = app.run("spot.refreshSource", json!({})); @@ -631,12 +640,11 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { let _ = app.run("view.maskOverlayColor", json!({})); continue; } - // while cropping: O cycles the guides, Shift+O their orientation, A locks the aspect + // while cropping: O cycles the guides, Shift+O their orientation if app.ui.right == crate::state::RightPanel::Crop { let crop_key = match f.as_str() { "view.maskOverlay" => Some(("view.cropOverlay", json!({}))), "view.cropOverlay" => Some(("view.cropOverlayOrientation", json!({}))), - "view.visualizeSpots" => Some(("crop.aspect", json!({"aspect": "toggle"}))), _ => None, }; if let Some((cmd, p)) = crop_key { diff --git a/crates/ui-egui/src/tests_keymap.rs b/crates/ui-egui/src/tests_keymap.rs index 92997d941..19e4027dd 100644 --- a/crates/ui-egui/src/tests_keymap.rs +++ b/crates/ui-egui/src/tests_keymap.rs @@ -453,3 +453,52 @@ fn cmd_o_does_not_fire_while_a_dialog_is_open() { press(&mut h, "O", &["cmd"]); assert_eq!(calls.load(std::sync::atomic::Ordering::SeqCst), 1); } + +/// The crop aspect lock is a first-class row of Help - Keyboard Shortcuts: A, listed in the +/// editor like any other command, sharing the key with Visualize Spots as a contextual pair. +#[test] +fn lock_aspect_ratio_is_a_listed_row_on_a() { + use crate::shortcuts::{CONTEXTUAL, Keymap, bindable, shortcut_of}; + let b = bindable().iter().find(|b| b.id == "crop.lockAspect").expect("crop.lockAspect is mappable"); + assert_eq!(b.label, "Lock Aspect Ratio"); + assert_eq!(b.default, Some("A")); + assert_eq!(shortcut_of(&Keymap::new(), "crop.lockAspect"), Some("A")); + assert!(CONTEXTUAL.contains(&("view.visualizeSpots", "crop.lockAspect")), "A is disambiguated by context"); + // the editor lists it: the row is on screen and records keys for it + let mut h = demo(); + record(&mut h, "lock", "crop.lockAspect"); + key(&mut h, "Escape", false); + assert_eq!(shortcut_of(&h.app.ui.settings.keymap, "crop.lockAspect"), Some("A"), "Esc cancels the recording: A stays"); + key(&mut h, "Escape", false); + assert!(h.app.ui.dialog.is_none()); +} + +/// A locks the aspect while the Crop panel is open — Visualize Spots, the other A, must not take +/// the panel or turn its view on — and outside the panel A opens Visualize Spots as usual. +#[test] +fn a_is_the_aspect_lock_under_crop_and_visualize_spots_elsewhere() { + use crate::state::RightPanel; + let mut h = demo(); + let aspect = |h: &Headless| { + let id = h.app.session.active().expect("an active photo"); + h.app.session.develop_of(id).expect("develop settings").crop.aspect + }; + let spots_before = h.app.ui.visualize_spots; + h.app.run("panel.crop", json!({})).unwrap(); + h.step(); + h.step(); + assert_eq!(h.app.ui.right, RightPanel::Crop); + let before = aspect(&h); + key(&mut h, "A", false); + assert_eq!(h.app.ui.right, RightPanel::Crop, "Visualize Spots must not take the panel"); + assert_eq!(h.app.ui.visualize_spots, spots_before, "nor turn its view on"); + assert_ne!(aspect(&h), before, "A locked the aspect"); + key(&mut h, "A", false); + assert_eq!(aspect(&h), before, "A again unlocks it"); + // outside the panel the same key is Visualize Spots again + h.app.ui.right = RightPanel::None; + key(&mut h, "A", false); + assert_eq!(h.app.ui.right, RightPanel::Remove, "A opens Remove with the visualization"); + assert!(h.app.ui.visualize_spots, "Visualize Spots is on"); + assert_eq!(aspect(&h), before, "and the aspect is untouched"); +}