From e343b7aa6d53a14cbe7db4e331c9e4226b4571b8 Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Mon, 12 Oct 2026 17:41:55 +0200 Subject: [PATCH] fix: right-click Delete Photo goes through the menu bar's run_item (#1239) Simplify review: the photo menu's Delete Photo now calls menubar::run_item, the one place a user's menu click is run (it already asks before photo.delete), instead of a second copy of the check. The test uses the file's right_click / click / exec helpers and drops a redundant settle; shorter doc comment on the setting. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/grid.rs | 5 ++--- crates/ui-egui/src/state.rs | 5 ++--- crates/ui-egui/src/tests_appearance.rs | 1 - crates/ui-egui/src/tests_panels.rs | 24 +++++++----------------- 4 files changed, 11 insertions(+), 24 deletions(-) diff --git a/crates/ui-egui/src/panels/grid.rs b/crates/ui-egui/src/panels/grid.rs index e008e043a..e40c5dc7a 100644 --- a/crates/ui-egui/src/panels/grid.rs +++ b/crates/ui-egui/src/panels/grid.rs @@ -1140,9 +1140,8 @@ pub fn context_menu(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { } else { let delete = ui.button(crate::i18n::tr("Delete Photo")); register(ui.ctx(), "button:contextDeletePhoto", delete.rect); - // asks first like the key and the menu bar (Settings ▸ General) - if delete.clicked() && !crate::menus::confirm_delete(app) { - let _ = app.run("photo.delete", json!({})); + if delete.clicked() { + let _ = crate::menubar::run_item(app, "photo.delete", json!({})); } } } diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index b1d51b7cb..61573f879 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -234,9 +234,8 @@ pub struct AppSettings { /// [`RECENT_LIBRARIES`]). pub recent_libraries: Vec, pub startup_view: StartupView, - /// Ask before moving photos to Recently Deleted (keyboard, menu bar and a photo's menu); on by - /// default. Saved as `askBeforeDelete`: earlier versions saved `confirmDelete: false` for - /// everyone who never opened Settings, so that key is no longer read (#1239). + /// Ask before moving photos to Recently Deleted. Saved under a new key: earlier versions saved + /// `confirmDelete: false` by default (#1239). #[serde(rename = "askBeforeDelete")] pub confirm_delete: bool, /// GPU rendering allowed (`app.gpu`). diff --git a/crates/ui-egui/src/tests_appearance.rs b/crates/ui-egui/src/tests_appearance.rs index 78a48194f..3703f0219 100644 --- a/crates/ui-egui/src/tests_appearance.rs +++ b/crates/ui-egui/src/tests_appearance.rs @@ -41,7 +41,6 @@ fn new_users_keep_the_dark_look_and_old_settings_load_unchanged() { /// A choice made from now on survives a restart. #[test] fn delete_asks_first_by_default_also_for_settings_saved_by_earlier_versions() { - assert!(AppSettings::default().confirm_delete); let old: UiState = serde_json::from_str(r#"{"settings": {"confirmDelete": false}}"#).unwrap(); assert!(old.settings.confirm_delete, "the old default does not turn the question off"); let mut ui = UiState::default(); diff --git a/crates/ui-egui/src/tests_panels.rs b/crates/ui-egui/src/tests_panels.rs index dd6e509e6..0fa5a2d45 100644 --- a/crates/ui-egui/src/tests_panels.rs +++ b/crates/ui-egui/src/tests_panels.rs @@ -2150,9 +2150,7 @@ fn deleting_a_photo_asks_first_from_the_key_the_menu_and_the_right_click_menu() let mut h = demo([1400.0, 900.0], json!({"view": "photoGrid"})); assert!(h.app.ui.settings.confirm_delete, "on by default"); let photo = h.app.session.visible_cloned()[0]; - let r = h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [photo.0]}}), T); - assert_eq!(r["ok"], true, "{r}"); - h.settle(SETTLE); + exec(&mut h, "library.select", json!({"ids": [photo.0]})); let deleted = |h: &Headless| h.app.session.catalog.photo(photo).unwrap().deleted; // ⌫ and Photo ▸ Delete Photo ask; Cancel keeps the photo @@ -2167,17 +2165,9 @@ fn deleting_a_photo_asks_first_from_the_key_the_menu_and_the_right_click_menu() assert!(!deleted(&h)); // the right-click menu's Delete Photo asks too; OK deletes - let right_click_delete = |h: &mut Headless| { - let thumb = widget(h, &format!("thumb:{}", photo.0)); - let r = h.request("ui.click", json!({"x": thumb.center().x, "y": thumb.center().y, "button": "right"}), T); - assert_eq!(r["ok"], true, "{r}"); - h.step(); - h.step(); - let r = h.request("ui.clickWidget", json!({"id": "button:contextDeletePhoto"}), T); - assert_eq!(r["ok"], true, "{r}"); - h.step(); - }; - right_click_delete(&mut h); + let thumb = format!("thumb:{}", photo.0); + right_click(&mut h, &thumb); + click(&mut h, "button:contextDeletePhoto"); assert_eq!(h.app.ui.dialog, Some(Dialog::ConfirmDelete { count: 1 }), "the right-click menu asks"); assert!(!deleted(&h), "nothing deleted before the answer"); let r = h.request("ui.dialog.confirm", json!({}), T); @@ -2185,11 +2175,11 @@ fn deleting_a_photo_asks_first_from_the_key_the_menu_and_the_right_click_menu() assert!(deleted(&h)); // turned off, the right-click menu deletes at once - h.request("engine.execute", json!({"command": "edit.undo"}), T); + exec(&mut h, "edit.undo", json!({})); assert!(!deleted(&h)); h.app.ui.settings.confirm_delete = false; - h.settle(SETTLE); - right_click_delete(&mut h); + right_click(&mut h, &thumb); + click(&mut h, "button:contextDeletePhoto"); assert_eq!(h.app.ui.dialog, None); assert!(deleted(&h)); }