Issue #27: Menus: keep Quit last in the in-window File menu

Registered File commands the explicit layout doesn't list were appended
after Quit LightCraft. Items in LAST (app.quit) are now held back and
placed after the fallback entries, in their own separator group. The
macOS native menu filters Quit into the app menu and is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas
2026-10-04 10:38:44 -04:00
co-authored by Claude Opus 5.5
parent fb8e1e93aa
commit 7c662bc5d6
2 changed files with 35 additions and 3 deletions
+34 -2
View File
@@ -47,7 +47,8 @@ impl MenuNode {
pub const MENUS: &[&str] = &["File", "Edit", "View", "Photo", "Window", "Help"];
/// Order and grouping per menu: command ids, `---` separators and `@Submenu` placeholders.
/// Entries with a menu path that aren't listed are appended at the end of their menu.
/// Entries with a menu path that aren't listed are appended at the end of their menu, before any
/// [`LAST`] item.
const LAYOUT: &[(&str, &[&str])] = &[
(
"File",
@@ -219,6 +220,10 @@ const LAYOUT: &[(&str, &[&str])] = &[
),
];
/// Items that always end their menu in their own separator group, after the entries appended
/// because the layout doesn't list them (Quit is the last item of the in-window File menu).
const LAST: &[&str] = &["app.quit"];
/// Registry entries that are reached another way (parameterized commands are expanded into
/// submenus below; others duplicate a dialog command).
const HIDDEN: &[&str] = &[
@@ -535,6 +540,7 @@ pub fn menu_bar(app: &LightcraftApp) -> Vec<(String, Vec<MenuNode>)> {
for title in MENUS {
let layout = LAYOUT.iter().find(|(t, _)| t == title).map(|(_, l)| *l).unwrap_or(&[]);
let mut items = Vec::new();
let mut last = Vec::new();
let sub = |name: &str, used: &mut [bool]| -> Vec<MenuNode> {
let mut children = expanded(app, name).unwrap_or_default();
let mut dialogs = Vec::new();
@@ -559,7 +565,7 @@ pub fn menu_bar(app: &LightcraftApp) -> Vec<(String, Vec<MenuNode>)> {
items.push(MenuNode::Submenu { label: name.to_string(), children });
} else if let Some(i) = entries.iter().position(|e| e.id == *slot && e.menu.first().map(String::as_str) == Some(title)) {
used[i] = true;
items.push(node(app, &entries[i]));
if LAST.contains(slot) { last.push(node(app, &entries[i])) } else { items.push(node(app, &entries[i])) }
}
}
// everything else that names this menu
@@ -580,6 +586,10 @@ pub fn menu_bar(app: &LightcraftApp) -> Vec<(String, Vec<MenuNode>)> {
let children = sub(&name, &mut used);
items.push(MenuNode::Submenu { label: name, children });
}
if !last.is_empty() {
items.push(MenuNode::Separator);
items.extend(last);
}
bar.push((title.to_string(), tidy(items)));
}
bar
@@ -773,6 +783,28 @@ mod tests {
assert_eq!(*checked, Some(app.ui.filmstrip));
}
#[test]
fn quit_ends_the_file_menu_after_unlisted_entries() {
let app = app();
// the File menu has registry entries the layout doesn't list (appended as fallbacks)
let listed = LAYOUT.iter().find(|(t, _)| *t == "File").unwrap().1;
let unlisted: Vec<String> = crate::menus::menu_entries(&app)
.into_iter()
.filter(|e| e.menu.first().map(String::as_str) == Some("File") && !listed.contains(&e.id.as_str()) && !HIDDEN.contains(&e.id.as_str()))
.map(|e| e.id)
.collect();
assert!(!unlisted.is_empty(), "no fallback File entries to order");
let bar = menu_bar(&app);
let file = &bar.iter().find(|(t, _)| t == "File").unwrap().1;
// Quit is the last item, alone in its separator group
assert!(matches!(file.last(), Some(MenuNode::Item { id, .. }) if id == "app.quit"), "{file:?}");
assert_eq!(file.get(file.len() - 2), Some(&MenuNode::Separator));
// every fallback entry is still reachable, before Quit
for id in &unlisted {
assert!(find(&file[..file.len() - 1], id).is_some(), "{id} missing before Quit");
}
}
#[test]
fn export_with_preset_submenu() {
let mut app = app();
+1 -1
View File
@@ -504,7 +504,7 @@ visible menu bar yet: the menu model is only exposed through the control channel
| MENU-APP-SYNC | Sync status / pause | OOS | 🚫 | | |
| MENU-APP-SIGNOUT | Sign out | OOS | 🚫 | | |
| MENU-APP-HIDE | Hide / hide others / show all | P1 | ✅ | `apps/lightcraft/src/native_menu.rs` | the system's own items in the app menu (⌘H, ⌥⌘H) |
| MENU-APP-QUIT | Quit | P0 | ✅ | `cmd:app.quit`, `apps/lightcraft/src/native_menu.rs` | macOS: app menu (native); elsewhere: File → Quit LightCraft |
| MENU-APP-QUIT | Quit | P0 | ✅ | `cmd:app.quit`, `apps/lightcraft/src/native_menu.rs` | macOS: app menu (native); elsewhere: File → Quit LightCraft, always the last item in its own group (after commands the File layout does not list) |
| MENU-FILE-ADDPHOTOS | Add Photos… | P0 | ✅ | `cmd:file.addPhotos` | |
| MENU-FILE-ADDFOLDER | Add Folder… | P0 | ✅ | `cmd:file.addFolder`, `cmd:library.importPreview` | folder picker (desktop) → the import review, subfolders included; `path` param for agents |
| MENU-FILE-MIGRATE | Migrate photos | OOS | 🚫 | | |