fix(macos): ask to save on Cmd-Q instead of quitting outright (#1943)

The native app menu's Quit item was muda's predefined quit, which calls
AppKit's terminate: and owns the Cmd-Q key equivalent, so the window never
got the close request the unsaved-changes prompt waits for. Quit is now a
regular menu item that sends the app.quit command, which closes the window
through the prompt, on the command's shortcut (Cmd-Q unless remapped).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Sam SandweissandClaude Opus 5.5 committed 2026-10-19 10:53:01 -07:00
1 parent 105c09bd80
commit 8eb2d43649
2 files changed
+51 -3

No files matched your search

+31 -3
View File
@@ -19,6 +19,13 @@ fn accel(s: &str) -> Option<Accelerator> {
mapped.parse().ok()
}
/// The app menu's Quit item: the `app.quit` command (which asks to save unsaved changes) on its
/// shortcut, Cmd+Q unless remapped in the Keyboard Shortcuts editor.
fn quit_item(app: &FilmcraftApp) -> (&'static str, Option<Accelerator>) {
let keys = app.session.shortcuts.primary("app.quit").unwrap_or_else(|| "Cmd+Q".into());
("app.quit", accel(&keys))
}
/// Updates native item labels, key equivalents and enablement when the menu tree changes.
pub type MenuUpdater = Box<dyn FnMut(&[Item])>;
@@ -53,9 +60,12 @@ pub fn install(app: &FilmcraftApp, ctx: egui::Context) -> (Receiver<String>, Men
let predefined = [
(hide_label, PredefinedMenuItem::hide(Some(app.ui.language.tr(hide_label)))),
(hide_others_label, PredefinedMenuItem::hide_others(Some(app.ui.language.tr(hide_others_label)))),
(quit_label, PredefinedMenuItem::quit(Some(app.ui.language.tr(quit_label)))),
];
let [(_, hide), (_, hide_others), (_, quit)] = &predefined;
let [(_, hide), (_, hide_others)] = &predefined;
// Quit is ours, not the predefined item: that one calls `terminate:` and skips the unsaved
// changes prompt (#1943); the `app.quit` command closes the window through it
let (quit_id, quit_accel) = quit_item(app);
let quit = MenuItem::with_id(quit_id, app.ui.language.tr(quit_label), true, quit_accel);
let _ = app_menu.append_items(&[
&about,
&discord,
@@ -65,7 +75,7 @@ pub fn install(app: &FilmcraftApp, ctx: egui::Context) -> (Receiver<String>, Men
hide,
hide_others,
&PredefinedMenuItem::separator(),
quit,
&quit,
]);
let _ = bar.append(&app_menu);
for top in MENUS {
@@ -147,6 +157,7 @@ pub fn install(app: &FilmcraftApp, ctx: egui::Context) -> (Receiver<String>, Men
for (label, item) in &predefined {
item.set_text(language.tr(label));
}
quit.set_text(language.tr(quit_label));
for (shown, mi) in &mut native {
let Some(it) = items.iter().find(|i| i.id == shown.id) else { continue };
if it.label != shown.label {
@@ -164,3 +175,20 @@ pub fn install(app: &FilmcraftApp, ctx: egui::Context) -> (Receiver<String>, Men
});
(rx, update)
}
#[cfg(test)]
mod tests {
use super::*;
use filmcraft_engine::Session;
/// #1943: Cmd+Q must reach the `app.quit` command, which closes the window through the unsaved
/// changes prompt, instead of the predefined Quit item's `terminate:`.
#[test]
fn quit_item_runs_the_app_quit_command_on_cmd_q() {
let app = FilmcraftApp::new(Session::default());
let (id, accelerator) = quit_item(&app);
assert_eq!(id, "app.quit");
assert_eq!(accelerator, accel("Cmd+Q"));
assert!(accelerator.is_some(), "Cmd+Q is a native key equivalent");
}
}
+20
View File
@@ -347,6 +347,26 @@ fn quitting_with_unsaved_changes_asks_to_save() {
assert!(!d.sent(&egui::ViewportCommand::CancelClose), "the confirmed quit goes through");
}
/// #1943: the macOS menu bar's Quit (Cmd+Q) sends `app.quit` to the command inbox; with unsaved
/// changes that closes the window through the prompt instead of quitting outright.
#[test]
fn native_menu_quit_with_unsaved_changes_asks_to_save() {
let mut d = Driver::new();
d.edit();
let (tx, rx) = channel();
d.app().command_inbox = Some(rx);
tx.send("app.quit".to_string()).unwrap();
d.frames(1);
assert!(d.sent(&egui::ViewportCommand::Close), "Quit asks the window to close");
assert!(!d.app().quit_confirmed, "nothing is confirmed yet");
// the window's close request is cancelled and the prompt asks instead
d.request_close();
assert!(d.sent(&egui::ViewportCommand::CancelClose), "the window must stay open");
d.frames(2);
assert!(d.has("quit.save") && d.has("quit.dontSave") && d.has("quit.cancel"));
assert!(d.app().session.is_dirty());
}
#[test]
fn quit_save_on_a_new_project_uses_the_save_dialog_and_scripted_quit_does_not_ask() {
let path = std::env::temp_dir().join(format!("fc-quit-save-{}.fcproj", std::process::id()));