From 0d8e3e3913fad64148c5fdcd490c230e97d64a69 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Tue, 13 Oct 2026 09:04:10 +0200 Subject: [PATCH] LR-IMP-ADD-DIALOG: an import ends showing its first photo, in All Photos when the view doesn't hold it, unless another photo was chosen meanwhile (fixes #1446) --- crates/ui-egui/src/headless.rs | 34 ++++++++++++++++++++++++++++ crates/ui-egui/src/import.rs | 19 ++++++++++++++-- crates/ui-egui/src/tests_activity.rs | 18 +++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 3ce1331a2..91002ce20 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -3458,6 +3458,40 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// Importing while an album is shown (issue #1446): the new photos aren't in it, so the view + /// switches to All Photos with the first of them active, instead of an active photo the grid + /// and the filmstrip don't show. + #[test] + fn an_import_from_an_album_view_shows_the_first_new_photo() { + let dir = std::env::temp_dir().join(format!("lc-ui-import-show-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + for i in 0..3u8 { + let img = lightcraft_raster::Rgba8 { width: 8, height: 8, data: vec![[i * 40, 3, 9, 255]; 64] }; + let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &lightcraft_codecs::EncodeMeta::default()).unwrap(); + std::fs::write(dir.join(format!("IMG_{i:02}.png")), png).unwrap(); + } + let mut h = demo([1300.0, 900.0]); + let t = Duration::from_secs(10); + let album = h.app.session.catalog.albums().find(|a| !a.folder && !a.is_smart()).map(|a| a.id.0).expect("a demo album"); + let r = h.request("engine.execute", json!({"command": "library.source", "params": {"kind": "album", "id": album}}), t); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + let before: Vec = h.app.session.catalog.photos().map(|p| p.id).collect(); + let r = h.request("engine.execute", json!({"command": "file.addPhotos", "params": {"paths": [dir.to_string_lossy()]}}), t); + assert_eq!(r["result"]["scanning"], true, "{r}"); + h.step_until(SETTLE, |h| matches!(h.app.ui.dialog, Some(crate::state::Dialog::Import { .. }))); + let r = h.request("ui.dialog.confirm", json!({}), t); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert!(h.app.import.is_none(), "finished"); + let active = h.app.session.selection.active.expect("a photo is active"); + assert!(!before.contains(&active), "the active photo is a new one: {active:?}"); + assert_eq!(h.app.session.source, lightcraft_engine::LibrarySource::All); + assert!(h.app.session.visible_cloned().contains(&active), "the grid shows the active photo"); + let _ = std::fs::remove_dir_all(&dir); + } + /// Import dialog, copy mode: a chosen destination, one folder, renamed copies numbered across /// the import's batches. #[test] diff --git a/crates/ui-egui/src/import.rs b/crates/ui-egui/src/import.rs index 4e797a572..a1a720a8e 100644 --- a/crates/ui-egui/src/import.rs +++ b/crates/ui-egui/src/import.rs @@ -262,6 +262,11 @@ pub struct ImportTask { auto: bool, /// Auto Import: the selection to keep. keep_selection: Option, + /// The selection and source as the import last left them: a change between frames is the + /// user choosing another photo or view, which the end of the import doesn't take away. + left: Option<(lightcraft_engine::Selection, lightcraft_engine::LibrarySource)>, + /// The user chose another photo or view while the import ran. + moved_on: bool, run: Option, /// The import's row in the activity stack (its ✕ and `activity.cancel` set the run's cancel flag). guard: Option, @@ -689,6 +694,9 @@ pub fn tick(app: &mut LightcraftApp, ctx: &egui::Context) { let Some(run) = task.run.as_mut() else { return }; let batches = run.next_batches(); let finished = batches.is_none(); + if task.left.as_ref().is_some_and(|(sel, source)| *sel != app.session.selection || *source != app.session.source) { + task.moved_on = true; + } for prepared in batches.into_iter().flatten() { commit_batch(app, &mut task, prepared); // arriving photos don't take over the selection @@ -696,6 +704,7 @@ pub fn tick(app: &mut LightcraftApp, ctx: &egui::Context) { app.session.selection = sel.clone(); } } + task.left = Some((app.session.selection.clone(), app.session.source)); if let Some(run) = &task.run { task.total = run.total.load(Ordering::Relaxed).max(task.total); task.done = run.done.load(Ordering::Relaxed); @@ -807,7 +816,8 @@ fn xmp_warnings_text(warnings: &[ImportWarning]) -> String { ) } -/// The import is done (or cancelled): one undo step, select the first photo, say what happened. +/// The import is done (or cancelled): one undo step, show and select the first photo (unless the +/// user chose another meanwhile), say what happened. fn finish(app: &mut LightcraftApp, ctx: &egui::Context, task: ImportTask) { let steps = app.session.undo.len().saturating_sub(task.undo0); let plural = |n: usize| if n == 1 { "" } else { "s" }; @@ -835,7 +845,12 @@ fn finish(app: &mut LightcraftApp, ctx: &egui::Context, task: ImportTask) { } return; } - if let Some(f) = task.first { + if let Some(f) = task.first.filter(|_| !task.moved_on) { + // a view the new photos aren't in (an album, Picks, a Local folder) would leave the active + // photo off screen: show All Photos, as Lightroom shows the import's own collection (#1446) + if !app.session.visible().contains(&lightcraft_catalog::PhotoId(f)) { + let _ = app.run("library.source", json!({"kind": "all"})); + } let _ = app.run("library.select", json!({"ids": [f]})); } let plural = |n: usize| if n == 1 { "" } else { "s" }; diff --git a/crates/ui-egui/src/tests_activity.rs b/crates/ui-egui/src/tests_activity.rs index 8833ddc79..34c7f7491 100644 --- a/crates/ui-egui/src/tests_activity.rs +++ b/crates/ui-egui/src/tests_activity.rs @@ -281,6 +281,24 @@ fn import_cancelled_by_command_reports_cancelled() { let _ = std::fs::remove_dir_all(&dir); } +/// A photo chosen while an import runs stays the active one when the import ends: the import +/// selects its first photo only if nobody picked something else meanwhile (issue #1446). +#[test] +fn a_photo_chosen_during_an_import_stays_selected() { + let (mut h, dir) = slow_import("chosen", 20); + crate::import::start_paths(&mut h.app, vec![dir.to_string_lossy().to_string()]).unwrap(); + assert!(h.step_until(Duration::from_secs(60), |h| h.app.import.as_ref().is_some_and(|t| t.imported >= 2))); + // the newest photo so far, never the import's first + let chosen = h.app.session.catalog.photos().map(|p| p.id).max().expect("photos arrived"); + let r = h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [chosen.0]}}), T); + assert_eq!(r["ok"], true, "{r}"); + assert!(h.app.import.is_some(), "still importing"); + assert!(h.step_until(Duration::from_secs(60), |h| h.app.import.is_none())); + assert_eq!(h.app.session.catalog.len(), 20, "all imported"); + assert_eq!(h.app.session.selection.active, Some(chosen)); + let _ = std::fs::remove_dir_all(&dir); +} + #[test] fn scan_shows_a_row_and_cancel_closes_it() { let (mut h, dir) = slow_import("scan", 120);