From 382cca190cf76713393ca554f56769ea11973771 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Thu, 1 Oct 2026 09:50:20 -0400 Subject: [PATCH] =?UTF-8?q?M16.1:=20shortcuts=20=E2=80=94=20Lightroom-desk?= =?UTF-8?q?top=20secondary=20bindings,=20no=20double-firing=20keys?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds secondary bindings for existing commands (Cmd+D select none, Shift+E export dialog, Space toggle zoom, Shift+M create version, Shift+X / Shift+U reject / unflag and advance), listed in Help > Keyboard Shortcuts. Shift+Y no longer also toggles the History panel; a UI command now shadows the engine command it wraps on the same key (W, Cmd+Shift+I). New tests reject conflicting bindings. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/menus.rs | 2 +- crates/ui-egui/src/panels/dialogs.rs | 12 ++++ crates/ui-egui/src/shortcuts.rs | 84 ++++++++++++++++++++++++++-- 3 files changed, 93 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 1dd7101f..4c1b22a7 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -39,7 +39,7 @@ pub const UI_COMMANDS: &[UiCommand] = &[ ("panel.info", "Info", Some("I"), "View"), ("panel.keywords", "Keywords", Some("K"), "View"), ("panel.versions", "Versions", Some("Shift+V"), "View"), - ("panel.activity", "History", Some("Shift+Y"), "View"), + ("panel.activity", "History", None, "View"), ("panel.close", "Close Panel", None, "View"), ("section.light", "Light", Some("Cmd+Alt+1"), "View"), ("section.color", "Color", Some("Cmd+Alt+2"), "View"), diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 0269aad2..9a086292 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -180,6 +180,18 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { ui.end_row(); } } + for (sc, id, _) in crate::shortcuts::ALIASES { + let label = crate::menus::UI_COMMANDS + .iter() + .find(|c| c.0 == *id) + .map(|c| c.1) + .or_else(|| lightcraft_engine::find_command(id).map(|c| c.label)) + .unwrap_or(id); + ui.label(label); + ui.label(*sc); + ui.label(egui::RichText::new(*id).color(t.text_dim)); + ui.end_row(); + } }); }); } diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index af53a4bd..1690be12 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -5,6 +5,18 @@ use serde_json::json; use crate::LightcraftApp; +/// Secondary key bindings for commands that already exist: `(shortcut, command id, params JSON)`. +/// They complement the primary shortcut declared on the command (Lightroom-desktop keys that our +/// primary keymap assigns elsewhere, see docs/parity.md → Shortcuts). Shown in Help → Keyboard Shortcuts. +pub const ALIASES: &[(&str, &str, &str)] = &[ + ("Cmd+D", "library.selectNone", "{}"), + ("Shift+E", "dialog.export", "{}"), + ("Space", "view.zoomToggle", "{}"), + ("Shift+M", "version.create", "{}"), + ("Shift+X", "photo.flag", r#"{"flag": "reject", "advance": true}"#), + ("Shift+U", "photo.flag", r#"{"flag": "none", "advance": true}"#), +]; + pub fn parse(s: &str) -> Option<(Modifiers, Key)> { let mut m = Modifiers::NONE; let mut key = None; @@ -50,21 +62,34 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { return; } let mut fire: Vec = Vec::new(); + let mut aliased: Vec<(&str, serde_json::Value)> = Vec::new(); ctx.input(|i| { + let mut ui_keys = Vec::new(); for (id, _, sc, _) in crate::menus::UI_COMMANDS { - if let Some((m, k)) = sc.and_then(parse) - && matches(i, m, k) - { - fire.push(id.to_string()); + if let Some((m, k)) = sc.and_then(parse) { + ui_keys.push((m, k)); + if matches(i, m, k) { + fire.push(id.to_string()); + } } } for c in lightcraft_engine::command_specs() { + // A UI command bound to the same key wraps the engine command (e.g. `W` opens the + // White Balance Selector tool rather than sampling without a point): the UI one wins. if let Some((m, k)) = c.shortcut.and_then(parse) + && !ui_keys.contains(&(m, k)) && matches(i, m, k) { fire.push(c.id.to_string()); } } + for (sc, id, params) in ALIASES { + if let Some((m, k)) = parse(sc) + && matches(i, m, k) + { + aliased.push((id, serde_json::from_str(params).unwrap_or_default())); + } + } // rating 0-5, colour labels 6-9 (with Shift: and advance) for (n, key) in [Key::Num0, Key::Num1, Key::Num2, Key::Num3, Key::Num4, Key::Num5].iter().enumerate() { if matches(i, Modifiers::NONE, *key) { @@ -80,6 +105,9 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { } } }); + for (id, params) in aliased { + let _ = app.run(id, params); + } for f in fire { if let Some(rest) = f.strip_prefix("rate:") { let (n, adv) = rest.split_once(':').unwrap_or(("0", "0")); @@ -133,4 +161,52 @@ mod tests { } } } + + #[test] + fn aliases_parse_and_target_existing_commands() { + let ui: Vec<&str> = crate::menus::UI_COMMANDS.iter().map(|c| c.0).collect(); + for (sc, id, params) in ALIASES { + assert!(parse(sc).is_some(), "{id}: {sc}"); + assert!(ui.contains(id) || lightcraft_engine::find_command(id).is_some(), "alias {sc} → unknown command {id}"); + assert!(serde_json::from_str::(params).is_ok(), "alias {sc}: bad params"); + } + } + + /// Engine commands that intentionally share a key and are disambiguated by context in [`handle`]. + const CONTEXTUAL: &[(&str, &str)] = &[("photo.reject", "crop.rotateAspect")]; + + /// No key fires two different actions (a UI command may shadow the engine command it wraps). + #[test] + fn no_conflicting_bindings() { + let mut ui: Vec<((Modifiers, Key), String)> = Vec::new(); + for (id, _, sc, _) in crate::menus::UI_COMMANDS { + if let Some(k) = sc.and_then(parse) { + ui.push((k, id.to_string())); + } + } + for (sc, id, _) in ALIASES { + ui.push((parse(sc).unwrap(), format!("alias {id}"))); + } + let mut engine: Vec<((Modifiers, Key), &str)> = Vec::new(); + for c in lightcraft_engine::command_specs() { + if let Some(k) = c.shortcut.and_then(parse) { + engine.push((k, c.id)); + } + } + for (sc, id, _) in ALIASES { + let k = parse(sc).unwrap(); + assert!(!engine.iter().any(|(k2, _)| *k2 == k), "alias {sc} ({id}) shadows an engine shortcut"); + } + for (i, (k, a)) in ui.iter().enumerate() { + for (k2, b) in &ui[i + 1..] { + assert!(k != k2, "{a} and {b} share a key"); + } + } + for (i, (k, a)) in engine.iter().enumerate() { + for (k2, b) in &engine[i + 1..] { + let contextual = CONTEXTUAL.iter().any(|(x, y)| (x == a && y == b) || (x == b && y == a)); + assert!(k != k2 || contextual, "{a} and {b} share a key"); + } + } + } }