diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 7506068e..fab17896 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -83,6 +83,27 @@ pub struct UndoEntry { pub folder: Option, } +/// The one photo an undo step changes, when it changes exactly one through its edit, rating, flag, +/// label, metadata or versions (`depth` bounds nested batches). +fn single_photo(op: &Op, depth: usize) -> Option { + match op { + Op::SetDevelop { id, .. } + | Op::SetRating { id, .. } + | Op::SetFlag { id, .. } + | Op::SetLabel { id, .. } + | Op::SetMeta { id, .. } + | Op::SetVersions { id, .. } + | Op::SetHistory { id, .. } + | Op::PushHistory { id, .. } => Some(*id), + Op::Batch { ops } if depth < 8 => { + let mut ids = ops.iter().map(|o| single_photo(o, depth + 1)); + let first = ids.next()??; + ids.all(|id| id == Some(first)).then_some(first) + } + _ => None, + } +} + /// A folder renamed or moved on disk as part of an undo step. #[derive(Clone, Debug, PartialEq, Eq)] pub struct FolderMove { @@ -428,6 +449,7 @@ impl Session { return Err(err); } }; + self.show_undone(&e.op); self.pending_log.push(e.op); self.redo.push(UndoEntry { label: e.label.clone(), op: redo, folder: e.folder.as_ref().map(FolderMove::reversed) }); Ok(e.label) @@ -442,11 +464,30 @@ impl Session { return Err(err); } }; + self.show_undone(&e.op); self.pending_log.push(e.op); self.undo.push(UndoEntry { label: e.label.clone(), op: undo, folder: e.folder.as_ref().map(FolderMove::reversed) }); Ok(e.label) } + /// After an undo or redo that changed one photo (its edit, rating, flag, label, metadata or + /// versions), that photo becomes the active one, as in Lightroom Classic, so the change is on + /// screen (issue #293). It joins the selection if it isn't in it; nothing changes for steps + /// that touch several photos, albums or files, or for a photo not in the current view. + fn show_undone(&mut self, op: &Op) { + let Some(id) = single_photo(op, 0) else { return }; + if self.selection.active == Some(id) || !self.visible().contains(&id) { + return; + } + if !self.selection.contains(id) { + self.selection = Selection { ids: vec![id], active: Some(id) }; + } else { + self.selection.active = Some(id); + } + self.active_mask = None; + self.active_spot = None; + } + /// Apply an undo/redo op, first renaming the step's folder and moving the files its renames /// imply (all or nothing). Files that can't be moved back after a failure are reported and the /// library follows them (a logged change outside the undo history), so none goes missing; the diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 0dd76887..2b09c586 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -925,3 +925,29 @@ fn saved_presets_with_unknown_keys_still_export() { assert_eq!(o.watermark.map(|w| w.text), Some("©".to_string())); assert!(ExportOptions::from_params(&s.export_params(&json!({"preset": "old", "qualty": 80})).unwrap()).is_err()); } + +/// Undo and redo bring the photo they change on screen (issue #293): after editing one photo and +/// moving on, Undo makes it the active photo again; a step that changes several photos leaves the +/// selection alone. +#[test] +fn undo_and_redo_show_the_photo_they_change() { + let mut s = demo(); + let vis = s.visible_cloned(); + let (a, b) = (vis[0], vis[1]); + s.execute("library.select", &json!({"ids": [a.0]})).unwrap(); + s.execute("develop.set", &json!({"control": "light.exposure", "value": 1.0})).unwrap(); + s.execute("library.select", &json!({"ids": [b.0]})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.active(), Some(a), "undo shows the photo it changed"); + assert_eq!(s.selection.ids, vec![a]); + s.execute("library.select", &json!({"ids": [b.0]})).unwrap(); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!(s.active(), Some(a), "so does redo"); + // rating both selected photos is one step over two photos: undoing it leaves the selection + s.execute("library.select", &json!({"ids": [a.0, b.0], "active": b.0})).unwrap(); + s.execute("photo.rate", &json!({"rating": 3})).unwrap(); + s.execute("library.select", &json!({"ids": [a.0, b.0], "active": a.0})).unwrap(); + let before = s.selection.clone(); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.selection, before, "a step over several photos keeps the selection"); +}