diff --git a/crates/ui-egui/src/panels/keymap.rs b/crates/ui-egui/src/panels/keymap.rs index 1bda17758..11290b1b1 100644 --- a/crates/ui-egui/src/panels/keymap.rs +++ b/crates/ui-egui/src/panels/keymap.rs @@ -80,7 +80,8 @@ fn menu_text(sc: &str, mac: bool) -> String { } fn row(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens, b: &Bindable, sc: Option<&str>, mac: bool) { - let changed = app.ui.settings.keymap.contains_key(b.id); + // a declared key that yields to another command's saved key counts as changed: ↺ takes it back + let changed = app.ui.settings.keymap.contains_key(b.id) || (sc.is_none() && b.default.is_some()); let mut name = RichText::new(crate::i18n::tr(b.label)).color(t.text); if changed { name = name.strong(); diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 032548f6a..32fc6bea4 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -54,15 +54,30 @@ pub fn find_bindable(id: &str) -> Option<&'static Bindable> { /// The shortcut `id` has now: the user's choice (`""` = none), else the declared one. An entry /// that doesn't parse (a hand-edited `ui.json`, a modifier-only key saved by an older build) is -/// ignored. +/// ignored. The declared shortcut yields to a saved entry that gives its key to another known +/// command other than its contextual partner. pub fn binding<'a>(keymap: &'a Keymap, id: &str, default: Option<&'a str>) -> Option<&'a str> { match keymap.get(id) { Some(sc) if sc.is_empty() => None, Some(sc) if parse(sc).is_some() => Some(sc.as_str()), - _ => default, + _ => default.filter(|sc| !assigned_elsewhere(keymap, id, sc)), } } +/// Whether the keymap gives the key `sc` to a known command other than `id` and its contextual +/// partner. +fn assigned_elsewhere(keymap: &Keymap, id: &str, sc: &str) -> bool { + let Some(key) = parse(sc) else { return false }; + saved_keys(keymap) + .any(|(other, k)| other != id && k == key && !CONTEXTUAL.iter().any(|(x, y)| (*x == id && *y == other) || (*y == id && *x == other))) +} + +/// The keys the keymap gives to known commands, with each command's id. Entries for commands this +/// build doesn't know and entries that don't parse are ignored. +fn saved_keys(keymap: &Keymap) -> impl Iterator { + keymap.iter().filter(|(id, _)| find_bindable(id).is_some()).filter_map(|(id, sc)| Some((id.as_str(), parse(sc)?))) +} + /// The effective shortcut of command `id` (`None` for unknown commands). pub fn shortcut_of<'a>(keymap: &'a Keymap, id: &str) -> Option<&'a str> { find_bindable(id).and_then(|b| binding(keymap, id, b.default)) @@ -373,8 +388,8 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { // The grid navigation keys act on the photos in view, so they wait while the full-screen preview // covers the grid: a key must not move the selection onto photos nobody can see. let grid_navigation = grid && !app.ui.fullscreen; - // keys the user gave to a command: the fixed bindings below (aliases, ratings) yield to them - let taken: Vec<(Modifiers, Key)> = keymap.values().filter_map(|s| parse(s)).collect(); + // keys the user gave to a known command: the fixed bindings below (aliases, ratings) yield to them + let taken: Vec<(Modifiers, Key)> = saved_keys(keymap).map(|(_, k)| k).collect(); // 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); @@ -828,6 +843,50 @@ mod tests { } } + /// A saved keymap entry that gives a key to one command (kept from before another command + /// declared that key) wins: the press runs that command alone, and the other command's declared + /// key comes back once the entry is gone. + #[test] + fn a_saved_key_wins_over_another_commands_declared_key() { + use lightcraft_catalog::{Flag, PhotoId}; + let mut h = library(); + h.app.ui.settings.keymap.insert("photo.increaseRating".into(), "P".into()); + assert_eq!(super::shortcut_of(&h.app.ui.settings.keymap, "photo.pick"), None); + assert_eq!(super::conflicts(&h.app.ui.settings.keymap, "view.survey", "P"), vec!["photo.increaseRating"]); + key(&mut h, "P", false); + let photo = h.app.session.catalog.photo(PhotoId(1)).unwrap(); + assert_eq!(photo.rating, 1, "the saved key raised the rating"); + assert_eq!(photo.flag, Flag::None, "Flag as Pick's declared key yielded"); + // with the entry gone, P picks again + h.app.ui.settings.keymap.remove("photo.increaseRating"); + assert_eq!(super::shortcut_of(&h.app.ui.settings.keymap, "photo.pick"), Some("P")); + key(&mut h, "P", false); + assert_eq!(h.app.session.catalog.photo(PhotoId(1)).unwrap().flag, Flag::Pick); + // a contextual partner's saved key does not silence the other side + h.app.ui.settings.keymap.insert("crop.rotateAspect".into(), "X".into()); + assert_eq!(super::shortcut_of(&h.app.ui.settings.keymap, "photo.reject"), Some("X")); + // nor does an entry for a command this build doesn't know + h.app.ui.settings.keymap.insert("no.suchCommand".into(), "U".into()); + assert_eq!(super::shortcut_of(&h.app.ui.settings.keymap, "photo.unflag"), Some("U")); + } + + /// Keymap entries for commands this build doesn't know take no key: an alias, a rating key and + /// a declared shortcut on the same keys keep working. + #[test] + fn junk_keymap_entries_leave_aliases_ratings_and_declared_keys() { + use lightcraft_catalog::{Flag, PhotoId}; + let mut h = library(); + for (id, sc) in [("no.suchCommand", "Shift+X"), ("no.otherCommand", "5"), ("no.thirdCommand", "P")] { + h.app.ui.settings.keymap.insert(id.into(), sc.into()); + } + key(&mut h, "5", false); + assert_eq!(h.app.session.catalog.photo(PhotoId(1)).unwrap().rating, 5, "5 still rates"); + key(&mut h, "P", false); + assert_eq!(h.app.session.catalog.photo(PhotoId(1)).unwrap().flag, Flag::Pick, "P still picks"); + key(&mut h, "X", true); + assert_eq!(h.app.session.catalog.photo(PhotoId(1)).unwrap().flag, Flag::Reject, "Shift+X still rejects"); + } + #[test] fn classic_f6_and_f7_toggle_the_filmstrip_and_left_panel() { let mut h = library(); diff --git a/crates/ui-egui/src/tests_keymap.rs b/crates/ui-egui/src/tests_keymap.rs index b48da7cdf..9f2e0504a 100644 --- a/crates/ui-egui/src/tests_keymap.rs +++ b/crates/ui-egui/src/tests_keymap.rs @@ -103,6 +103,27 @@ fn a_key_given_to_a_command_overrides_a_fixed_key() { assert_eq!(before, after, "the rating key must not fire too"); } +/// A declared key that yields to another command's saved key can be taken back with ↺. +#[test] +fn restore_takes_back_a_declared_key_another_command_holds() { + use crate::shortcuts::shortcut_of; + let mut h = demo(); + h.app.ui.settings.keymap.insert("photo.increaseRating".into(), "P".into()); + assert_eq!(shortcut_of(&h.app.ui.settings.keymap, "photo.pick"), None); + h.app.run("app.shortcuts", json!({})).unwrap(); + h.step(); + h.step(); + click(&mut h, "check:shortcuts.showAll"); + click(&mut h, "field:shortcutsSearch"); + assert_eq!(h.request("ui.text", json!({"text": "flag as pick"}), T)["ok"], true); + h.step(); + h.step(); + click(&mut h, "button:shortcutReset-photo.pick"); + assert_eq!(shortcut_of(&h.app.ui.settings.keymap, "photo.pick"), Some("P")); + assert_eq!(shortcut_of(&h.app.ui.settings.keymap, "photo.increaseRating"), None); + assert!(h.app.ui.settings.keymap.is_empty(), "{:?}", h.app.ui.settings.keymap); +} + /// Open the editor, filtered to `search`, and start recording for `id`. fn record(h: &mut Headless, search: &str, id: &str) { if h.app.ui.dialog.is_none() { diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index d059494ce..df3481f67 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -437,7 +437,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-PREF-PERFORMANCE | GPU / performance | P1 | ✅ | `cmd:app.gpu`, `cmd:app.memoryBudget`, `cmd:app.settings` | GPU on/off (a crash while the GPU device or its kernels are built, issue #250, turns it off for the next launch), preview size (Automatic = the whole-frame loupe render at canvas size, up to 2560 px; or a 1600–5120 px size for it; zoomed views add a window at up to 100 %, issue #323), memory budget (a quarter of it is the idle per-view stage caches' share; the open photo's views are counted and reported, and cut only past four times that, the before side's and the zoom window's first — a Before/After at 1:1 on a 2800 px canvas holds about 1 GB of GPU stages, plus the one 288 MB copy of the original, which `ui.inspect` reports as `sharedSourceBytes`), thumbnail cache size in Settings | | LR-PREF-PEOPLE | Face recognition | P2 | 🟡 | `cmd:faces.enable`, `cmd:faces.models.list`, `cmd:faces.models.inspect`, `cmd:faces.models.install`, `cmd:faces.models.download`, `cmd:faces.models.downloads`, `cmd:faces.models.downloadCancel`, `cmd:faces.models.remove`, `cmd:faces.models.select`, `cmd:dialog.faceModel`, `crates/faces/src/lib.rs`, `crates/ui-egui/src/panels/faces.rs` | Settings ▸ Faces: an off-by-default toggle and the face model list. Models are opt-in and never bundled, the 232 KB YuNet detector included. Press Download (terms first; then a pinned address, size and SHA-256 checked, fetched over pure-Rust https with `lightcraft-fetch`), or add any `.onnx` by picking or dropping it: it is recognised by hash or described from its shape, shows its licence terms and training-data note, runs a self-test, and installs only after an "I accept" box; an installed model is chosen and recognition switched on at once (the newest wins, each model keeps its own embedding cache). Non-commercial models are add-by-file only, never offered for download. Suggestions of names run on the shared pure-Rust CPU interpreter (`crates/faces/src/net.rs`, `crates/faces/src/runtime.rs`; bounded im2col/GEMM, checked ONNX shapes/operators; no external inference runtime; 40 public crops agree within 2.1e-6, one-thread SFace 34–36 ms / AuraFace 233–252 ms, see docs/faces.md) in a background worker and are never applied automatically. Thresholds are starting points calibrated on busts, not yet on live faces; the Recognise faces switch is off until a recognition model is chosen and installed (`faces.enable {enabled: true}` refuses otherwise; removing the last recogniser switches it off), issue #678 | | LR-PREF-WATERMARK | Watermark settings | P1 | ✅ | `cmd:export.savePreset`, `crates/ui-egui/src/panels/dialogs.rs` | set in the Export dialog; kept with Export with Previous and in saved export presets | -| LR-PREF-SHORTCUTS | Shortcut customisation | — | ✅ | `cmd:app.setShortcut`, `cmd:app.resetShortcuts`, `crates/ui-egui/src/panels/keymap.rs` | an extra (the reference app has no keymap editor): Help ▸ Keyboard Shortcuts (⌘/) lists every command; click a shortcut and press the new keys (Esc cancels), × removes, ↺ restores, Reset All; a key another command had moves over; saved in `ui.json`; native menu accelerators follow. Rating/label digits and secondary keys are listed read-only and yield to a key given to a command | +| LR-PREF-SHORTCUTS | Shortcut customisation | — | ✅ | `cmd:app.setShortcut`, `cmd:app.resetShortcuts`, `crates/ui-egui/src/panels/keymap.rs` | an extra (the reference app has no keymap editor): Help ▸ Keyboard Shortcuts (⌘/) lists every command; click a shortcut and press the new keys (Esc cancels), × removes, ↺ restores, Reset All; a key another command had moves over; a declared key that `ui.json` already gives another command stays with that command until ↺ takes it back; saved in `ui.json`; native menu accelerators follow. Rating/label digits and secondary keys are listed read-only and yield to a key given to a command | | LR-PREF-TECHPREVIEW | Early-access toggles | P2 | ⬜ | | | | LR-PREF-NOTIFICATIONS | Notifications | OOS | 🚫 | | | | LR-PREF-DEVICE | Device settings | P2 | ⬜ | | |