diff --git a/apps/lightcraft/src/native_menu.rs b/apps/lightcraft/src/native_menu.rs index 4ac40bcac..67e20e31c 100644 --- a/apps/lightcraft/src/native_menu.rs +++ b/apps/lightcraft/src/native_menu.rs @@ -16,7 +16,7 @@ use std::collections::{HashMap, HashSet}; use std::sync::mpsc::{Receiver, channel}; use lightcraft_ui_egui::LightcraftApp; -use lightcraft_ui_egui::menubar::{MenuNode, menu_bar}; +use lightcraft_ui_egui::menubar::{MenuNode, menu_bar, native_item_presentation, native_menu_delivers as menu_delivers}; use muda::accelerator::{Accelerator, Code, Modifiers}; use muda::{CheckMenuItem, Menu, MenuEvent, MenuItem, PredefinedMenuItem, Submenu}; use serde_json::Value; @@ -154,13 +154,6 @@ fn yields_to_text(sc: &str) -> bool { !sc.contains("Cmd") && !sc.contains("Ctrl") || TEXT_EDIT.contains(&sc) } -/// Whether AppKit hands this shortcut to the menu bar: only key presses with ⌘ or ⌃, and function -/// keys. A plain `C` or `⇧P` never reaches it (the window's view takes the key), so egui has to run -/// those even though their menu item shows the key. -fn menu_delivers(sc: &str) -> bool { - sc.split('+').any(|part| matches!(part, "Cmd" | "Ctrl") || part.strip_prefix('F').is_some_and(|n| n.parse::().is_ok())) -} - /// The shortcuts the menu bar runs itself, for `LightcraftApp::native_shortcuts`. fn owned_by_menu<'a>(installed: impl Iterator, text_focus: bool) -> HashSet { installed.filter(|sc| menu_delivers(sc) && !(text_focus && yields_to_text(sc))).map(str::to_string).collect() @@ -264,7 +257,7 @@ impl NativeMenu { .cloned() .collect(); let nodes = tidy_separators(nodes); - self.append_nodes(&sub, &nodes); + self.append_nodes(&sub, &nodes, &app.ui.settings.keymap); if title == "File" { // ⌘W, the system's own item let _ = sub.append_items(&[ @@ -290,7 +283,7 @@ impl NativeMenu { self.publish_shortcuts(app); } - fn append_nodes(&mut self, sub: &Submenu, nodes: &[MenuNode]) { + fn append_nodes(&mut self, sub: &Submenu, nodes: &[MenuNode], keymap: &lightcraft_ui_egui::shortcuts::Keymap) { for n in nodes { match n { MenuNode::Separator => { @@ -298,7 +291,7 @@ impl NativeMenu { } MenuNode::Submenu { label, children } => { let s = Submenu::new(label_text(label), true); - self.append_nodes(&s, children); + self.append_nodes(&s, children, keymap); let _ = sub.append(&s); } MenuNode::Item { id, params, label, shortcut, enabled, checked } => { @@ -306,26 +299,16 @@ impl NativeMenu { if self.items.contains_key(&key) { continue; // muda ids must be unique } - let accel = shortcut.as_deref().filter(|s| !CONTEXTUAL.contains(s)).and_then(accelerator); + let (native_label, native_key) = native_item_presentation(keymap, id, params, label, shortcut.as_deref()); + let accel = native_key.filter(|s| !CONTEXTUAL.contains(s)).and_then(accelerator); let handle = match checked { Some(c) => { - let it = CheckMenuItem::with_id( - key.clone(), - lightcraft_ui_egui::menubar::display_item_label(id, params, label).replace('&', "&&"), - *enabled, - *c, - accel, - ); + let it = CheckMenuItem::with_id(key.clone(), native_label.replace('&', "&&"), *enabled, *c, accel); let _ = sub.append(&it); Handle::Check(it) } None => { - let it = MenuItem::with_id( - key.clone(), - lightcraft_ui_egui::menubar::display_item_label(id, params, label).replace('&', "&&"), - *enabled, - accel, - ); + let it = MenuItem::with_id(key.clone(), native_label.replace('&', "&&"), *enabled, accel); let _ = sub.append(&it); Handle::Plain(it) } @@ -338,7 +321,7 @@ impl NativeMenu { params: params.clone(), shortcut: shortcut.clone(), accel, - label: label.clone(), + label: native_label, enabled: *enabled, checked: *checked, }, @@ -382,18 +365,19 @@ impl NativeMenu { self.rebuild(app); return quit; } - fn walk(nodes: &[MenuNode], items: &mut HashMap) { + fn walk(nodes: &[MenuNode], items: &mut HashMap, keymap: &lightcraft_ui_egui::shortcuts::Keymap) { for n in nodes { match n { - MenuNode::Submenu { children, .. } => walk(children, items), - MenuNode::Item { id, params, label, enabled, checked, .. } => { + MenuNode::Submenu { children, .. } => walk(children, items, keymap), + MenuNode::Item { id, params, label, shortcut, enabled, checked } => { let Some(it) = items.get_mut(&MenuNode::key(id, params)) else { continue }; - if it.label != *label { + let (native_label, _) = native_item_presentation(keymap, id, params, label, shortcut.as_deref()); + if it.label != native_label { match &it.handle { - Handle::Plain(h) => h.set_text(lightcraft_ui_egui::menubar::display_item_label(id, params, label).replace('&', "&&")), - Handle::Check(h) => h.set_text(lightcraft_ui_egui::menubar::display_item_label(id, params, label).replace('&', "&&")), + Handle::Plain(h) => h.set_text(native_label.replace('&', "&&")), + Handle::Check(h) => h.set_text(native_label.replace('&', "&&")), } - it.label = label.clone(); + it.label = native_label; } if it.enabled != *enabled { match &it.handle { @@ -414,7 +398,7 @@ impl NativeMenu { } } } - walk(&bar.iter().flat_map(|(_, v)| v.clone()).collect::>(), &mut self.items); + walk(&bar.iter().flat_map(|(_, v)| v.clone()).collect::>(), &mut self.items, &app.ui.settings.keymap); let focus = ctx.egui_wants_keyboard_input(); let capturing = app.recording_shortcut.is_some(); diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs index 96463df61..278ef178b 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -55,6 +55,32 @@ pub fn display_item_label<'a>(id: &str, params: &Value, label: &'a str) -> &'a s } } +/// Whether AppKit offers a shortcut to its menu instead of the window. Plain letters stay +/// with egui even when the native menu displays them as key equivalents. +pub fn native_menu_delivers(sc: &str) -> bool { + sc.split('+').any(|part| matches!(part, "Cmd" | "Ctrl") || part.strip_prefix('F').is_some_and(|n| n.parse::().is_ok())) +} + +/// Native label and accelerator for a menu row. A grid row may borrow the cycle key as a +/// hint, but must not consume a modified key and turn cycling into direct grid selection. +/// AppKit has no separate shortcut-hint column: borrowed native-owned keys are appended to +/// the translated label instead, leaving their dispatch with egui. Ordinary G stays in the +/// native key-equivalent column because AppKit never hands it to the menu. +pub fn native_item_presentation<'a>( + keymap: &crate::shortcuts::Keymap, + id: &str, + params: &Value, + label: &str, + shortcut: Option<&'a str>, +) -> (String, Option<&'a str>) { + let label = display_item_label(id, params, label); + let borrowed = matches!(id, "view.photoGrid" | "view.squareGrid") && shortcut != crate::shortcuts::shortcut_of(keymap, id); + match shortcut.filter(|sc| borrowed && native_menu_delivers(sc)) { + Some(sc) => (format!("{label} ({})", shortcut_text(sc, true)), None), + None => (label.to_string(), shortcut), + } +} + /// Top-level menus in order (the macOS app menu is added by the host). pub const MENUS: &[&str] = &["File", "Edit", "View", "Photo", "Window", "Help"]; @@ -740,6 +766,7 @@ fn node(app: &LightcraftApp, e: &MenuEntry) -> MenuNode { // in an album the Delete key removes from the album (issue #1805): the menu says so let delete_key = crate::shortcuts::shortcut_of(&app.ui.settings.keymap, "photo.delete"); let shortcut = match e.id.as_str() { + "view.photoGrid" | "view.squareGrid" => shortcut.or_else(|| crate::shortcuts::shortcut_of(&app.ui.settings.keymap, "view.gridToggle")), "album.removePhotos" => delete_key, "photo.delete" if matches!(app.session.source, lightcraft_engine::LibrarySource::Album(_)) => None, _ => shortcut, @@ -1076,6 +1103,107 @@ mod tests { }) } + /// #2500: a direct-grid binding takes precedence over the borrowed cycle hint. + #[test] + fn grid_menu_shortcut_hints_preserve_direct_bindings_and_clicks() { + let mut a = app(); + crate::shortcuts::assign(&mut a.ui.settings.keymap, "view.photoGrid", Some("Cmd+Shift+1")).unwrap(); + let bar = menu_bar(&a); + let nodes = &bar.iter().find(|(title, _)| title == "View").unwrap().1; + assert!(matches!(find(nodes, "view.photoGrid"), Some(MenuNode::Item { shortcut: Some(sc), .. }) if sc == "Cmd+Shift+1")); + assert!(matches!(find(nodes, "view.squareGrid"), Some(MenuNode::Item { shortcut: Some(sc), .. }) if sc == "G")); + // A hint never assigns the cycle key to another command. + assert_eq!(crate::shortcuts::shortcut_of(&a.ui.settings.keymap, "view.squareGrid"), None); + run_item(&mut a, "view.photoGrid", Value::Null).unwrap(); + assert_eq!(a.ui.view, ViewMode::PhotoGrid); + run_item(&mut a, "view.photoGrid", Value::Null).unwrap(); + assert_eq!(a.ui.view, ViewMode::PhotoGrid, "clicking the active row does not cycle"); + run_item(&mut a, "view.squareGrid", Value::Null).unwrap(); + assert_eq!(a.ui.view, ViewMode::SquareGrid); + run_item(&mut a, "view.gridToggle", Value::Null).unwrap(); + assert_eq!(a.ui.view, ViewMode::PhotoGrid); + } + + /// #2500: native hints never install a borrowed modifier/function key as a direct-grid + /// accelerator. Plain G remains in AppKit's shortcut column and is dispatched by egui. + #[test] + fn grid_menu_shortcut_hints_keep_native_toggle_dispatch() { + let mut keymap = crate::shortcuts::Keymap::new(); + for id in ["view.photoGrid", "view.squareGrid"] { + let (label, accelerator) = native_item_presentation(&keymap, id, &Value::Null, "Photo Grid", Some("G")); + assert_eq!(label, "Photo Grid"); + assert_eq!(accelerator, Some("G")); + assert!(!native_menu_delivers(accelerator.unwrap())); + for (key, text) in [("Cmd+Shift+G", "⌘⇧G"), ("Ctrl+G", "⌃G"), ("F3", "F3")] { + crate::shortcuts::assign(&mut keymap, "view.gridToggle", Some(key)).unwrap(); + let (label, accelerator) = native_item_presentation(&keymap, id, &Value::Null, "Photo Grid", Some(key)); + assert_eq!(label, format!("Photo Grid ({text})")); + assert_eq!(accelerator, None, "egui keeps {key}"); + } + } + crate::shortcuts::assign(&mut keymap, "view.photoGrid", Some("Cmd+Shift+1")).unwrap(); + let (label, accelerator) = native_item_presentation(&keymap, "view.photoGrid", &Value::Null, "Photo Grid", Some("Cmd+Shift+1")); + assert_eq!(label, "Photo Grid"); + assert_eq!(accelerator, Some("Cmd+Shift+1"), "the row's own shortcut is a real accelerator"); + } + + /// #2500: the control channel runs the real keyboard path exactly once and menu commands + /// continue to select their named grid after the cycle shortcut is remapped. + #[test] + fn grid_menu_shortcut_hints_preserve_control_keyboard_dispatch() { + let mut h = crate::headless::Headless::new(app(), [960.0, 720.0], 1.0); + h.app.ui.view = ViewMode::PhotoGrid; + for _ in 0..3 { + h.step(); + } + let timeout = std::time::Duration::from_secs(20); + let press = |h: &mut crate::headless::Headless, params| { + let reply = h.request("ui.key", params, timeout); + assert_eq!(reply["ok"], true, "{reply}"); + }; + press(&mut h, json!({"key": "G"})); + assert_eq!(h.app.ui.view, ViewMode::SquareGrid); + press(&mut h, json!({"key": "G"})); + assert_eq!(h.app.ui.view, ViewMode::PhotoGrid); + crate::shortcuts::assign(&mut h.app.ui.settings.keymap, "view.gridToggle", Some("Cmd+Shift+G")).unwrap(); + press(&mut h, json!({"key": "G"})); + assert_eq!(h.app.ui.view, ViewMode::PhotoGrid); + press(&mut h, json!({"key": "G", "cmd": true, "shift": true})); + assert_eq!(h.app.ui.view, ViewMode::SquareGrid); + press(&mut h, json!({"key": "G", "cmd": true, "shift": true})); + assert_eq!(h.app.ui.view, ViewMode::PhotoGrid); + for _ in 0..2 { + let reply = h.request("ui.menu.invoke", json!({"command": "view.squareGrid"}), timeout); + assert_eq!(reply["ok"], true, "{reply}"); + assert_eq!(h.app.ui.view, ViewMode::SquareGrid); + } + } + + /// #2500: grid view rows show the separate cycle command's current shortcut. + #[test] + fn grid_menu_shortcut_hints_follow_the_cycle_key() { + let mut a = app(); + let hint = |a: &LightcraftApp, id: &str| { + let bar = menu_bar(a); + let nodes = &bar.iter().find(|(title, _)| title == "View").expect("View menu").1; + match find(nodes, id).expect("grid row") { + MenuNode::Item { shortcut, .. } => shortcut.clone(), + _ => panic!("grid row must be an item"), + } + }; + for id in ["view.photoGrid", "view.squareGrid"] { + assert_eq!(hint(&a, id).as_deref(), Some("G"), "{id}"); + } + crate::shortcuts::assign(&mut a.ui.settings.keymap, "view.gridToggle", Some("Cmd+Shift+G")).unwrap(); + for id in ["view.photoGrid", "view.squareGrid"] { + assert_eq!(hint(&a, id).as_deref(), Some("Cmd+Shift+G"), "{id}"); + } + crate::shortcuts::assign(&mut a.ui.settings.keymap, "view.gridToggle", None).unwrap(); + for id in ["view.photoGrid", "view.squareGrid"] { + assert_eq!(hint(&a, id), None, "{id}"); + } + } + #[test] fn view_menu_offers_the_hold_and_the_refresh_and_checks_the_hold_while_it_is_on() { let mut a = app(); diff --git a/docs/gaps.md b/docs/gaps.md index f6686e2b4..f215744a1 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Gaps: where LightCraft falls short of Lightroom -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-15 · **Change:** minor (gap 15: Print Page Options — page numbers, page info, crop marks) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-21 · **Change:** patch (gap 28: the View menu shows the grid-cycle shortcut) Every known shortfall, one entry each, ranked by user impact (how many Lightroom users hit it, and how hard it blocks them), then by effort. **Agents pick work from the top** (the first entry nobody is @@ -43,7 +43,7 @@ Hours are Opus 5.5 agent wall-clock hours (see [target-app-parity.md](target-app | 26 | **Performance at scale**: 85k-photo library opens in 1.7–4.7 s; 100k grid test pending; colour NR at half resolution; at 100k photos stack and album lookups are indexed (an "In a Stack" or "Album" smart album counts in ~14 ms, was 0.6–2.7 s) | ROADMAP log, gpu-pipeline.md | large-catalog users | 15–30 | [target-app-parity](target-app-parity.md#by-dimension) | | 27 | **Windows / Linux runtime coverage**: Windows installer UI unverified on Windows (#79); GPU path proven on Apple and on one AMD GPU on Linux (Radeon 8060S, Vulkan); NVIDIA, Intel and DX12 unverified; the Linux riscv64 tarball (new in 0.6.0, #844) is cross-compiled and never run beyond a QEMU smoke test | ROADMAP, issues, `release.yml` | non-Mac users find bugs first | 13–27 | [hardware-parity](hardware-parity.md) | | 29 | **HDR merge leftovers** (LR-MERGE-HDR): AI Denoise doesn't run on merged (linear) DNGs; JPEG brackets are treated as linear (no camera response curve); every frame is held as 32-bit float, ~0.86 GB per 67 MP frame (half-float frames would halve it) | tested on Sony A7R VI brackets 2026-10-14 ([merge.md](merge.md)); Lightroom's Denoise supports HDR DNGs | HDR shooters who denoise their merges, merge JPEGs, or merge 7+ frames of high-resolution raws on 16 GB machines | 10–20 (+ denoise linear-RGB contract, gap 11) | [merge](merge.md) | -| 28 | **Small Classic shortcuts and keys**: rating `[` `]`, flag cycling, filter-bar keys, F1 help, panel keys (Tab, F5, F8; F6 and F7 toggle the filmstrip and the left panel) | [ui-parity](ui-parity.md#shortcuts) | Classic muscle memory | 4–8 | [ui-parity](ui-parity.md) | +| 28 | **Small Classic shortcuts and keys**: rating `[` `]`, flag cycling, filter-bar keys, F1 help, panel keys (Tab, F5, F8; F6 and F7 toggle the filmstrip and the left panel); the grid-cycle key is now shown beside both grid rows in View (#2500) | [ui-parity](ui-parity.md#shortcuts) | Classic muscle memory | 4–8 | [ui-parity](ui-parity.md) | Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform work they need around them, **~340–600 h** to beta (incl. 40–80 h for the ~92 open core-path user issues) (see [target-app-parity.md](target-app-parity.md#stage)). @@ -77,6 +77,7 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w | Date | Change | Summary | |---|---|---| +| 2026-10-21 | patch | Gap 28: View shows the current grid-cycle key beside Photo Grid and Square Grid (#2500); no new binding and no change to the remaining shortcut gap or hours | | 2026-10-18 | patch | Gap 1: six X-Trans CMOS 4 Fujifilm bodies (X-T30 …) take the X-T3's spectral matrices instead of the neutral matrix | | 2026-10-15 | minor | Gap 15 shrinks: the Page panel's Page Options (Page Numbers, Page Info, Crop Marks) print on Single Image and Contact Sheet pages; identity plate, watermark, photo info, Cut Guides, direct printing and JPEG output remain | | 2026-10-14 | minor | Gap 26: at 100k photos stack and album lookups are indexed, so an "In a Stack" or "Album" smart album counts in ~14 ms (was 0.6–2.7 s); open time unchanged | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 02f5bf08f..547009ff7 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-20 · **Change:** minor (LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-21 · **Change:** patch (KEY-GRID: live grid-cycle shortcut hints in the View menu) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -603,7 +603,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | KEY-EDIT | Edit — E | P0 | ✅ | `cmd:panel.edit` | | | KEY-FULLSCREEN | Full-screen preview — F | P1 | ✅ | `cmd:view.fullScreenPreview` | | | KEY-LIGHTSOUT | Lights Out — Shift+L (Classic's L is Linear Gradient here) | P2 | ✅ | `cmd:view.lightsOut` | On → Dim (80 %) → Off; Esc turns the lights on | -| KEY-GRID | Grid — G | P0 | ✅ | `cmd:view.gridToggle`, `cmd:view.photoGrid` | G toggles Photo Grid ↔ Square Grid | +| KEY-GRID | Grid — G | P0 | ✅ | `cmd:view.gridToggle`, `cmd:view.photoGrid` | G toggles Photo Grid ↔ Square Grid; both View menu rows show its live shortcut (including remapping/unbinding), with direct grid bindings taking priority; menu clicks still choose the named grid (#2500) | | KEY-INFO | Info — I | P0 | ✅ | `cmd:panel.info` | | | KEY-KEYWORDS | Keywords — K | P0 | ✅ | `cmd:panel.keywords` | | | KEY-CLIPBOARD | Copy / paste edit settings — ⌘C / ⌘V | P0 | ✅ | `cmd:develop.copy`, `cmd:develop.paste`, `crates/ui-egui/src/shortcuts.rs` (`matches`) | ⌘X has nothing to cut outside text fields; on Windows and Linux, where the windowing layer sends Ctrl+C / Ctrl+V as clipboard events rather than key presses, those run the shortcuts too (⇧Insert pastes plainly); there Ctrl+V only arrives while the system clipboard holds text (egui-winit drops it otherwise), so with an empty clipboard Paste Settings needs the menu | @@ -797,6 +797,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-21 | patch | KEY-GRID: both grid rows in View show the cycle shortcut and follow keymap changes; direct bindings and native accelerator ownership are preserved (#2500); status and counts unchanged | | 2026-10-20 | patch | LR-VIEW-ZOOM: the 20 ms rule clears a slow-drag verdict only on a drag timed at full size; a drag that drafts small leaves it unchanged, as the code does; status unchanged | | 2026-10-19 | patch | LR-VIEW-ZOOM: the slow slider-drag verdict is described as per photo and control, as the code keys it; status unchanged | | 2026-10-15 | minor | LR-VIEW-ZOOM: a slow slider-drag verdict needs two drags over 20 ms and expires after 30 s (doubling to 240 s); hidden-window frame times are ignored; the slow set forgets its oldest entry, not all (issue #330); status unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index cfeed33ed..a415e1244 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,11 @@ # What's new in LightCraft +## 2026-10-21 + +### Find the grid shortcut in View +- Photo Grid and Square Grid now show the grid-cycle shortcut (G by default) in the View menu. The hints follow + shortcut changes; clicking a menu row still selects that grid (issue #2500). + ## 2026-10-19 ### Hidden Local locations come back one at a time