MENU-BAR: give each native menu row its own id so repeated commands keep their rows and labels

This commit is contained in:
Nathaniel Ledford
2026-10-27 16:57:12 -04:00
parent 07bc2008d9
commit ebfd198ea0
3 changed files with 141 additions and 49 deletions
+133 -47
View File
@@ -303,6 +303,50 @@ fn tidy_separators(mut nodes: Vec<MenuNode>) -> Vec<MenuNode> {
nodes
}
/// The model's rows of one menu as the native menu installs them: About, Settings and Quit live in
/// the app menu on macOS.
fn install_nodes(nodes: &[MenuNode]) -> Vec<MenuNode> {
let kept = nodes.iter().filter(|n| !matches!(n, MenuNode::Item { id, .. } if id == "app.about" || id == SETTINGS || id == QUIT));
tidy_separators(kept.cloned().collect())
}
/// Native (muda) ids, one per menu row. muda ids must be unique, but the model may hold the same
/// command and params twice (`dialog.export` is File ▸ Export… and Export with Preset ▸ Custom…;
/// `dialog.newAlbum` is in File and in Photo ▸ Add to Album), so a repeated key gets an occurrence
/// suffix. The first row keeps the plain [`MenuNode::key`]. Rows are numbered in model order, so
/// the install and the per-frame sync, which both walk the model in that order, agree.
#[derive(Default)]
struct RowIds {
seen: HashMap<String, usize>,
}
impl RowIds {
/// The row's native id, and whether an earlier row already has the same command and params.
fn next(&mut self, id: &str, params: &Value) -> (String, bool) {
let key = MenuNode::key(id, params);
let n = self.seen.entry(key.clone()).or_insert(0);
*n += 1;
if *n == 1 { (key, false) } else { (format!("{key}#{n}"), true) }
}
}
/// The key a row is installed with. A repeated row has none: AppKit would list one key twice, and
/// the first row of the command keeps it (a click on either row runs the command).
fn row_accelerator(native_key: Option<&str>, repeated: bool) -> Option<Accelerator> {
native_key.filter(|s| !repeated && !CONTEXTUAL.contains(s)).and_then(accelerator)
}
/// Every item of the model with its native id, in model order.
fn native_rows<'a>(nodes: &'a [MenuNode], ids: &mut RowIds, out: &mut Vec<(String, &'a MenuNode)>) {
for n in nodes {
match n {
MenuNode::Item { id, params, .. } => out.push((ids.next(id, params).0, n)),
MenuNode::Submenu { children, .. } => native_rows(children, ids, out),
MenuNode::Separator => {}
}
}
}
impl NativeMenu {
/// Build the menu bar and install it as the application menu (main thread, after launch).
pub fn install(app: &mut LightcraftApp, ctx: &egui::Context) -> NativeMenu {
@@ -371,6 +415,7 @@ impl NativeMenu {
}
let _ = self.menu.append(&app_menu);
let mut ids = RowIds::default();
for (title, nodes) in &bar {
let sub = Submenu::new(label_text(title), true);
if title == "Window" {
@@ -380,16 +425,10 @@ impl NativeMenu {
&PredefinedMenuItem::separator(),
]);
}
// About and Settings live in the app menu on macOS
let nodes: Vec<MenuNode> = nodes
.iter()
.filter(|n| !matches!(n, MenuNode::Item { id, .. } if id == "app.about" || id == SETTINGS || id == QUIT))
.cloned()
.collect();
let nodes = tidy_separators(nodes);
let nodes = install_nodes(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, &app.ui.settings.keymap, title != "Help");
self.append_nodes(&sub, &nodes, &app.ui.settings.keymap, title != "Help", &mut ids);
if title == "File" {
// ⌘W, the system's own item
let _ = sub.append_items(&[
@@ -418,7 +457,7 @@ impl NativeMenu {
/// Install translated rows with their effective accelerators, preserving hint-only keys.
/// `swappable`: whether its items may be replaced to drop their key (see `Item::swappable`).
fn append_nodes(&mut self, sub: &Submenu, nodes: &[MenuNode], keymap: &lightcraft_ui_egui::shortcuts::Keymap, swappable: bool) {
fn append_nodes(&mut self, sub: &Submenu, nodes: &[MenuNode], keymap: &lightcraft_ui_egui::shortcuts::Keymap, swappable: bool, ids: &mut RowIds) {
for n in nodes {
match n {
MenuNode::Separator => {
@@ -426,16 +465,13 @@ impl NativeMenu {
}
MenuNode::Submenu { label, children } => {
let s = Submenu::new(label_text(label), true);
self.append_nodes(&s, children, keymap, swappable);
self.append_nodes(&s, children, keymap, swappable, ids);
let _ = sub.append(&s);
}
MenuNode::Item { id, params, label, shortcut, enabled, checked } => {
let key = MenuNode::key(id, params);
if self.items.contains_key(&key) {
continue; // muda ids must be unique
}
let (key, repeated) = ids.next(id, params);
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 accel = row_accelerator(native_key, repeated);
let handle = match checked {
Some(c) => {
let it = CheckMenuItem::with_id(key.clone(), native_label.replace('&', "&&"), *enabled, *c, accel);
@@ -508,41 +544,37 @@ impl NativeMenu {
// to, so the sync below runs in this same frame and no key reaches the menu in between)
self.rebuild(app);
}
/// Synchronize live labels and states; accelerator ownership changes already triggered a rebuild.
fn walk(nodes: &[MenuNode], items: &mut HashMap<String, Item>, keymap: &lightcraft_ui_egui::shortcuts::Keymap) {
for n in nodes {
match n {
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 };
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(native_label.replace('&', "&&")),
Handle::Check(h) => h.set_text(native_label.replace('&', "&&")),
}
it.label = native_label;
}
if it.enabled != *enabled {
match &it.handle {
Handle::Plain(h) => h.set_enabled(*enabled),
Handle::Check(h) => h.set_enabled(*enabled),
}
it.enabled = *enabled;
}
if let (Handle::Check(h), Some(c)) = (&it.handle, checked) {
// also resyncs after AppKit toggled it on click
if h.is_checked() != *c {
h.set_checked(*c);
}
it.checked = Some(*c);
}
}
MenuNode::Separator => {}
// Synchronize live labels and states row by row (accelerator ownership changes already
// triggered a rebuild).
let all: Vec<MenuNode> = bar.into_iter().flat_map(|(_, v)| v).collect();
let mut rows = Vec::new();
native_rows(&all, &mut RowIds::default(), &mut rows);
for (native_id, node) in rows {
let MenuNode::Item { id, params, label, shortcut, enabled, checked } = node else { continue };
let Some(it) = self.items.get_mut(&native_id) else { continue };
let (native_label, _) = native_item_presentation(&app.ui.settings.keymap, id, params, label, shortcut.as_deref());
if it.label != native_label {
match &it.handle {
Handle::Plain(h) => h.set_text(native_label.replace('&', "&&")),
Handle::Check(h) => h.set_text(native_label.replace('&', "&&")),
}
it.label = native_label;
}
if it.enabled != *enabled {
match &it.handle {
Handle::Plain(h) => h.set_enabled(*enabled),
Handle::Check(h) => h.set_enabled(*enabled),
}
it.enabled = *enabled;
}
if let (Handle::Check(h), Some(c)) = (&it.handle, checked) {
// also resyncs after AppKit toggled it on click
if h.is_checked() != *c {
h.set_checked(*c);
}
it.checked = Some(*c);
}
}
walk(&bar.iter().flat_map(|(_, v)| v.clone()).collect::<Vec<_>>(), &mut self.items, &app.ui.settings.keymap);
let focus = ctx.egui_wants_keyboard_input();
let capturing = app.recording_shortcut.is_some();
@@ -587,6 +619,60 @@ mod tests {
set_language(Locale::En);
}
/// Every row of the model gets its own native row and keeps its own label, and a click on a
/// native id runs that row's command (two rows may share a command and params).
#[test]
fn rows_sharing_a_command_get_their_own_native_ids_and_labels() {
let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Default::default());
let bar = menu_bar(&app);
let all: Vec<MenuNode> = bar.iter().flat_map(|(_, v)| v.clone()).collect();
let mut rows = Vec::new();
native_rows(&all, &mut RowIds::default(), &mut rows);
fn count(n: &[MenuNode]) -> usize {
n.iter()
.map(|x| match x {
MenuNode::Item { .. } => 1,
MenuNode::Submenu { children, .. } => count(children),
MenuNode::Separator => 0,
})
.sum()
}
assert_eq!(rows.len(), count(&all));
let ids: HashSet<&str> = rows.iter().map(|(id, _)| id.as_str()).collect();
assert_eq!(ids.len(), rows.len(), "a native id per row");
// The install walks each menu as `install_nodes` leaves it, the per-frame sync walks the whole
// bar: a row the menu has must get the same id, so the same node (label, command, params),
// from both, and a click on its id runs that row's command.
let mut installed = Vec::new();
let mut install_ids = RowIds::default();
let menus: Vec<Vec<MenuNode>> = bar.iter().map(|(_, nodes)| install_nodes(nodes)).collect();
for nodes in &menus {
native_rows(nodes, &mut install_ids, &mut installed);
}
let synced: HashMap<&str, &MenuNode> = rows.iter().map(|(id, n)| (id.as_str(), *n)).collect();
for (nid, n) in &installed {
assert_eq!(synced.get(nid.as_str()), Some(n), "{nid}");
}
for (cmd, labels) in [("dialog.export", ["Export…", "Custom…"]), ("dialog.newAlbum", ["New Album…", "New Album…"])] {
let own: Vec<(&str, &str)> = installed
.iter()
.filter_map(|(nid, n)| match n {
MenuNode::Item { id, label, params, .. } if id == cmd && params.is_null() => Some((nid.as_str(), label.as_str())),
_ => None,
})
.collect();
assert_eq!(own.len(), 2, "{cmd} is in two menus");
assert_ne!(own[0].0, own[1].0, "{cmd}: one native id per row");
assert_eq!([own[0].1, own[1].1], labels, "{cmd}: each row keeps its label");
}
// A repeated row is installed without its key (AppKit would list one key twice).
assert_eq!(row_accelerator(Some("G"), false), accelerator("G"));
assert_eq!(row_accelerator(Some("G"), true), None);
let mut ids = RowIds::default();
assert_eq!(ids.next("view.layoutImage", &Value::Null), ("view.layoutImage".to_string(), false));
assert_eq!(ids.next("view.layoutImage", &Value::Null), ("view.layoutImage#2".to_string(), true));
}
#[test]
fn accelerators_yield_to_text_focus_and_shortcut_recording() {
let g = accelerator("G");
+3 -2
View File
@@ -1,6 +1,6 @@
# Lightroom parity checklist
> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-25 · **Change:** minor (LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; 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; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; 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
> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-27 · **Change:** minor (MENU-BAR: on macOS two menu rows that run the same command each get their own native row and label (File ▸ Export… and File ▸ Export with Preset ▸ Custom…; File ▸ New Album… and Photo ▸ Add to Album ▸ New Album…); LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; 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; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; 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
@@ -494,7 +494,7 @@ visible menu bar yet: the menu model is only exposed through the control channel
| Id | Feature | Tier | Status | Evidence | Missing / notes |
|---|---|---|---|---|---|
| MENU-BAR | Menu bar rendering | P1 | ✅ | `crates/ui-egui/src/menus/menubar.rs`, `apps/lightcraft/src/native_menu.rs` | native macOS menu bar (muda) with live labels/enabled/checked; in-window menus on web/Windows/Linux (with one menu open, the pointer on another title switches to it, as in native menu bars); ⌫ and X stay egui-handled (contextual), so they show no key in the native menu; a scrolled menu draws no scrollbar over its rows: the ▲ / ▼ strips, the wheel and dragging scroll it (issue #691: egui's floating bar hid the shortcut text's last character; `crates/ui-egui/src/tests/menubar.rs`) |
| MENU-BAR | Menu bar rendering | P1 | ✅ | `crates/ui-egui/src/menus/menubar.rs`, `apps/lightcraft/src/native_menu.rs` | native macOS menu bar (muda) with live labels/enabled/checked; in-window menus on web/Windows/Linux (with one menu open, the pointer on another title switches to it, as in native menu bars); ⌫ and X stay egui-handled (contextual), so they show no key in the native menu; a scrolled menu draws no scrollbar over its rows: the ▲ / ▼ strips, the wheel and dragging scroll it (issue #691: egui's floating bar hid the shortcut text's last character; `crates/ui-egui/src/tests/menubar.rs`); on macOS every row of the model has its own native row, also when two rows run the same command with the same parameters (the second one has no key of its own) |
| MENU-APP-ABOUT | About | P2 | ✅ | `cmd:app.about` | |
| MENU-APP-SETTINGS | Settings… | P0 | ✅ | `cmd:app.settings` | app menu on macOS, Edit menu elsewhere |
| MENU-APP-UPDATES | Check for updates | P2 | ⬜ | | |
@@ -959,3 +959,4 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove
| 2026-10-23 | patch | LR-BEHAV-ACCESS: the name field of New Album, New Smart Album, Rename and the other one-field dialogs takes the keyboard focus once when the dialog opens instead of every frame, so Tab and Shift+Tab reach the check box, Cancel and OK; status unchanged (🟡) |
| 2026-10-22 | minor | LR-CROP-AUTO: Auto straighten turns by at most 10° and only on at least 3 agreeing lines, and says when it finds nothing to level by; measured on 68 photos straightened by hand and 300 left as shot, unwanted turns of more than 1° fell from 79 to 19 of the 300. Downgraded ✅ → 🟡: it still leaves most tilted photos alone |
| 2026-10-23 | patch | LRC-LIB-FILTERBAR: showing an album (`library.source`) while the filter holds an album drops the filter's album, as showing a library folder already dropped its folder filter; before, the old album stayed in the filter unseen (the filter bar has no album chip) and narrowed the next source such as All Photos; the row's note says so; status unchanged (✅) |
| 2026-10-27 | patch | MENU-BAR: on macOS two menu rows that run the same command with the same parameters each get their own native row and label; before, the second was left out and the first took its label, so File ▸ Export… read “Custom…”, Export with Preset lost its Custom… row and Add to Album lost New Album…; status unchanged (✅) |
+5
View File
@@ -1,5 +1,10 @@
# What's new in LightCraft
## 2026-10-27
### The macOS menu bar shows every row
- File ▸ Export… no longer reads “Custom…”, File ▸ Export with Preset keeps its Custom… row, and Photo ▸ Add to Album keeps New Album…. When two menu rows ran the same command, the menu bar left the second one out and gave the first one its label; now each row is its own item and a click on either one runs the command. The View menu now also lists its second Layout Image row (without a key), as the in-window menu does.
## 2026-10-24
### Relative-date filters and smart albums follow the clock