Merge PR #1242

# Conflicts:
#	crates/ui-egui/src/tests_panels.rs
#	docs/parity-checklist.md

crates/ui-egui/src/headless.rs

@@ -3796,7 +3796,10 @@ mod tests {
             assert_eq!(r["ok"], true, "{tab}: {r}");
             assert_eq!(h.app.ui.dialog, Some(crate::state::Dialog::Settings { tab: tab.into() }));
         }
-        // General: confirm before delete, startup view
+        // General: confirm before delete (on by default), startup view
+        assert!(h.app.ui.settings.confirm_delete);
+        h.request("ui.clickWidget", json!({"id": "check:settings.confirmDelete"}), t);
+        assert!(!h.app.ui.settings.confirm_delete);
         h.request("ui.clickWidget", json!({"id": "check:settings.confirmDelete"}), t);
         assert!(h.app.ui.settings.confirm_delete);
         h.request("ui.clickWidget", json!({"id": "button:settingsStartup-2"}), t);

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

@@ -1185,8 +1185,12 @@ pub fn context_menu(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) {
         if ui.button(crate::i18n::tr("Delete Permanently")).clicked() {
             let _ = app.run("photo.deletePermanently", json!({}));
         }
-    } else if ui.button(crate::i18n::tr("Delete Photo")).clicked() {
-        let _ = app.run("photo.delete", json!({}));
+    } else {
+        let delete = ui.button(crate::i18n::tr("Delete Photo"));
+        register(ui.ctx(), "button:contextDeletePhoto", delete.rect);
+        if delete.clicked() {
+            let _ = crate::menubar::run_item(app, "photo.delete", json!({}));
+        }
     }
 }
 

crates/ui-egui/src/state.rs

