From 4b805d64bc0975413d8289d7f2a9111795541c0d Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sat, 31 Oct 2026 06:19:13 -0400 Subject: [PATCH] Objects: each drag of an object is its own undo step A drag merges its frames into one undo step by dropping a new step whose label matches the one before it. Its first frame did the same, so a second drag of an object (a move after a move, or a resize after a move) merged into the first drag's "Move Object" step, and one undo took back both. `Drag::Object` records whether the drag has made its step; only the later frames of the same drag merge into it. Test: a move and then a resize of a picture take two undos to reverse, the first restoring the moved picture. --- crates/ui-egui/src/grid.rs | 3 +++ crates/ui-egui/src/grid/interact.rs | 16 ++++++++++++---- crates/ui-egui/tests/it/objects.rs | 20 ++++++++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 0e20d7e0b..85ff53b07 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -137,6 +137,9 @@ pub enum Drag { start: Pos2, orig: (CellRef, f32, f32, f32, f32), handle: Option, + /// This drag has made its undo step; later frames merge into it, and never into the + /// step of an earlier drag. + stepped: bool, }, } diff --git a/crates/ui-egui/src/grid/interact.rs b/crates/ui-egui/src/grid/interact.rs index 84d8dbc33..a7bbede7e 100644 --- a/crates/ui-egui/src/grid/interact.rs +++ b/crates/ui-egui/src/grid/interact.rs @@ -134,7 +134,7 @@ pub(super) fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Respo } if primary_drag && let Some(a) = sh.object_anchor(obj) { let handle = crate::objects::handle_at(rect, p); - app.grid.drag = Drag::Object { obj, start: p, orig: (a.cell, a.dx, a.dy, a.width, a.height), handle }; + app.grid.drag = Drag::Object { obj, start: p, orig: (a.cell, a.dx, a.dy, a.width, a.height), handle, stepped: false }; // The first recognised drag frame already holds movement away from the press. if let Some(now) = pos { drag_object(app, geo, sh, now); @@ -956,9 +956,9 @@ pub(super) fn hidden_row_edge(geo: &Geo, sh: &Sheet, y: f32) -> Option { } /// Moves the dragged object, or resizes it from the grabbed handle, to follow the pointer at `p` -/// (logical coordinates). The whole drag coalesces into one undo step. +/// (logical coordinates). The whole drag coalesces into one undo step of its own. fn drag_object(app: &mut SheetApp, geo: &Geo, sh: &Sheet, p: Pos2) { - let Drag::Object { obj, start, orig, handle } = app.grid.drag.clone() else { return }; + let Drag::Object { obj, start, orig, handle, stepped } = app.grid.drag.clone() else { return }; let d = (p - start) / geo.z; let (cell, dx, dy, w, h) = orig; let x0 = sh.col_left(cell.col) as f32 + dx; @@ -977,8 +977,16 @@ fn drag_object(app: &mut SheetApp, geo: &Geo, sh: &Sheet, p: Pos2) { "dx": x0 - sh.col_left(nc.col) as f32, "dy": y0 - sh.row_top(nc.row) as f32, "width": x1 - x0, "height": y1 - y0, }); + let revision = app.session.active().map(|d| d.revision); let _ = app.session.run("object.move", params); - coalesce_last_undo(app); + if stepped { + coalesce_last_undo(app); + } else if app.session.active().map(|d| d.revision) != revision + && let Drag::Object { stepped, .. } = &mut app.grid.drag + { + // The first step of this drag stays apart from an earlier drag's step of the same name. + *stepped = true; + } } /// A click on a hyperlink. References select; web and e-mail addresses ask first (the dialog's diff --git a/crates/ui-egui/tests/it/objects.rs b/crates/ui-egui/tests/it/objects.rs index b9912559c..521a2f44c 100644 --- a/crates/ui-egui/tests/it/objects.rs +++ b/crates/ui-egui/tests/it/objects.rs @@ -324,3 +324,23 @@ fn a_handle_under_the_headers_does_not_take_the_press() { drag(&mut h, grab, grab + vec2(40.0, 0.0)); assert_eq!(rect(&h, o.shape), before, "the shape is neither moved nor resized"); } + +#[test] +fn each_drag_of_an_object_is_its_own_undo_step() { + let (mut h, o) = fixture(); + let start = rect(&h, o.image); + click(&mut h, start.center()); + drag(&mut h, start.center(), start.center() + vec2(30.0, 20.0)); + let moved = rect(&h, o.image); + assert!((moved.min - start.min - vec2(30.0, 20.0)).length() < 1.0); + let grab = Handle::BottomRight.pos(moved); + drag(&mut h, grab, grab + vec2(24.0, 18.0)); + let resized = rect(&h, o.image); + assert!((resized.size() - moved.size() - vec2(24.0, 18.0)).length() < 1.0); + h.state_mut().run("edit.undo", json!({})).unwrap(); + h.run_steps(2); + assert_eq!(rect(&h, o.image), moved, "one undo takes back the resize only"); + h.state_mut().run("edit.undo", json!({})).unwrap(); + h.run_steps(2); + assert_eq!(rect(&h, o.image), start, "a second undo takes back the move"); +}