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)

This commit is contained in:
Marco Arnulfo committed 2026-10-13 09:04:10 +02:00
1 parent b3d392b499
commit 0d8e3e3913
3 files changed
+69 -2

No files matched your search

+34
View File
@@ -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<lightcraft_catalog::PhotoId> = 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]
+17 -2
View File
@@ -262,6 +262,11 @@ pub struct ImportTask {
auto: bool,
/// Auto Import: the selection to keep.
keep_selection: Option<lightcraft_engine::Selection>,
/// 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<ImportRun>,
/// The import's row in the activity stack (its ✕ and `activity.cancel` set the run's cancel flag).
guard: Option<lightcraft_engine::activity::TaskGuard>,
@@ -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" };
+18
View File
@@ -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);