diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 8cb91b9b3..6083c4575 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -3076,6 +3076,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] diff --git a/crates/ui-egui/src/import.rs b/crates/ui-egui/src/import.rs index a18357eca..89fe2efe7 100644 --- a/crates/ui-egui/src/import.rs +++ b/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; } @@ -933,7 +940,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); } } }); @@ -1282,7 +1289,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"), @@ -1291,7 +1299,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); } @@ -1313,6 +1321,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()); @@ -1580,6 +1597,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(); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 77ae26e4c..22833854c 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -107,7 +107,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 |