Merge pull request #66 from storytold/fix/issue-27-quit-last

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

crates/ui-egui/src/menubar.rs

@@ -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();

docs/parity.md

@@ -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 | 🚫 | | |