diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 8a548502..1cf3211a 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -1358,6 +1358,71 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// The import review offers what to do with files that are in Recently Deleted (issue #298): + /// they are unchecked until Restore or Import as new is chosen; then confirming restores the + /// photo (with its edits) or imports the file afresh. + #[test] + fn import_review_offers_recently_deleted_files() { + for (choice, tag) in [("restore", "r"), ("fresh", "f")] { + let dir = std::env::temp_dir().join(format!("lc-ui-import-trash-{tag}-{}", 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") }; + assert_eq!(opts.selected_paths().len(), 1, "only the new file: the trashed one waits for a choice"); + opts.set_on_deleted(choice); + assert_eq!(opts.selected_paths().len(), 2, "the trashed file is checked once a choice is made"); + // a per-cell choice survives switching between the two options, and "Leave them" unchecks + let trashed = (0..opts.candidates.len()).find(|i| opts.is_trashed(*i)).unwrap(); + opts.checked[trashed] = false; + opts.set_on_deleted(if choice == "restore" { "fresh" } else { "restore" }); + assert_eq!(opts.selected_paths().len(), 1, "still unchecked"); + opts.set_on_deleted(""); + assert_eq!(opts.selected_paths().len(), 1); + opts.set_on_deleted(choice); + assert_eq!(opts.selected_paths().len(), 2); + let r = h.request("ui.dialog.confirm", json!({}), t); + assert_eq!(r["ok"], true, "{r}"); + // frames until the import has finished, so its (timed) toast is read before it expires + 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 toast = h.app.ui.toast.clone().map(|t| t.0).unwrap_or_default(); + let photos = &h.app.session.catalog; + assert_eq!(photos.photos().filter(|p| !p.deleted).count(), 2, "{choice}"); + let old = photos.photo(lightcraft_catalog::PhotoId(id)); + if choice == "restore" { + assert!(toast.contains("1 restored"), "{toast}"); + assert!(old.is_some_and(|p| !p.deleted && p.rating == 3), "restored with its edits"); + assert_eq!(photos.len(), 2); + } else { + assert!(old.is_none(), "the trashed record is gone"); + assert_eq!(photos.len(), 2); + } + let _ = std::fs::remove_dir_all(&dir); + } + } + /// Adding a file again that is in Recently Deleted shows it there (side panel opened, photo /// selected) instead of only saying "duplicate skipped"; its menus offer Restore, and once /// restored a re-add selects it in All Photos. diff --git a/crates/ui-egui/src/import.rs b/crates/ui-egui/src/import.rs index 3ac7c9eb..e88e518a 100644 --- a/crates/ui-egui/src/import.rs +++ b/crates/ui-egui/src/import.rs @@ -53,6 +53,10 @@ pub struct ImportDialog { pub metadata_preset: String, /// Copy: raws are copied as DNG. pub dng: bool, + /// Per candidate: the library already has it in Recently Deleted. + pub trashed: Vec, + /// What to do with those: "" (leave them), `restore` or `fresh` (the `onDeleted` param). + pub on_deleted: String, } impl ImportDialog { @@ -61,14 +65,33 @@ impl ImportDialog { ImportDialog { candidates, checked, ..Default::default() } } pub fn importable(&self, i: usize) -> bool { - self.candidates.get(i).is_some_and(|c| c.duplicate.is_none() && c.error.is_none()) + 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())) + } + /// Candidate `i` is a file whose photo is in Recently Deleted. + pub fn is_trashed(&self, i: usize) -> bool { + self.trashed.get(i).copied().unwrap_or(false) + } + /// Choose what happens to the files in Recently Deleted (`""`, `restore` or `fresh`); they are + /// checked once there is a choice and unchecked again without one. + pub fn set_on_deleted(&mut self, choice: &str) { + // switching between restore and fresh keeps the user's per-cell choices + let changed = self.on_deleted.is_empty() != choice.is_empty(); + self.on_deleted = choice.to_string(); + for i in (0..self.candidates.len()).filter(|_| changed) { + if self.is_trashed(i) + && let Some(c) = self.checked.get_mut(i) + { + *c = !choice.is_empty(); + } + } } pub fn selected_paths(&self) -> Vec { self.candidates .iter() .zip(&self.checked) - .filter(|(c, on)| **on && c.duplicate.is_none() && c.error.is_none()) - .map(|(c, _)| c.path.clone()) + .enumerate() + .filter(|(i, (_, on))| **on && self.importable(*i)) + .map(|(_, (c, _))| c.path.clone()) .collect() } /// The `organize` param of `library.import` (`None` = the default, by day); an unusable @@ -104,6 +127,8 @@ pub struct ImportTask { pub done: usize, params: Value, pub imported: usize, + /// Brought back from Recently Deleted (`onDeleted: restore`). + pub restored: usize, pub duplicates: usize, /// The library photos the skipped duplicates match (shown when nothing new was added). existing: Vec, @@ -400,6 +425,14 @@ pub fn poll_scan(app: &mut LightcraftApp, ctx: &egui::Context) { } app.renderer.forget_imports(); let mut d = ImportDialog::new(out.candidates); + d.trashed = d + .candidates + .iter() + .map(|c| { + c.duplicate.is_some() + && c.existing.is_some_and(|id| app.session.catalog.photo(lightcraft_catalog::PhotoId(id)).is_some_and(|p| p.deleted)) + }) + .collect(); d.copy = task.copy; d.sources = task.sources; app.ui.dialog = Some(crate::state::Dialog::Import { opts: Box::new(d) }); @@ -461,6 +494,9 @@ pub fn start(app: &mut LightcraftApp, d: &ImportDialog) -> Result (true, true) => "move", }; let mut params = json!({"mode": mode, "keywords": d.keywords()}); + if !d.on_deleted.is_empty() { + params["onDeleted"] = json!(d.on_deleted); + } if let Some(a) = album { params["album"] = json!(a); } @@ -564,6 +600,7 @@ fn commit_batch(app: &mut LightcraftApp, task: &mut ImportTask, prepared: lightc } let len = |k: &str| v[k].as_array().map_or(0, Vec::len); task.imported += len("imported"); + task.restored += len("restored"); task.duplicates += len("duplicates"); task.existing.extend(v["duplicates"].as_array().into_iter().flatten().filter_map(|d| d["existing"].as_u64())); task.failed += len("failed"); @@ -573,7 +610,7 @@ fn commit_batch(app: &mut LightcraftApp, task: &mut ImportTask, prepared: lightc log::warn!("import: kept {}: {}", k["path"].as_str().unwrap_or(""), k["reason"].as_str().unwrap_or("")); } if task.first.is_none() { - task.first = v["imported"].as_array().and_then(|a| a.first()).and_then(Value::as_u64); + task.first = ["imported", "restored"].iter().find_map(|k| v[*k].as_array().and_then(|a| a.first()).and_then(Value::as_u64)); } } Err(e) => { @@ -586,7 +623,14 @@ fn commit_batch(app: &mut LightcraftApp, task: &mut ImportTask, prepared: lightc /// The import is done (or cancelled): one undo step, select the first photo, say what happened. fn finish(app: &mut LightcraftApp, ctx: &egui::Context, task: ImportTask) { let steps = app.session.undo.len().saturating_sub(task.undo0); - let label = crate::i18n::tr_format!("Add {} Photo{}", task.imported, if task.imported == 1 { "" } else { "s" }); + let plural = |n: usize| if n == 1 { "" } else { "s" }; + let label = if task.imported == 0 && task.restored > 0 { + format!("Restore {} Photo{}", task.restored, plural(task.restored)) + } else if task.restored > 0 { + format!("Add {} Photo{}, restore {}", task.imported, plural(task.imported), task.restored) + } else { + crate::i18n::tr_format!("Add {} Photo{}", task.imported, if task.imported == 1 { "" } else { "s" }) + }; app.session.merge_undo(steps, &label); if task.auto { if task.imported > 0 { @@ -606,7 +650,7 @@ fn finish(app: &mut LightcraftApp, ctx: &egui::Context, task: ImportTask) { let plural = |n: usize| if n == 1 { "" } else { "s" }; // nothing new, only files the library already has: show where they are (Recently Deleted is // easy to miss, and the side panel that lists it starts collapsed) - if task.imported == 0 && task.failed == 0 && !task.cancelled && show_existing(app, ctx, &task.existing) { + if task.imported == 0 && task.restored == 0 && task.failed == 0 && !task.cancelled && show_existing(app, ctx, &task.existing) { return; } let mut msg = if task.cancelled { @@ -614,6 +658,9 @@ fn finish(app: &mut LightcraftApp, ctx: &egui::Context, task: ImportTask) { } else { crate::i18n::tr_format!("Imported {} photo{}", task.imported, plural(task.imported)) }; + if task.restored > 0 { + msg.push_str(&format!(" · {} restored from Recently Deleted", task.restored)); + } if task.params["mode"] == "move" { msg.push_str(&crate::i18n::tr_format!(" · {} moved", task.moved)); if task.kept > 0 { @@ -895,6 +942,32 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog) { ); } } + if d.trashed.iter().any(|t| *t) { + field(ui, "In Recently Deleted", |ui| { + let cur = match d.on_deleted.as_str() { + "restore" => "Restore them", + "fresh" => "Import as new", + _ => "Leave them", + }; + let mut pick = None; + egui::ComboBox::from_id_salt("import-on-deleted").selected_text(crate::i18n::tr(cur)).show_ui(ui, |ui| { + for (v, label) in [("", "Leave them"), ("restore", "Restore them"), ("fresh", "Import as new")] { + if ui.selectable_label(d.on_deleted == v, crate::i18n::tr(label)).clicked() { + pick = Some(v); + } + } + }); + if let Some(v) = pick { + d.set_on_deleted(v); + } + let t = Tokens::get(ui.ctx()); + ui.label( + egui::RichText::new(crate::i18n::tr("Restore keeps their edits; Import as new deletes the old record first.")) + .color(t.text_dim) + .small(), + ); + }); + } let albums: Vec<(u64, String)> = { let mut v: Vec<(u64, String)> = app.session.catalog.albums().filter(|a| !a.folder && !a.is_smart()).map(|a| (a.id.0, a.name.clone())).collect(); @@ -1000,6 +1073,7 @@ 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" }), (Some(_), _) if in_trash => Some("In Recently Deleted"), (Some(r), _) if r == "path" => Some("In library"), (Some(_), _) => Some("Duplicate"), @@ -1117,7 +1191,7 @@ pub const DEFAULT_FOLDER_TEMPLATE: &str = "{date:%Y}/{date:%Y%m%d}"; /// Where the first selected photo would be copied to (destination, folders, name), for the /// dialog's example line; `None` without a photo or with an unusable folder template. pub fn example_destination(app: &LightcraftApp, d: &ImportDialog) -> Option { - let c = d.candidates.iter().zip(&d.checked).find(|(c, on)| **on && c.duplicate.is_none() && c.error.is_none()).map(|(c, _)| c)?; + let c = d.candidates.iter().zip(&d.checked).enumerate().find(|(i, (_, on))| **on && d.importable(*i)).map(|(_, (c, _))| c)?; let organize = match d.organize_param().ok()? { Some(o) => lightcraft_engine::import::Organize::parse(&o)?, None => Default::default(), diff --git a/docs/parity.md b/docs/parity.md index 628532ef..eeafbd17 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -150,7 +150,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-LIB-PEOPLE | People / faces | P2 | 🟡 | `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. No face detection, no unnamed people / suggestions, 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` | 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); adding a file that is in Recently Deleted again selects it there (side panel opened) and says how to restore it or import it afresh; Photo ▸ Empty Recently Deleted (`cmd:library.emptyRecentlyDeleted`, one undo step, shown while the trash is the source); import takes `onDeleted` skip / restore / fresh for files in the trash (`cmd:library.import`; the report flags `existingDeleted` and lists `restored`; the desktop import dialog still uses skip and points at Restore); 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` | 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); adding a file that is in Recently Deleted again selects it there (side panel opened) and says how to restore it or import it afresh; Photo ▸ Empty Recently Deleted (`cmd:library.emptyRecentlyDeleted`, one undo step, shown while the trash is the source); import takes `onDeleted` skip / restore / fresh for files in the trash (`cmd:library.import`; the report flags `existingDeleted` and lists `restored`; the import review dialog offers Leave them / Restore them / Import as new for files that are in the trash; dropped files and Add Photos without a review still skip and point at Restore); 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 |