Fix #293: undo and redo show the photo they change
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 instead of happening to a photo out of sight. It joins the selection if it isn't in it; steps that touch several photos, albums or files, and photos outside the current view, leave the selection alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01912WaK6Rmew3wxZuPyX7E8
This commit is contained in:
parent
2472021091
commit
3a9b6ccee4
|
|
@ -82,6 +82,27 @@ pub struct UndoEntry {
|
|||
pub folder: Option<FolderMove>,
|
||||
}
|
||||
|
||||
/// 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<PhotoId> {
|
||||
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 {
|
||||
|
|
@ -423,6 +444,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)
|
||||
|
|
@ -437,11 +459,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
|
||||
|
|
|
|||
|
|
@ -817,3 +817,29 @@ fn face_job_follows_rotate_right() {
|
|||
let (pw, ph) = ((r.x1 - r.x0) * h, (r.y1 - r.y0) * w);
|
||||
assert!((pw - ph).abs() < 1e-6 * w.max(h), "square in rotated pixels: {pw} × {ph}");
|
||||
}
|
||||
|
||||
/// 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");
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue