From 4d8dbddb86b451b783e3735436a16414007667d1 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Thu, 22 Oct 2026 16:40:34 -0400 Subject: [PATCH] MENU-BAR: a menu item really gives its key up to a text field or shortcut recording on macOS --- apps/lightcraft/src/native_menu.rs | 135 ++++++++++++++++++++++++++--- docs/parity-checklist.md | 3 +- docs/whats-new.md | 10 +++ 3 files changed, 137 insertions(+), 11 deletions(-) diff --git a/apps/lightcraft/src/native_menu.rs b/apps/lightcraft/src/native_menu.rs index 4ac40bcac..a60354bd1 100644 --- a/apps/lightcraft/src/native_menu.rs +++ b/apps/lightcraft/src/native_menu.rs @@ -39,18 +39,86 @@ enum Handle { struct Item { handle: Handle, + /// The submenu the item sits in, to swap the item when its accelerator has to go (macOS). + parent: Submenu, cmd: String, params: Value, shortcut: Option, + /// The accelerator the item's shortcut maps to. accel: Option, + /// The accelerator the native item has now: `accel`, or `None` while it yields. + current: Option, label: String, enabled: bool, checked: Option, + /// Whether the item may be replaced to drop its key. Not in the Help menu: a swap there left + /// the old row and added a second one (muda 0.21, macOS 27), so those keys stay. + swappable: bool, +} + +impl Item { + fn native_label(&self) -> String { + lightcraft_ui_egui::menubar::display_item_label(&self.cmd, &self.params, &self.label).replace('&', "&&") + } + + /// Puts `want` on the native item. muda's `set_accelerator(None)` does nothing on macOS (the + /// old key equivalent stays and the menu still takes the key), so there the item is replaced + /// by an identical one without an accelerator, at the same position and with the same id, + /// text, enabled and checked state. Only done when the accelerator changes, never per frame. + fn set_accelerator(&mut self, want: Option) { + if self.current == want { + return; + } + self.current = want; + if cfg!(target_os = "macos") && want.is_none() && self.swappable && self.replace_without_accelerator() { + return; + } + let _ = match &self.handle { + Handle::Plain(h) => h.set_accelerator(want), + Handle::Check(h) => h.set_accelerator(want), + }; + } + + fn replace_without_accelerator(&mut self) -> bool { + let (id, old): (_, &dyn muda::IsMenuItem) = match &self.handle { + Handle::Plain(h) => (h.id().clone(), h), + Handle::Check(h) => (h.id().clone(), h), + }; + let Some(position) = self.parent.items().iter().position(|k| *k.id() == id) else { return false }; + let new = match &self.handle { + Handle::Plain(_) => Handle::Plain(MenuItem::with_id(id, self.native_label(), self.enabled, None)), + Handle::Check(h) => Handle::Check(CheckMenuItem::with_id(id, self.native_label(), self.enabled, h.is_checked(), None)), + }; + if self.parent.remove(old).is_err() { + return false; + } + let new_ref: &dyn muda::IsMenuItem = match &new { + Handle::Plain(h) => h, + Handle::Check(h) => h, + }; + if self.parent.insert(new_ref, position).is_err() { + // the old item is already gone: an item at the end beats a missing one + let _ = self.parent.append(new_ref); + } + self.handle = new; + true + } +} + +/// The accelerator a native item has: none while a shortcut is being recorded, and none for the +/// keys a focused text field needs. +fn wanted_accelerator(accel: Option, sc: &str, text_focus: bool, capturing: bool) -> Option { + if capturing || (text_focus && yields_to_text(sc)) { None } else { accel } } pub struct NativeMenu { menu: Menu, items: HashMap, + /// Settings… and Quit in the application submenu: ordinary items, so they give up ⌘, and ⌘Q + /// to the shortcut recorder like the rest. (The system's own items there and in File/Window, + /// such as Hide ⌘H, Close Window ⌘W and Minimize ⌘M, are muda predefined items whose key + /// cannot be changed or swapped.) + app_items: Vec, rx: Receiver, /// Structure of the installed menu (ids, kinds, submenu names); a change rebuilds it. structure: String, @@ -216,7 +284,15 @@ impl NativeMenu { let _ = tx.send(e.id.0); repaint.request_repaint(); })); - let mut m = NativeMenu { menu: Menu::new(), items: HashMap::new(), rx, structure: String::new(), text_focus: false, capturing: false }; + let mut m = NativeMenu { + menu: Menu::new(), + items: HashMap::new(), + app_items: Vec::new(), + rx, + structure: String::new(), + text_focus: false, + capturing: false, + }; m.rebuild(app); app.native_menu = true; m @@ -227,6 +303,7 @@ impl NativeMenu { self.structure = structure_of(&bar); self.menu = Menu::new(); self.items.clear(); + self.app_items.clear(); // the application menu let app_menu = Submenu::new("LightCraft", true); @@ -246,6 +323,22 @@ impl NativeMenu { &PredefinedMenuItem::separator(), &quit, ]); + for (handle, cmd, shortcut, text) in [(&settings, SETTINGS, SETTINGS_KEY, "Settings…"), (&quit, QUIT, "Cmd+Q", "Quit LightCraft")] { + let accel = accelerator(shortcut); + self.app_items.push(Item { + handle: Handle::Plain(handle.clone()), + parent: app_menu.clone(), + swappable: true, + cmd: cmd.to_string(), + params: Value::Null, + shortcut: Some(shortcut.to_string()), + accel, + current: accel, + label: text.to_string(), + enabled: true, + checked: None, + }); + } let _ = self.menu.append(&app_menu); for (title, nodes) in &bar { @@ -264,7 +357,9 @@ impl NativeMenu { .cloned() .collect(); let nodes = tidy_separators(nodes); - self.append_nodes(&sub, &nodes); + // Swapping an item in the Help menu leaves the old row in place and adds a second one + // (measured on macOS 27 with muda 0.21): the Help menu is not addressed row for row. + self.append_nodes(&sub, &nodes, title != "Help"); if title == "File" { // ⌘W, the system's own item let _ = sub.append_items(&[ @@ -290,7 +385,8 @@ impl NativeMenu { self.publish_shortcuts(app); } - fn append_nodes(&mut self, sub: &Submenu, nodes: &[MenuNode]) { + /// `swappable`: whether its items may be replaced to drop their key (see `Item::swappable`). + fn append_nodes(&mut self, sub: &Submenu, nodes: &[MenuNode], swappable: bool) { for n in nodes { match n { MenuNode::Separator => { @@ -298,7 +394,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, swappable); let _ = sub.append(&s); } MenuNode::Item { id, params, label, shortcut, enabled, checked } => { @@ -334,10 +430,13 @@ impl NativeMenu { key, Item { handle, + parent: sub.clone(), + swappable, cmd: id.clone(), params: params.clone(), shortcut: shortcut.clone(), accel, + current: accel, label: label.clone(), enabled: *enabled, checked: *checked, @@ -421,16 +520,13 @@ impl NativeMenu { if focus != self.text_focus || capturing != self.capturing { self.text_focus = focus; self.capturing = capturing; - for it in self.items.values() { + for it in self.items.values_mut().chain(self.app_items.iter_mut()) { let Some(sc) = it.shortcut.as_deref() else { continue }; if it.accel.is_none() { continue; } - let accel = if capturing || (focus && yields_to_text(sc)) { None } else { it.accel }; - let _ = match &it.handle { - Handle::Plain(h) => h.set_accelerator(accel), - Handle::Check(h) => h.set_accelerator(accel), - }; + let want = wanted_accelerator(it.accel, sc, focus, capturing); + it.set_accelerator(want); } self.publish_shortcuts(app); } @@ -463,6 +559,25 @@ mod tests { set_language(Locale::En); } + #[test] + fn accelerators_yield_to_text_focus_and_shortcut_recording() { + let g = accelerator("G"); + let cmd_s = accelerator("Cmd+S"); + assert_eq!(wanted_accelerator(g, "G", false, false), g); + assert_eq!(wanted_accelerator(g, "G", true, false), None, "typing a G in a field"); + assert_eq!(wanted_accelerator(cmd_s, "Cmd+S", true, false), cmd_s, "⌘S still saves while typing"); + assert_eq!(wanted_accelerator(cmd_s, "Cmd+S", false, true), None, "the editor records the key"); + assert_eq!(wanted_accelerator(cmd_s, "Cmd+S", true, true), None, "recording wins over typing"); + // the application menu's Settings… and Quit are tracked like the other items + for sc in [SETTINGS_KEY, "Cmd+Q"] { + let a = accelerator(sc); + assert!(a.is_some()); + assert_eq!(wanted_accelerator(a, sc, true, false), a, "{sc} stays while typing"); + assert_eq!(wanted_accelerator(a, sc, false, true), None, "{sc} goes while recording"); + } + assert_eq!(wanted_accelerator(None, "Escape", false, false), None, "no accelerator stays none"); + } + #[test] fn shortcuts_map_to_accelerators() { assert_eq!(accelerator("Cmd+Shift+Z"), Some(Accelerator::new(Modifiers::META | Modifiers::SHIFT, Code::KeyZ))); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 99f17561e..3e741604a 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-21 · **Change:** minor (LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-22 · **Change:** minor (MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **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 @@ -927,3 +927,4 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-16 | minor | LR-VIEW-PHOTOGRID: when the photos shown change by those an edit touched (an import's batch, a capture time, a crop), the grid reads only those photos' dates and shapes again instead of every photo of the list (a 16-photo batch at 181,749 photos: grid frame 101 ms → 7 ms); status unchanged | | 2026-10-19 | minor | LRC-LIB-LIBFOLDERS: the sidebar trees indent by a narrower step (14 / 12 / 10 / 8 pt) when the deepest open row would not fit the panel, instead of cutting its name or scrolling sideways at once (issue #2298); status unchanged (✅) | | 2026-10-15 | patch | LR-EXP-DIM: Width & Height (`fit: widthHeight`) limits the width and the height as the photo stands, beside Dimensions (W × H, either orientation), as in Lightroom (#1620); status unchanged (✅) | +| 2026-10-22 | minor | MENU-BAR: on macOS the native menu gives a key back to the window while a text field has focus or a shortcut is being recorded (muda's `set_accelerator(None)` does nothing there, so the menu item is now swapped for one without a key, and the key is put back on that item afterwards); status unchanged (✅) | diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2..cd48a7204 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,15 @@ # What's new in LightCraft +## 2026-10-22 + +### Keys reach text fields and the shortcut editor on macOS +- While a text field has focus, the native menu now lets go of the keys the field needs (`G`, the digits `0`–`9`, + `P`, `U` and the like), and while Keyboard Shortcuts is recording a key it lets go of all of them (⌘, and ⌘Q in + the LightCraft menu too), except the system's own items (Hide LightCraft ⌘H, Hide Others ⌥⌘H, Close Window ⌘W + and Minimize ⌘M) and the two keys of the Help menu (⌘/ and F1). Before, the menu item kept its key on macOS, so the + key could still run the menu command instead of reaching the field or the editor. The keys come back on the menu + items when the field loses focus or recording ends; the menu's order, checkmarks and enabled state are unchanged. + ## 2026-10-21 ### Sharper resampling in HDR and panorama merges