@@ -234,7 +234,9 @@ pub struct AppSettings {
     /// [`RECENT_LIBRARIES`]).
     pub recent_libraries: Vec<String>,
     pub startup_view: StartupView,
-    /// Ask before moving photos to Recently Deleted (keyboard and menu).
+    /// 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`).
     pub gpu: bool,
@@ -281,7 +283,7 @@ impl Default for AppSettings {
             library_path: String::new(),
             recent_libraries: Vec::new(),
             startup_view: StartupView::Last,
-            confirm_delete: false,
+            confirm_delete: true,
             gpu: true,
             preview_limit: 0,
             external_editor: String::new(),

crates/ui-egui/src/tests_appearance.rs

@@ -27,15 +27,28 @@ fn new_users_keep_the_dark_look_and_old_settings_load_unchanged() {
     assert_eq!((s.appearance_mode, s.dark_theme, s.light_theme), (AppearanceMode::Dark, DarkTheme::Charcoal, LightTheme::Silver));
     assert_eq!(s.theme(Some(egui::Theme::Light)), ThemeKind::Charcoal, "Auto is opt-in");
     // a ui.json saved before appearance modes existed
-    let old: UiState = serde_json::from_str(r#"{"settings": {"gridBadges": "always", "confirmDelete": true}}"#).unwrap();
+    let old: UiState = serde_json::from_str(r#"{"settings": {"gridBadges": "always", "startupView": "detail"}}"#).unwrap();
     assert_eq!(old.settings.appearance_mode, AppearanceMode::Dark);
     assert_eq!(old.settings.dark_theme, DarkTheme::Charcoal);
     assert_eq!(old.settings.light_theme, LightTheme::Silver);
-    assert!(old.settings.confirm_delete);
+    assert_eq!(old.settings.startup_view, crate::state::StartupView::Detail);
     // and the Charcoal tokens are the ones LightCraft always had
     assert_eq!(crate::theme::Tokens::for_kind(ThemeKind::Charcoal).chrome, crate::theme::Tokens::default().chrome);
 }
 
+/// Delete asks first by default (#1239). Earlier versions saved `"confirmDelete": false` for
+/// everyone who never opened Settings, so that key is not read any more: those installs ask too.
+/// A choice made from now on survives a restart.
+#[test]
+fn delete_asks_first_by_default_also_for_settings_saved_by_earlier_versions() {
+    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();
+    ui.settings.confirm_delete = false;
+    let back: UiState = serde_json::from_str(&serde_json::to_string(&ui).unwrap()).unwrap();
+    assert!(!back.settings.confirm_delete, "a choice made now is kept");
+}
+
 #[test]
 fn choices_validate_and_survive_a_restart() {
     let mut a = app();

crates/ui-egui/src/tests_panels.rs

@@ -2269,4 +2269,44 @@ fn an_album_menu_imports_into_that_album() {
     assert_eq!(r["result"]["scanning"], true, "{r}");
     assert_eq!(review_album(&mut h), None);
     let _ = std::fs::remove_dir_all(&dir);
+/// Delete asks before photos go to Recently Deleted, by default and from every way in: the key, the
+/// Photo menu and a photo's right-click menu (#1239). With the setting off, it deletes at once.
+#[test]
+fn deleting_a_photo_asks_first_from_the_key_the_menu_and_the_right_click_menu() {
+    use crate::state::Dialog;
+    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];
+    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
+    h.request("ui.key", json!({"key": "delete"}), T);
+    assert_eq!(h.app.ui.dialog, Some(Dialog::ConfirmDelete { count: 1 }), "the key asks");
+    h.request("ui.dialog.cancel", json!({}), T);
+    assert!(!deleted(&h));
+    // (what a click on the menu bar's item runs; `ui.menu.invoke` is the agents' engine.execute)
+    crate::menubar::run_item(&mut h.app, "photo.delete", json!({})).unwrap();
+    assert_eq!(h.app.ui.dialog, Some(Dialog::ConfirmDelete { count: 1 }), "the menu asks");
+    h.request("ui.dialog.cancel", json!({}), T);
+    assert!(!deleted(&h));
+
+    // the right-click menu's Delete Photo asks too; OK deletes
+    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);
+    assert_eq!(r["ok"], true, "{r}");
+    assert!(deleted(&h));
+
+    // turned off, the right-click menu deletes at once
+    exec(&mut h, "edit.undo", json!({}));
+    assert!(!deleted(&h));
+    h.app.ui.settings.confirm_delete = false;
+    right_click(&mut h, &thumb);
+    click(&mut h, "button:contextDeletePhoto");
+    assert_eq!(h.app.ui.dialog, None);
+    assert!(deleted(&h));
 }

docs/parity-checklist.md

@@ -118,6 +118,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts).
 | LR-LIB-STACK | Stacks | P1 | ✅ | `cmd:stack.group`, `cmd:stack.ungroup`, `cmd:stack.toggle`, `cmd:stack.setTop`, `cmd:stack.remove`, `cmd:stack.auto`, `crates/catalog/src/stacks.rs` | grid/filmstrip count badges, expand/collapse, auto-stack by capture time; no visual-similarity auto-stack |
 | LR-LIB-VERSIONS | Versions | P1 | ✅ | `cmd:version.create` | see section M |
 | LR-LIB-DELETE | Delete / Recently Deleted | P0 | ✅ | `cmd:photo.delete`, `cmd:photo.restore`, `cmd:photo.deletePermanently` | Delete and Delete Permanently retain surviving selected photos; if none remain, select the next visible survivor (previous at the end) in the current source/filter/sort; permanent deletion removes catalog records, not files on disk; Restore and Delete Permanently in the Photo menu and the photo context menus for photos in Recently Deleted (before, the UI had no way to restore); importing a file that is in Recently Deleted again restores the photo with its edits, rating and albums (issues #298, #706: before, the import skipped it as "already in the library" while the grid showed it deleted) — the review dialog checks it, badges it "Will be restored", counts it as "in Recently Deleted" rather than "already in the library", and its choice Restore them / Import as new / Leave them starts on Restore; with Leave them such a file starts unchecked and Check All and the file-type toggles leave it out, but a click still checks it and it is restored (issue #1285: before, the click did nothing and the "In Recently Deleted" badge was drawn over the checkbox, which now sits above it); dropped files restore too ("Restored 1 photo from Recently Deleted"); a deleted photo matched by content whose file is gone is relinked to the imported file; Photo ▸ Empty Recently Deleted (`cmd:library.emptyRecentlyDeleted`, one undo step, shown while the trash is the source); import takes `onDeleted` restore (default) / skip / fresh for files in the trash (`cmd:library.import`; the report lists `restored`, and with skip flags the duplicate `existingDeleted`); browsing a folder in Local (`cmd:library.browse`), Auto Import and a Lightroom catalog import never restore a deleted photo; no confirmation dialog, no auto-purge after N days (needs a deleted-at date in the catalog) |
+| LR-LIB-DELETE | Delete / Recently Deleted | P0 | ✅ | `cmd:photo.delete`, `cmd:photo.restore`, `cmd:photo.deletePermanently` | Delete and Delete Permanently retain surviving selected photos; if none remain, select the next visible survivor (previous at the end) in the current source/filter/sort; permanent deletion removes catalog records, not files on disk; Restore and Delete Permanently in the Photo menu and the photo context menus for photos in Recently Deleted (before, the UI had no way to restore); importing a file that is in Recently Deleted again restores the photo with its edits, rating and albums (issues #298, #706: before, the import skipped it as "already in the library" while the grid showed it deleted) — the review dialog checks it, badges it "Will be restored", counts it as "in Recently Deleted" rather than "already in the library", and its choice Restore them / Import as new / Leave them starts on Restore; dropped files restore too ("Restored 1 photo from Recently Deleted"); a deleted photo matched by content whose file is gone is relinked to the imported file; Photo ▸ Empty Recently Deleted (`cmd:library.emptyRecentlyDeleted`, one undo step, shown while the trash is the source); import takes `onDeleted` restore (default) / skip / fresh for files in the trash (`cmd:library.import`; the report lists `restored`, and with skip flags the duplicate `existingDeleted`); browsing a folder in Local (`cmd:library.browse`), Auto Import and a Lightroom catalog import never restore a deleted photo; Delete asks first ("Move this photo to Recently Deleted?") from the key, the Photo menu and the photo context menu, on by default and switched off in Settings ▸ General (#1239: before, it was off by default and the context menu never asked); no auto-purge after N days (needs a deleted-at date in the catalog) |
 | LR-LIB-REMOVE-ALBUM | Remove from album | P0 | ✅ | `cmd:album.removePhotos` | |
 | LR-LIB-DUPLICATE | Duplicate a photo | P2 | ✅ | `cmd:photo.duplicate` | Photo ▸ Duplicate: a real `-copy` file with the same settings, metadata and albums (virtual copies share the file) |
 | LR-LIB-RENAME | Batch rename | P1 | ✅ | `cmd:photo.rename`, `cmd:photo.renamePreview`, `cmd:photo.renameTokens`, `cmd:dialog.rename`, `crates/engine/src/rename.rs` (`TOKENS`) | template tokens {name} {num} {seq:N} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext} (shared with export naming and import rename); a Tags picker beside every template field (Rename Photos, import Rename, export File name) lists each tag with meaning + live example, the date directives and the rules, and inserts the clicked tag at the text cursor; unknown tags are flagged; preview; renames files on disk with their XMP sidecars (a stem sidecar another file still shares, e.g. raw + JPEG, is copied rather than taken), never overwriting (-1, -2… suffixes; a change of letter case only is checked by file identity, so on case-sensitive volumes `img_1.jpg` next to `IMG_1.JPG` is a collision), rolls back on failure (a file that can't be moved back is reported old → new and the library follows it as an undoable partial rename; the same for undo/redo); undo/redo move the files; virtual copies follow |
@@ -576,7 +577,7 @@ visible menu bar yet: the menu model is only exposed through the control channel
 | MENU-PHOTO-RENAME | Rename N photos… | P1 | ✅ | `cmd:dialog.rename` | live “Rename N Photos…” label |
 | MENU-PHOTO-CAPTURETIME | Edit capture time… | P1 | ✅ | `cmd:dialog.captureTime` | |
 | MENU-PHOTO-COVER | Set as album cover | P2 | ✅ | `cmd:album.setCover` | |
-| MENU-PHOTO-DELETE | Delete N photos… | P0 | ✅ | `cmd:photo.delete` | no confirmation; static label |
+| MENU-PHOTO-DELETE | Delete N photos… | P0 | ✅ | `cmd:photo.delete` | asks first (Settings ▸ General, on by default, #1239); static label |
 | MENU-PHOTO-MOVETOCLOUD | Move/copy to cloud | OOS | 🚫 | | |
 | MENU-WINDOW-MINIMIZE | Minimize / zoom | P1 | ✅ | `apps/lightcraft/src/native_menu.rs` | the system's Minimize (⌘M) and Zoom items at the top of Window |
 | MENU-WINDOW-PANELS | Panel switches | P0 | ✅ | `cmd:panel.edit`, `cmd:panel.crop`, `cmd:panel.remove`, `cmd:panel.masking`, `cmd:panel.presets`, `cmd:panel.versions` | |