Merge pull request #1019 from pravdomil/claude/confirm-unguarded-deletes

# Conflicts:
#	crates/ui-egui/src/panels/shortcut_editor.rs

crates/ui-egui/src/i18n/ui.rs

@@ -513,6 +513,7 @@ const JAPANESE: &[(&str, &str)] = &[
     ("Welcome to EffectCraft", "EffectCraftへようこそ"),
     ("Width:", "幅:"),
     ("You can undo this.", "この操作は取り消せます。"),
+    ("You can't undo this.", "この操作は取り消せません。"),
     ("Your changes will be lost if you don't save them.", "保存しないと、変更内容は失われます。"),
     ("Zoom", "ズーム"),
     ("built-in", "組み込み"),
@@ -959,6 +960,7 @@ const SIMPLIFIED_CHINESE: &[(&str, &str)] = &[
     ("Welcome to EffectCraft", "欢迎使用 EffectCraft"),
     ("Width:", "宽度："),
     ("You can undo this.", "您可以撤销此操作。"),
+    ("You can't undo this.", "此操作无法撤销。"),
     ("Your changes will be lost if you don't save them.", "如果不存储，你的更改将会丢失。"),
     ("Zoom", "缩放"),
     ("built-in", "内置"),
@@ -1680,6 +1682,7 @@ const TRADITIONAL_CHINESE: &[(&str, &str)] = &[
     ("Welcome to EffectCraft", "歡迎使用 EffectCraft"),
     ("Width:", "寬度："),
     ("You can undo this.", "您可以復原此操作。"),
+    ("You can't undo this.", "此操作無法復原。"),
     ("Your changes will be lost if you don't save them.", "如果不儲存，你的更改將會丟失。"),
     ("Zoom", "縮放"),
     ("built-in", "內建"),
@@ -2415,6 +2418,7 @@ const UKRAINIAN: &[(&str, &str)] = &[
     ("Welcome to EffectCraft", "Ласкаво просимо до EffectCraft"),
     ("Width:", "Ширина:"),
     ("You can undo this.", "Це можна скасувати."),
+    ("You can't undo this.", "Це не можна скасувати."),
     ("Your changes will be lost if you don't save them.", "Якщо не зберегти зміни, їх буде втрачено."),
     ("Zoom", "Масштаб"),
     ("built-in", "вбудований"),

crates/ui-egui/src/lib.rs

@@ -97,6 +97,8 @@ pub enum Dialog {
     DeleteItems,
     /// A gradient property's Gradient Editor (`panels::gradient_editor`, `prop.editGradient`).
     GradientEditor,
+    /// Delete a user template from Home? (`panels::templates`).
+    DeleteTemplate,
 }
 
 /// The desktop's light or dark appearance where egui cannot see it (Wayland compositors that send

crates/ui-egui/src/menus.rs

@@ -1671,7 +1671,11 @@ fn handle_shortcuts_impl(app: &mut EffectcraftApp, ctx: &egui::Context, tab_only
     // Dialog cancellation owns Escape even when a text field has keyboard focus.
     if !tab_only && app.dialog.is_some() && ctx.input(|i| i.key_pressed(egui::Key::Escape)) {
         // A popup or colour picker takes Escape first (the picker goes back to its original colour).
-        if crate::panels::shortcut_editor::recording(app) || egui::Popup::is_any_open(ctx) || crate::color_picker::open_recently(ctx) {
+        if crate::panels::shortcut_editor::recording(app)
+            || crate::panels::shortcut_editor::confirming(app)
+            || egui::Popup::is_any_open(ctx)
+            || crate::color_picker::open_recently(ctx)
+        {
             return;
         }
         ctx.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape));
@@ -1780,8 +1784,9 @@ fn handle_shortcuts_impl(app: &mut EffectcraftApp, ctx: &egui::Context, tab_only
         }
         // Escape closes dialogs.
         if key == egui::Key::Escape && app.dialog.is_some() {
-            // Escape while recording a shortcut cancels the recording, not the editor.
-            if crate::panels::shortcut_editor::recording(app) {
+            // Escape while recording a shortcut cancels the recording, not the editor; so does
+            // Escape in its Delete Preset prompt.
+            if crate::panels::shortcut_editor::recording(app) || crate::panels::shortcut_editor::confirming(app) {
                 continue;
             }
             if app.dialog == Some(crate::Dialog::Info) {

crates/ui-egui/src/panels/dialogs.rs

@@ -65,6 +65,8 @@ pub struct DialogState {
     pub delete_items: super::delete_items::Pending,
     /// The Gradient Editor's draft and the property it edits.
     pub gradient: super::gradient_editor::Draft,
+    /// The user template the Delete Template prompt asks about: its id and name.
+    pub delete_template: Option<(String, String)>,
 }
 
 pub fn open_new_comp(app: &mut EffectcraftApp) {
@@ -230,6 +232,7 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context) {
         Dialog::UnsavedChanges => super::unsaved::show(app, ctx, &t),
         Dialog::DeleteItems => super::delete_items::show(app, ctx, &t),
         Dialog::GradientEditor => super::gradient_editor::show(app, ctx, &t),
+        Dialog::DeleteTemplate => super::templates::show_delete(app, ctx, &t),
     }
 }
 
@@ -401,11 +404,12 @@ pub(crate) fn press_key(app: &mut EffectcraftApp, ctx: &egui::Context, key: egui
 }
 
 /// 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
+/// Delete Items, Delete Template and Delete Preset, 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<egui::Key> {
     match app.dialog? {
-        Dialog::DeleteItems => Some(egui::Key::D),
+        Dialog::DeleteItems | Dialog::DeleteTemplate => Some(egui::Key::D),
+        Dialog::Shortcuts if super::shortcut_editor::confirming(app) => Some(egui::Key::D),
         Dialog::UnsavedChanges if super::unsaved::is_revert(app) => Some(egui::Key::R),
         _ => None,
     }

crates/ui-egui/src/panels/shortcut_editor.rs

@@ -12,7 +12,8 @@
 //! Automation ids: `shortcuts.preset`, `shortcuts.search`, `shortcuts.mod.<ctrl|cmd|alt|shift>`,
 //! `shortcuts.key.<Key>`, `shortcuts.row.<n>`, `shortcuts.record`, `shortcuts.assign`,
 //! `shortcuts.removeKey.<n>`, `shortcuts.resetCommand`, `shortcuts.duplicate`,
-//! `shortcuts.rename`, `shortcuts.delete`, `shortcuts.import`, `shortcuts.export`,
+//! `shortcuts.rename`, `shortcuts.delete` (asks first: `dialog.deletePreset.delete`,
+//! `dialog.deletePreset.cancel`), `shortcuts.import`, `shortcuts.export`,
 //! `shortcuts.resetPreset`, `shortcuts.close`.
 
 use crate::i18n::{tr, tr_args};
@@ -39,6 +40,8 @@ pub struct EditorState {
     pub pending: String,
     /// Rename field for the active preset.
     pub rename: Option<String>,
+    /// The preset the Delete Preset prompt asks about.
+    pub confirm_delete: Option<String>,
 }
 
 const APP_COLOR: Color32 = Color32::from_rgb(0x8e, 0x6c, 0xe8);
@@ -54,6 +57,11 @@ pub fn recording(app: &EffectcraftApp) -> bool {
     app.dialog == Some(Dialog::Shortcuts) && app.dialog_state.shortcuts.recording
 }
 
+/// `true` while Delete Preset asks over the editor (Escape answers it, not the editor).
+pub fn confirming(app: &EffectcraftApp) -> bool {
+    app.dialog == Some(Dialog::Shortcuts) && app.dialog_state.shortcuts.confirm_delete.is_some()
+}
+
 /// Shortcut text of a key event (`Cmd+Shift+K`).
 pub fn combo_text(m: egui::Modifiers, k: egui::Key) -> Option<String> {
     use egui::Key::*;
@@ -274,10 +282,11 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) {
                     st.rename = Some(active.clone());
                 }
             }
+            // A deleted preset can't be brought back: Delete asks first (the prompt below).
             let b = ui.add_enabled(custom, egui::Button::new(tr("Delete")));
             app.auto.add("shortcuts.delete", b.rect, "Delete");
             if b.clicked() {
-                run.push(("shortcuts.preset".into(), json!({"op": "delete", "name": active})));
+                st.confirm_delete = Some(active.clone());
             }
             let b = ui.add_enabled(custom, egui::Button::new(tr("Reset Preset")));
             app.auto.add("shortcuts.resetPreset", b.rect, "Reset Preset");
@@ -571,6 +580,41 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) {
             close |= r.clicked();
         });
     });
+    // Delete Preset, over the editor.
+    if let Some(name) = st.confirm_delete.clone() {
+        let mut answer: Option<bool> = None;
+        super::dialogs::modal(ctx, "Delete Preset", vec2(460.0, 150.0), t, |ui| {
+            ui.label(RichText::new(format!("Delete the preset “{name}”?")).color(t.tab_text_active));
+            ui.add_space(4.0);
+            ui.label(RichText::new(tr("You can't undo this.")).color(t.text_dim).size(11.0));
+            ui.add_space(18.0);
+            ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
+                let r = ui.add(egui::Button::new(RichText::new(tr("  Delete  ")).color(Color32::WHITE)).fill(t.danger));
+                app.auto.add("dialog.deletePreset.delete", r.rect, "Delete");
+                if r.clicked() {
+                    answer = Some(true);
+                }
+                let r = ui.button(tr("Cancel"));
+                app.auto.add("dialog.deletePreset.cancel", r.rect, "Cancel");
+                if r.clicked() {
+                    answer = Some(false);
+                }
+            });
+        });
+        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);
+        }
+        if let Some(delete) = answer {
+            st.confirm_delete = None;
+            if delete {
+                run.push(("shortcuts.preset".into(), json!({"op": "delete", "name": name})));
+            }
+        }
+    }
     app.dialog_state.shortcuts = st;
     for (cmd, params) in run {
         if let Err(e) = app.session.execute(&cmd, params) {
@@ -657,6 +701,49 @@ mod tests {
         assert_eq!(app.dialog, None);
     }
 
+    #[test]
+    fn deleting_a_preset_asks_first() {
+        fn click(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str) {
+            let [x, y, w, h] = app.auto.find(id).unwrap_or_else(|| panic!("{id} is not on screen")).rect;
+            let pos = pos2(x + w / 2.0, y + h / 2.0);
+            let button = |pressed| egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() };
+            for event in [egui::Event::PointerMoved(pos), button(true), button(false)] {
+                frame(app, ctx, vec![event]);
+            }
+            // Two more: the automation ids of the clicked frame age out.
+            frame(app, ctx, vec![]);
+            frame(app, ctx, vec![]);
+        }
+        let mut app = EffectcraftApp::new(effectcraft_engine::Session::default());
+        let ctx = egui::Context::default();
+        crate::theme::install(&ctx, &app.tokens, crate::i18n::language(&app));
+        app.session.execute("shortcuts.preset", json!({"op": "duplicate", "from": DEFAULT_PRESET, "name": "Mine"})).unwrap();
+        app.session.execute("shortcuts.preset", json!({"op": "select", "name": "Mine"})).unwrap();
+        let has = |app: &EffectcraftApp| app.session.keymaps.names().iter().any(|n| n == "Mine");
+        open(&mut app);
+        for _ in 0..3 {
+            frame(&mut app, &ctx, vec![]);
+        }
+        assert!(app.auto.find("dialog.deletePreset.delete").is_none());
+        click(&mut app, &ctx, "shortcuts.delete");
+        assert!(has(&app) && confirming(&app), "the click only asks");
+        click(&mut app, &ctx, "dialog.deletePreset.cancel");
+        assert!(has(&app) && app.auto.find("dialog.deletePreset.delete").is_none(), "Cancel keeps the preset");
+        assert_eq!(app.dialog, Some(Dialog::Shortcuts), "the editor stays open");
+        click(&mut app, &ctx, "shortcuts.delete");
+        click(&mut app, &ctx, "dialog.deletePreset.delete");
+        assert!(!has(&app), "confirmed: the preset is gone");
+        assert!(!confirming(&app) && app.dialog == Some(Dialog::Shortcuts));
+        // Cmd+D is Delete; Enter is not.
+        app.session.execute("shortcuts.preset", json!({"op": "duplicate", "from": DEFAULT_PRESET, "name": "Mine"})).unwrap();
+        app.session.execute("shortcuts.preset", json!({"op": "select", "name": "Mine"})).unwrap();
+        app.dialog_state.shortcuts.confirm_delete = Some("Mine".into());
+        crate::panels::dialogs::press_key(&mut app, &ctx, egui::Key::Enter, egui::Modifiers::NONE);
+        assert!(has(&app) && confirming(&app));
+        crate::panels::dialogs::press_key(&mut app, &ctx, egui::Key::D, egui::Modifiers::COMMAND);
+        assert!(!has(&app) && !confirming(&app));
+    }
+
     #[test]
     fn recording_a_shortcut_does_not_also_type_the_key_into_the_field() {
         let mut app = EffectcraftApp::new(effectcraft_engine::Session::default());

crates/ui-egui/src/panels/templates.rs

@@ -4,7 +4,10 @@
 //! are rendered by our own renderer, one per frame, the first time the gallery shows them.
 //!
 //! Automation ids: `home.templates.<id>` (card), `home.templates.<id>.open`,
-//! `home.templates.<id>.delete` (user templates), `home.templates.saveCurrent`.
+//! `home.templates.<id>.delete` (user templates), `home.templates.saveCurrent`. Deleting a user
+//! template removes its file and can't be undone, so the card's delete button asks first
+//! (`dialog.deleteTemplate.delete`, `dialog.deleteTemplate.cancel`); agents running
+//! `templates.delete` with `engine.execute` are never asked.
 
 use egui::{Align2, Color32, Rect, Sense, Stroke, StrokeKind, pos2, vec2};
 use serde_json::json;
@@ -13,7 +16,7 @@ use effectcraft_engine::templates::{self, THUMB_H, THUMB_W, TemplateInfo};
 
 use crate::icons::{self, Icon};
 use crate::theme::Tokens;
-use crate::{EffectcraftApp, widgets};
+use crate::{Dialog, EffectcraftApp, widgets};
 
 /// The gallery's entries (re-listed when the user template files change).
 pub fn entries(app: &mut EffectcraftApp) -> Vec<TemplateInfo> {
@@ -87,7 +90,7 @@ pub fn home_tab(app: &mut EffectcraftApp, ui: &mut egui::Ui, area: Rect) {
     let card_h = thumb_h + 62.0;
     let view = Rect::from_min_max(pos2(x, top), area.max);
     let mut open: Option<String> = None;
-    let mut delete: Option<String> = None;
+    let mut delete: Option<(String, String)> = None;
     let mut budget = true;
     let mut child = ui.new_child(egui::UiBuilder::new().max_rect(view).layout(egui::Layout::top_down(egui::Align::Min)));
     egui::ScrollArea::vertical().id_salt("home-templates-scroll").auto_shrink([false, false]).show(&mut child, |ui| {
@@ -142,7 +145,7 @@ pub fn home_tab(app: &mut EffectcraftApp, ui: &mut egui::Ui, area: Rect) {
                 app.auto.add(&format!("{base}.delete"), dr, "Delete Template");
                 if dresp.clicked() {
                     del_hit = true;
-                    delete = Some(tpl.id.clone());
+                    delete = Some((tpl.id.clone(), tpl.name.clone()));
                 }
             }
             if resp.clicked() && !del_hit {
@@ -150,13 +153,9 @@ pub fn home_tab(app: &mut EffectcraftApp, ui: &mut egui::Ui, area: Rect) {
             }
         }
     });
-    if let Some(id) = delete {
-        match app.session.execute("templates.delete", json!({"id": id})) {
-            Ok(_) => {
-                app.template_thumbs.remove(&id);
-            }
-            Err(e) => app.ui.status = e.to_string(),
-        }
+    if let Some(pending) = delete {
+        app.dialog_state.delete_template = Some(pending);
+        app.dialog = Some(Dialog::DeleteTemplate);
     } else if let Some(id) = open {
         // A modified project asks to save first (the template opens once answered).
         if super::unsaved::guard(app, "templates.create", &json!({"id": id})) {
@@ -169,6 +168,52 @@ pub fn home_tab(app: &mut EffectcraftApp, ui: &mut egui::Ui, area: Rect) {
     }
 }
 
+/// Delete Template: the user template waiting on the prompt (`dialog_state.delete_template`, its
+/// id and name).
+pub fn show_delete(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) {
+    let Some((id, name)) = app.dialog_state.delete_template.clone() else {
+        app.dialog = None;
+        return;
+    };
+    let mut answer: Option<bool> = None;
+    super::dialogs::modal(ctx, "Delete Template", vec2(460.0, 150.0), t, |ui| {
+        ui.label(egui::RichText::new(format!("Delete the template “{name}”?")).color(t.tab_text_active));
+        ui.add_space(4.0);
+        ui.label(egui::RichText::new(crate::i18n::tr("You can't undo this.")).color(t.text_dim).size(11.0));
+        ui.add_space(18.0);
+        ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
+            let r = ui.add(egui::Button::new(egui::RichText::new(crate::i18n::tr("  Delete  ")).color(Color32::WHITE)).fill(t.danger));
+            app.auto.add("dialog.deleteTemplate.delete", r.rect, "Delete");
+            if r.clicked() {
+                answer = Some(true);
+            }
+            let r = ui.button(crate::i18n::tr("Cancel"));
+            app.auto.add("dialog.deleteTemplate.cancel", r.rect, "Cancel");
+            if r.clicked() {
+                answer = Some(false);
+            }
+        });
+    });
+    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_template = None;
+    if delete {
+        match app.session.execute("templates.delete", json!({"id": id})) {
+            Ok(_) => {
+                app.template_thumbs.remove(&id);
+            }
+            Err(e) => app.ui.status = e.to_string(),
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -185,6 +230,27 @@ mod tests {
         .clear();
     }
 
+    /// Click the widget registered as `id`, then draw the frame that follows.
+    fn click(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str) {
+        let [x, y, w, h] = app.auto.find(id).unwrap_or_else(|| panic!("{id} is not on screen")).rect;
+        let pos = pos2(x + w / 2.0, y + h / 2.0);
+        let button = |pressed| egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() };
+        for event in [egui::Event::PointerMoved(pos), button(true), button(false)] {
+            ctx.run_ui(egui::RawInput { events: vec![event], ..Default::default() }, |ui| {
+                app.auto.begin_frame();
+                if app.ui.start_screen {
+                    crate::panels::home::show(app, ui, Rect::from_min_size(pos2(0.0, 0.0), vec2(1280.0, 860.0)));
+                }
+                crate::panels::dialogs::show(app, ui.ctx());
+            })
+            .textures_delta
+            .clear();
+        }
+        // Two more: a new dialog is laid out, and the automation ids of the clicked frame age out.
+        frame(app, ctx);
+        frame(app, ctx);
+    }
+
     fn app_with_store() -> EffectcraftApp {
         let s = effectcraft_engine::Session { config: Some(std::sync::Arc::new(effectcraft_engine::config::MemoryConfig::default())), ..Default::default() };
         EffectcraftApp::new(s)
@@ -240,10 +306,23 @@ mod tests {
         app.session.execute("templates.create", json!({"id": mine})).unwrap();
         assert!(app.session.path.is_none());
         assert!(app.session.active_comp().is_some_and(|c| c.layers.iter().any(|l| l.name == "BRAND NAME")));
-        app.session.execute("templates.delete", json!({"id": mine})).unwrap();
+        // The card's delete button asks first: the file can't be brought back.
+        app.ui.start_screen = true;
         frame(&mut app, &ctx);
+        click(&mut app, &ctx, &format!("home.templates.{mine}.delete"));
+        assert_eq!(app.dialog, Some(Dialog::DeleteTemplate));
+        assert!(app.session.path.is_none() && entries(&mut app).iter().any(|t| t.id == mine), "nothing deleted or opened yet");
+        click(&mut app, &ctx, "dialog.deleteTemplate.cancel");
+        assert!(app.dialog.is_none() && entries(&mut app).iter().any(|t| t.id == mine), "Cancel keeps the template");
+        // Enter is not Delete; Cmd+D is.
+        click(&mut app, &ctx, &format!("home.templates.{mine}.delete"));
+        crate::panels::dialogs::press_key(&mut app, &ctx, egui::Key::Enter, egui::Modifiers::NONE);
+        assert_eq!(app.dialog, Some(Dialog::DeleteTemplate));
+        crate::panels::dialogs::press_key(&mut app, &ctx, egui::Key::D, egui::Modifiers::COMMAND);
+        assert!(app.dialog.is_none() && app.dialog_state.delete_template.is_none());
         frame(&mut app, &ctx);
         assert!(app.auto.find(&format!("home.templates.{mine}")).is_none());
+        assert!(!entries(&mut app).iter().any(|t| t.id == mine));
     }
 
     #[test]