From 50eddc1207ad4e934598562f6fb47d16ef0f0d16 Mon Sep 17 00:00:00 2001 From: pravdomil <2387356+pravdomil@users.noreply.github.com> Date: Sun, 11 Oct 2026 23:35:37 +0200 Subject: [PATCH] UI: keyboard shortcuts for dialog answers Unsaved Changes: Enter is Save, Cmd+Backspace is Don't Save. Revert Project: Cmd+R is Revert (the button is now red). Delete Items: Cmd+D is Delete. Crash recovery: Enter is Open Auto-Save. Marker: Enter is OK, Cmd+Backspace is Delete (from a focused text field too). The macOS menu bar hands Cmd+D and Cmd+R back to the open prompt. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/native_menu.rs | 37 +++++++++++ crates/ui-egui/src/panels/delete_items.rs | 23 +++++++ crates/ui-egui/src/panels/dialogs.rs | 29 +++++++++ crates/ui-egui/src/panels/markers_ui.rs | 25 ++++++- crates/ui-egui/src/panels/settings.rs | 3 + crates/ui-egui/src/panels/unsaved.rs | 79 +++++++++++++++++++++-- 6 files changed, 190 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/native_menu.rs b/crates/ui-egui/src/native_menu.rs index e727b9f9..af051c4a 100644 --- a/crates/ui-egui/src/native_menu.rs +++ b/crates/ui-egui/src/native_menu.rs @@ -345,6 +345,17 @@ fn text_field_event(command: &str, clipboard: Option) -> Option Result { let item = menu.find(id).ok_or_else(|| format!("no native menu item `{id}`"))?; let (command, params) = (item.command.clone(), item.params.clone()); + // The menu bar takes Cmd+D and Cmd+R before the window sees them: while a confirmation that + // answers to the key is open, hand it back. Choosing the item with the mouse (no Cmd held) + // still runs it. + if let Some(key) = crate::panels::dialogs::command_key(app) + && item.accelerator.as_deref().is_some_and(|a| ["Cmd+", "Ctrl+"].iter().any(|m| a.strip_prefix(m) == Some(key.name()))) + && ctx.input(|i| i.modifiers.command) + { + app.synthetic.push(egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers: egui::Modifiers::COMMAND }); + ctx.request_repaint(); + return Ok(Value::Null); + } if ctx.egui_wants_keyboard_input() { let clip = if command == "edit.paste" { app.hooks.clipboard_text.as_ref().and_then(|f| f()) } else { None }; if let Some(events) = text_field_event(&command, clip) { @@ -469,6 +480,32 @@ mod tests { } } + #[test] + fn a_confirmation_gets_its_command_key_back_from_the_menu_bar() { + let mut app = app(); + let spec = build(&app); + let dup = spec.items().into_iter().find(|i| i.command == "edit.duplicate").map(|i| i.id.clone()).unwrap(); + let ctx = egui::Context::default(); + app.dialog_state.delete_items.command = Some((json!({"items": []}), json!({"items": 1, "layers": 1, "comps": 1}))); + app.dialog = Some(crate::Dialog::DeleteItems); + // Chosen with the mouse, Duplicate still runs. + ctx.run_ui(Default::default(), |ui| { + let _ = activate(&mut app, ui.ctx(), &spec, &dup); + }) + .textures_delta + .clear(); + assert!(app.synthetic.is_empty()); + // Cmd+D goes to the prompt instead. + let mut raw = egui::RawInput::default(); + raw.events.push(egui::Event::ModifiersChanged(egui::Modifiers::COMMAND)); + ctx.run_ui(raw, |ui| { + let _ = activate(&mut app, ui.ctx(), &spec, &dup); + }) + .textures_delta + .clear(); + assert!(matches!(app.synthetic.as_slice(), [egui::Event::Key { key: egui::Key::D, pressed: true, .. }])); + } + #[test] fn accelerators_convert() { assert_eq!(accelerator("Cmd+Shift+S").as_deref(), Some("Cmd+Shift+S")); diff --git a/crates/ui-egui/src/panels/delete_items.rs b/crates/ui-egui/src/panels/delete_items.rs index ca07fd18..b093fb9a 100644 --- a/crates/ui-egui/src/panels/delete_items.rs +++ b/crates/ui-egui/src/panels/delete_items.rs @@ -74,6 +74,10 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { if answer.is_none() && ctx.input(|i| i.key_pressed(egui::Key::Escape)) { answer = Some(false); } + // Cmd+D (Ctrl elsewhere) is Delete. + if answer.is_none() && super::dialogs::ui_command_key(ctx, egui::Key::D) { + answer = Some(true); + } let Some(delete) = answer else { return }; app.dialog = None; app.dialog_state.delete_items.command = None; @@ -89,3 +93,22 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { pub fn delete_confirmed(app: &mut EffectcraftApp, ctx: &egui::Context, params: Value) -> Result { crate::menus::run_engine(app, ctx, "project.delete", params) } + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn cmd_d_is_delete_and_enter_is_not() { + let ctx = egui::Context::default(); + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + crate::theme::install(&ctx, &app.tokens, crate::i18n::language(&app)); + app.dialog_state.delete_items.command = Some((json!({"items": []}), json!({"items": 1, "layers": 1, "comps": 1}))); + app.dialog = Some(Dialog::DeleteItems); + crate::panels::dialogs::press_key(&mut app, &ctx, egui::Key::Enter, egui::Modifiers::NONE); + assert_eq!(app.dialog, Some(Dialog::DeleteItems)); + crate::panels::dialogs::press_key(&mut app, &ctx, egui::Key::D, egui::Modifiers::COMMAND); + assert!(app.dialog.is_none() && app.dialog_state.delete_items.command.is_none(), "Cmd+D answered Delete"); + } +} diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 81fa6bab..9de8d879 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -334,6 +334,35 @@ fn comp_settings(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens, exis } } +/// One frame of the open dialog with `key` pressed. +#[cfg(test)] +pub(crate) fn press_key(app: &mut EffectcraftApp, ctx: &egui::Context, key: egui::Key, modifiers: egui::Modifiers) { + let mut raw = egui::RawInput::default(); + raw.events.push(egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }); + let mut out = ctx.run_ui(raw, |ui| { + app.auto.begin_frame(); + show(app, ui.ctx()); + }); + out.textures_delta.clear(); +} + +/// The letter that answers the open confirmation with Cmd (Ctrl elsewhere): Cmd+D is Delete in +/// Delete Items, Cmd+R is Revert in Revert Project. The native menu bar hands these keys back +/// while the prompt is open ([`crate::native_menu::activate`]). +pub(crate) fn command_key(app: &EffectcraftApp) -> Option { + match app.dialog? { + Dialog::DeleteItems => Some(egui::Key::D), + Dialog::UnsavedChanges if super::unsaved::is_revert(app) => Some(egui::Key::R), + _ => None, + } +} + +/// Cmd+`key` (Ctrl elsewhere) pressed, also while a text field has focus: ask before the dialog +/// is drawn, so the field does not edit with the key first. +pub(crate) fn ui_command_key(ctx: &egui::Context, key: egui::Key) -> bool { + ctx.input_mut(|i| i.consume_key(egui::Modifiers::COMMAND, key)) +} + pub(crate) fn ui_enter(ctx: &egui::Context) -> bool { !ctx.egui_wants_keyboard_input() && ctx.input(|i| i.key_pressed(egui::Key::Enter)) } diff --git a/crates/ui-egui/src/panels/markers_ui.rs b/crates/ui-egui/src/panels/markers_ui.rs index c3631604..3be0ca67 100644 --- a/crates/ui-egui/src/panels/markers_ui.rs +++ b/crates/ui-egui/src/panels/markers_ui.rs @@ -124,6 +124,8 @@ fn text_field(ui: &mut egui::Ui, app: &mut EffectcraftApp, id: &str, v: &mut Str pub fn dialog(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { let mut d = app.dialog_state.marker.clone(); let (mut ok, mut cancel, mut delete) = (false, false, false); + // Cmd+Backspace (Ctrl elsewhere) is Delete, in a text field too. + let delete_key = super::dialogs::ui_command_key(ctx, egui::Key::Backspace); let title = d.title.clone(); super::dialogs::modal(ctx, &title, vec2(520.0, 560.0), t, |ui| { egui::Grid::new("marker-grid").num_columns(2).spacing([12.0, 8.0]).show(ui, |ui| { @@ -217,7 +219,9 @@ pub fn dialog(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { }); }); app.dialog_state.marker = d.clone(); - if ok { + delete |= delete_key && !ok && !cancel; + // Enter is OK, unless a text field has it. + if ok || (!cancel && !delete && super::dialogs::ui_enter(ctx)) { if let Err(e) = app.session.execute("markers.set", draft_params(&d)) { app.ui.status = e.to_string(); } @@ -496,5 +500,24 @@ mod tests { assert_eq!(m["label"], "Blue"); app.session.undo(); assert_eq!(app.session.execute("markers.list", json!({})).unwrap()[0]["chapter"], ""); + // Enter is OK and Cmd+Backspace is Delete. + let ctx = egui::Context::default(); + crate::theme::install(&ctx, &app.tokens, crate::i18n::language(&app)); + app.dialog_state.marker.chapter = "Entered".into(); + crate::panels::dialogs::press_key(&mut app, &ctx, egui::Key::Enter, egui::Modifiers::NONE); + assert!(app.dialog.is_none()); + assert_eq!(app.session.execute("markers.list", json!({})).unwrap()[0]["chapter"], "Entered"); + open_dialog(&mut app, MarkerRef { layer: None, index: 0 }).unwrap(); + // Also from a focused text field: Tab to one, then press the key. + for _ in 0..40 { + if ctx.egui_wants_keyboard_input() { + break; + } + crate::panels::dialogs::press_key(&mut app, &ctx, egui::Key::Tab, egui::Modifiers::NONE); + } + assert!(ctx.egui_wants_keyboard_input(), "a text field has focus"); + crate::panels::dialogs::press_key(&mut app, &ctx, egui::Key::Backspace, egui::Modifiers::COMMAND); + assert!(app.dialog.is_none()); + assert_eq!(app.session.execute("markers.list", json!({})).unwrap().as_array().map(Vec::len), Some(0), "deleted"); } } diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index 385febef..3efa206b 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -778,6 +778,9 @@ pub fn recovery(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { } }); }); + if choice.is_none() && super::dialogs::ui_enter(ctx) { + choice = Some("autosave"); + } let Some(c) = choice else { return }; app.dialog = None; app.recovery = None; diff --git a/crates/ui-egui/src/panels/unsaved.rs b/crates/ui-egui/src/panels/unsaved.rs index e403a9d6..05f719fa 100644 --- a/crates/ui-egui/src/panels/unsaved.rs +++ b/crates/ui-egui/src/panels/unsaved.rs @@ -5,7 +5,8 @@ //! Revert only asks to discard. Agents running commands with `engine.execute` are never asked. //! //! Automation ids: `dialog.unsaved.save`, `dialog.unsaved.dontSave` (Revert: `dialog.unsaved.revert`), -//! `dialog.unsaved.cancel`. +//! `dialog.unsaved.cancel`. Enter answers Save, Cmd+Backspace (Ctrl elsewhere) Don't Save and, in +//! the Revert prompt, Cmd+R Revert. use crate::i18n::tr; use egui::{Color32, vec2}; @@ -73,6 +74,11 @@ pub fn window_title(app: &EffectcraftApp) -> String { format!("EffectCraft - {doc}{}", if app.session.is_dirty() { " *" } else { "" }) } +/// Whether the prompt is Revert Project (Revert / Cancel) instead of Unsaved Changes. +pub(crate) fn is_revert(app: &EffectcraftApp) -> bool { + app.dialog_state.unsaved.command.as_ref().is_some_and(|(id, _)| id == "file.revert") +} + enum Answer { Save, Discard, @@ -80,11 +86,11 @@ enum Answer { } pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { - let Some((id, _)) = app.dialog_state.unsaved.command.clone() else { + if app.dialog_state.unsaved.command.is_none() { app.dialog = None; return; - }; - let revert = id == "file.revert"; + } + let revert = is_revert(app); let name = project_name(app); let mut answer = None; let title = if revert { "Revert Project" } else { "Unsaved Changes" }; @@ -102,7 +108,8 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.add_space(18.0); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { let (label, key) = if revert { (" Revert ", "dialog.unsaved.revert") } else { (" Save ", "dialog.unsaved.save") }; - let r = ui.add(egui::Button::new(egui::RichText::new(label).color(Color32::WHITE)).fill(t.accent)); + // Revert discards work: red, like Delete. + let r = ui.add(egui::Button::new(egui::RichText::new(label).color(Color32::WHITE)).fill(if revert { t.danger } else { t.accent })); app.auto.add(key, r.rect, label.trim()); if r.clicked() { answer = Some(if revert { Answer::Discard } else { Answer::Save }); @@ -126,6 +133,15 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { if answer.is_none() && ctx.input(|i| i.key_pressed(egui::Key::Escape)) { answer = Some(Answer::Cancel); } + // Enter is Save, the default button; Cmd+Backspace (Ctrl elsewhere) is Don't Save, as in + // macOS save prompts. Revert discards work, so it takes neither: Cmd+R. + if answer.is_none() && !revert && super::dialogs::ui_enter(ctx) { + answer = Some(Answer::Save); + } + let discard = if revert { egui::Key::R } else { egui::Key::Backspace }; + if answer.is_none() && super::dialogs::ui_command_key(ctx, discard) { + answer = Some(Answer::Discard); + } if let Some(a) = answer { answer_with(app, ctx, a); } @@ -227,6 +243,59 @@ mod tests { assert!(app.dialog.is_none() && app.session.project.comps().count() == 0); } + #[test] + fn cmd_backspace_is_dont_save_but_not_revert() { + let press = |app: &mut EffectcraftApp, ctx: &egui::Context| crate::panels::dialogs::press_key(app, ctx, egui::Key::Backspace, egui::Modifiers::COMMAND); + let (mut app, ctx) = app(); + modified(&mut app); + crate::menus::invoke(&mut app, &ctx, "file.newProject", json!({})).unwrap(); + assert_eq!(app.dialog, Some(Dialog::UnsavedChanges)); + press(&mut app, &ctx); + assert!(app.dialog.is_none() && !app.session.is_dirty()); + assert_eq!(app.session.project.comps().count(), 0, "closed without saving"); + // The Revert prompt has no Don't Save. + let dir = std::env::temp_dir().join(format!("ec-unsaved-key-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + modified(&mut app); + app.session.execute("file.saveAs", json!({"path": dir.join("Kept.ecproj").to_string_lossy()})).unwrap(); + app.session.execute("layer.newNull", json!({})).unwrap(); + crate::menus::invoke(&mut app, &ctx, "file.revert", json!({})).unwrap(); + press(&mut app, &ctx); + assert_eq!(app.dialog, Some(Dialog::UnsavedChanges)); + assert_eq!(app.session.active_comp().unwrap().layers.len(), 1, "not reverted"); + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn enter_is_save_and_cmd_r_is_revert() { + let press = |app: &mut EffectcraftApp, ctx: &egui::Context| crate::panels::dialogs::press_key(app, ctx, egui::Key::Enter, egui::Modifiers::NONE); + let (mut app, ctx) = app(); + let dir = std::env::temp_dir().join(format!("ec-unsaved-enter-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("Kept.ecproj"); + modified(&mut app); + app.session.execute("file.saveAs", json!({"path": path.to_string_lossy()})).unwrap(); + // The Revert prompt discards work: Enter leaves it open. + app.session.execute("layer.newNull", json!({})).unwrap(); + crate::menus::invoke(&mut app, &ctx, "file.revert", json!({})).unwrap(); + press(&mut app, &ctx); + assert_eq!(app.dialog, Some(Dialog::UnsavedChanges)); + assert_eq!(app.session.active_comp().unwrap().layers.len(), 1, "not reverted"); + // Cmd+R is Revert. + crate::panels::dialogs::press_key(&mut app, &ctx, egui::Key::R, egui::Modifiers::COMMAND); + assert!(app.dialog.is_none()); + assert_eq!(app.session.active_comp().unwrap().layers.len(), 0, "reverted"); + app.session.execute("layer.newNull", json!({})).unwrap(); + // Save: the change is written, then the project closes. + crate::menus::invoke(&mut app, &ctx, "file.closeProject", json!({})).unwrap(); + press(&mut app, &ctx); + assert!(app.dialog.is_none()); + assert_eq!(app.session.project.comps().count(), 0, "closed"); + app.session.execute("file.open", json!({"path": path.to_string_lossy()})).unwrap(); + assert_eq!(app.session.active_comp().unwrap().layers.len(), 1, "the null was saved"); + let _ = std::fs::remove_dir_all(dir); + } + #[test] fn save_saves_then_continues_and_revert_only_asks_to_discard() { let (mut app, ctx) = app();