Merge PR #1301

crates/ui-egui/src/headless.rs

@@ -3204,6 +3204,70 @@ mod tests {
         }
     }
 
+    /// With "Leave them", a file in Recently Deleted is left unchecked but a click on its checkbox
+    /// (or on its "In Recently Deleted" badge) still checks it, and importing it restores the
+    /// photo (issue #1285).
+    #[test]
+    fn import_review_checks_a_recently_deleted_file_by_hand() {
+        let dir = std::env::temp_dir().join(format!("lc-ui-import-trash-click-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&dir);
+        std::fs::create_dir_all(&dir).unwrap();
+        for i in 0..2u8 {
+            let data: Vec<[u8; 4]> = (0..40 * 30).map(|k| [(k % 40 * 6) as u8, i * 90, 7, 255]).collect();
+            let img = lightcraft_raster::Rgba8 { width: 40, height: 30, data };
+            let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &lightcraft_codecs::EncodeMeta::default()).unwrap();
+            std::fs::write(dir.join(format!("img{i}.png")), png).unwrap();
+        }
+        let services = crate::Services { png: None, ..Default::default() };
+        let mut app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), services);
+        app.ui.view = crate::state::ViewMode::PhotoGrid;
+        let mut h = Headless::new(app, [1300.0, 900.0], 1.0);
+        let t = Duration::from_secs(10);
+        let r = h.request("engine.execute", json!({"command": "library.import", "params": {"paths": [dir.join("img0.png").to_string_lossy()]}}), t);
+        let id = r["result"]["imported"][0].as_u64().unwrap();
+        h.request("engine.execute", json!({"command": "photo.rate", "params": {"ids": [id], "rating": 3}}), t);
+        h.request("engine.execute", json!({"command": "photo.delete", "params": {"ids": [id]}}), t);
+        h.request("engine.execute", json!({"command": "file.addPhotos", "params": {"paths": [dir.to_string_lossy()]}}), t);
+        h.step_until(SETTLE, |h| matches!(h.app.ui.dialog, Some(crate::state::Dialog::Import { .. })));
+        let Some(crate::state::Dialog::Import { opts }) = &mut h.app.ui.dialog else { panic!("no import review") };
+        let trashed = (0..opts.candidates.len()).find(|i| opts.is_trashed(*i)).expect("a file in Recently Deleted");
+        opts.set_on_deleted("");
+        h.settle(SETTLE);
+        let checked = |h: &Headless| matches!(&h.app.ui.dialog, Some(crate::state::Dialog::Import { opts }) if opts.checked[trashed]);
+        assert!(!checked(&h), "Leave them: unchecked");
+        let cell = |h: &mut Headless| {
+            let r = h.request("ui.widgets", json!({"filter": format!("import:{trashed}")}), t);
+            let w = r["result"].as_array().and_then(|a| a.iter().find(|w| w["id"] == format!("import:{trashed}"))).cloned().expect("cell on screen");
+            [0usize, 1, 2, 3].map(|i| w["rect"][i].as_f64().unwrap())
+        };
+        // the checkbox: 16 pt, 6 pt in from the cell's top-left corner
+        let [x, y, ..] = cell(&mut h);
+        h.request("ui.click", json!({"x": x + 14.0, "y": y + 14.0}), t);
+        h.settle(SETTLE);
+        assert!(checked(&h), "a click on the checkbox checks the file");
+        // the badges ("Will be restored" at the top right, "In Recently Deleted" under the
+        // checkbox) don't swallow the click either
+        let [x, y, w, _] = cell(&mut h);
+        h.request("ui.click", json!({"x": x + w - 14.0, "y": y + 12.0}), t);
+        h.settle(SETTLE);
+        assert!(!checked(&h), "a click on the badge unchecks it again");
+        h.request("ui.click", json!({"x": x + w - 14.0, "y": y + 34.0}), t);
+        h.settle(SETTLE);
+        assert!(checked(&h));
+        let r = h.request("ui.dialog.confirm", json!({}), t);
+        assert_eq!(r["ok"], true, "{r}");
+        let t0 = std::time::Instant::now();
+        while h.app.import.is_some() && t0.elapsed() < Duration::from_secs(30) {
+            h.step();
+        }
+        assert!(h.app.import.is_none(), "finished");
+        let photos = &h.app.session.catalog;
+        assert!(photos.photo(lightcraft_catalog::PhotoId(id)).is_some_and(|p| !p.deleted && p.rating == 3), "restored with its edits");
+        assert_eq!(photos.photos().filter(|p| !p.deleted).count(), 2);
+        assert_eq!(photos.len(), 2, "restored, not imported a second time");
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
     /// The import review opens bigger than the old fixed 6 × 2.5 grid and can be resized by its
     /// corner: the photo grid takes the new height, and the window then keeps its size (issue #337).
     #[test]

crates/ui-egui/src/import.rs

@@ -8,7 +8,7 @@
 //! catalog between frames, with a row and ✕ in the activity stack (issue #345); the whole import is
 //! one undo step.
 
-use egui::{Align2, Color32, Rect, Sense, Stroke, StrokeKind, pos2, vec2};
+use egui::{Align2, Color32, Rect, Sense, Stroke, StrokeKind, Vec2, pos2, vec2};
 use lightcraft_engine::import::{
     ImportCandidate, ImportUprightMode, ImportWarning, ImportWarningCode, ScanInput, ScanOutput, ScanProgress, scan_with,
 };
@@ -123,14 +123,21 @@ impl ImportDialog {
             if !self.is_trashed(i) {
                 continue;
             }
-            let on = self.importable(i);
+            let on = self.checked_by_default(i);
             if let Some(c) = self.checked.get_mut(i) {
                 *c = on;
             }
         }
     }
+    /// Candidate `i` can be checked: a readable file the library doesn't have, or one whose photo
+    /// is in Recently Deleted — even with "Leave them", a click picks it (issue #1285).
     pub fn importable(&self, i: usize) -> bool {
-        self.candidates.get(i).is_some_and(|c| c.error.is_none() && (c.duplicate.is_none() || self.is_trashed(i) && !self.on_deleted.is_empty()))
+        self.candidates.get(i).is_some_and(|c| c.error.is_none() && (c.duplicate.is_none() || self.is_trashed(i)))
+    }
+    /// Candidate `i` is checked by Check All and the file type toggles: with "Leave them", a file in
+    /// Recently Deleted is only checked by hand.
+    pub fn checked_by_default(&self, i: usize) -> bool {
+        self.importable(i) && !(self.is_trashed(i) && self.on_deleted.is_empty())
     }
     /// The file types among the candidates (`ARW`, `JPG`…, upper case, sorted) and how many of each.
     pub fn file_types(&self) -> Vec<(String, usize)> {
@@ -149,7 +156,7 @@ impl ImportDialog {
     pub fn check_type(&mut self, ty: &str, on: bool) {
         for i in 0..self.candidates.len() {
             let same = self.candidates.get(i).is_some_and(|c| file_type(c) == ty);
-            let importable = self.importable(i);
+            let importable = self.checked_by_default(i);
             if same && let Some(c) = self.checked.get_mut(i) {
                 *c = on && importable;
             }
@@ -937,7 +944,7 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog) {
             }
             if crate::widgets::text_button(ui, "importAll", crate::i18n::tr("Check All"), false).clicked() {
                 for i in 0..n {
-                    d.checked[i] = d.importable(i);
+                    d.checked[i] = d.checked_by_default(i);
                 }
             }
         });
@@ -1286,7 +1293,8 @@ fn candidate_cell(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDial
     }
     let in_trash = c.existing.is_some_and(|id| app.session.catalog.photo(lightcraft_catalog::PhotoId(id)).is_some_and(|p| p.deleted));
     let badge = match (&c.duplicate, &c.error) {
-        (Some(_), _) if in_trash && ok => Some(if d.on_deleted == "restore" { "Will be restored" } else { "Will be replaced" }),
+        // checked with "Leave them", the file goes without `onDeleted`: the engine restores it
+        (Some(_), _) if in_trash && on => Some(if d.on_deleted == "fresh" { "Will be replaced" } else { "Will be restored" }),
         (Some(_), _) if in_trash => Some("In Recently Deleted"),
         (Some(r), _) if r == "path" => Some("In library"),
         (Some(_), _) => Some("Duplicate"),
@@ -1295,7 +1303,7 @@ fn candidate_cell(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDial
     };
     if let Some(b) = badge {
         let g = p.layout_no_wrap(crate::i18n::tr(b).to_string(), t.semibold(10.0), Color32::WHITE);
-        let br = Rect::from_min_size(pos2(img.right() - g.size().x - 12.0, img.top() + 6.0), g.size() + vec2(8.0, 4.0));
+        let br = badge_rect(img, cb, g.size());
         p.rect_filled(br, 3.0, Color32::from_rgba_unmultiplied(170, 60, 50, 220));
         p.galley(br.min + vec2(4.0, 2.0), g, Color32::WHITE);
     }
@@ -1317,6 +1325,15 @@ fn candidate_cell(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDial
     }
 }
 
+/// Where a cell's badge goes: top right, or under the checkbox when the text is too wide to sit
+/// beside it — drawn over the checkbox it hid it (issue #1285).
+fn badge_rect(img: Rect, cb: Rect, text: Vec2) -> Rect {
+    let size = text + vec2(8.0, 4.0);
+    let left = img.right() - size.x - 4.0;
+    let top = if left < cb.right() + 4.0 { cb.bottom() + 4.0 } else { img.top() + 6.0 };
+    Rect::from_min_size(pos2(left, top), size)
+}
+
 /// Reserve the actual translated tag button width beside a template field.
 pub(crate) fn tag_field_width(ui: &egui::Ui) -> f32 {
     let t = Tokens::get(ui.ctx());
@@ -1584,6 +1601,44 @@ mod tests {
         assert_eq!(d.checked, [true, true, false, false, true, true]);
     }
 
+    /// With "Leave them", a file in Recently Deleted stays out of Check All and the type toggles,
+    /// but a click still checks it: it is imported (restored) on its own (issue #1285).
+    #[test]
+    fn leave_them_keeps_a_trashed_file_out_until_it_is_clicked() {
+        let mut d = dialog();
+        d.set_trashed(vec![false, false, false, true, false, false]);
+        assert!(d.checked[3], "restored by default");
+        d.set_on_deleted("");
+        assert!(!d.checked[3]);
+        d.check_type("JPG", true);
+        assert!(!d.checked[3], "the type toggle leaves it out");
+        d.click(3, false);
+        assert!(d.checked[3], "a click checks it");
+        assert!(d.selected_paths().contains(&"img3.jpg".to_string()));
+        d.click(3, false);
+        assert!(!d.checked[3]);
+        // a plain duplicate (not in Recently Deleted) still can't be checked
+        d.set_trashed(vec![false; 6]);
+        d.click(3, false);
+        assert!(!d.checked[3]);
+    }
+
+    /// A badge too wide to fit beside the checkbox goes under it instead of covering it (issue #1285).
+    #[test]
+    fn a_badge_never_covers_the_checkbox() {
+        let img = Rect::from_min_size(pos2(100.0, 50.0), vec2(116.0, 116.0));
+        let cb = Rect::from_min_size(img.min + vec2(6.0, 6.0), vec2(16.0, 16.0));
+        // "Will be restored" fits beside it, top right
+        let short = badge_rect(img, cb, vec2(76.0, 12.0));
+        assert_eq!(short.right(), img.right() - 4.0);
+        assert_eq!(short.top(), img.top() + 6.0);
+        // "In Recently Deleted" doesn't
+        let long = badge_rect(img, cb, vec2(96.0, 12.0));
+        assert!(!long.intersects(cb), "{long:?} covers {cb:?}");
+        assert_eq!(long.right(), img.right() - 4.0);
+        assert!(img.contains_rect(long));
+    }
+
     #[test]
     fn shift_click_with_a_stale_anchor_toggles_one() {
         let mut d = dialog();

docs/parity-checklist.md

@@ -114,7 +114,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts).
 | LR-LIB-PEOPLE | People / faces | P2 | 🟡 | `cmd:faces.detect`, `cmd:view.people`, `cmd:view.faceBoxes`, `cmd:photo.removeRegion`, `cmd:photo.setRegion`, `cmd:library.filter`, `crates/ui-egui/src/panels/people.rs`, `crates/catalog/src/query.rs` (`people`), `crates/engine/src/media.rs` (`face_job`), `crates/meta/src/xmp.rs` | MVP: named face regions read from XMP (MWG-RS, read-only, see `docs/xmp-interop.md`; clipped to the photo; Lightroom's `Rotation` of ±π/2 and π handled, mirrored orientations taken as written; boxes follow Rotate Left/Right and flips; a sidecar stating `mwg-rs:Regions`, even empty, replaces the photo's on re-read, one without leaves them) become cards in the People view (View ▸ People, or the People button in the toolbar): a close-up of the person's largest face, name and photo count; a click filters the grid (`person` in `library.filter`, `person:` search token, filter chip). Face boxes in the loupe can be switched off (View ▸ Face Boxes, toolbar) resized with eight drag handles and removed with the × shown on hover (catalog-only and undoable, one undo step per drag; the sidecar is never rewritten for either); no moving a box by dragging it yet. Photo ▸ Detect Faces (`cmd:faces.detect`) finds faces with the YuNet detector (pure Rust, about 200 ms a photo, matches OpenCV's own YuNet on a 45-photo test set; the 232 KB model is a one-time download offered in the same step, never part of LightCraft) and adds them as unnamed boxes; names are suggested from the faces already named (recognition models are opt-in downloads, run on the CPU in a background thread; a face is named only when you confirm it); a person's page shows their cropped faces and the unnamed faces that look like them (LRC-LIB-PEOPLE); with recognition on, a background scan finds faces in photos that have none and embeds every face (one to three worker threads, raws through their embedded preview); an Unnamed faces section orders faces by likeness and names a selected group at once; no separated clusters, no manual tagging, no writing regions back |
 | 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; 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; 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-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 |