fix: undo and redo of a move put the objects back where they were

CHANGELOG.md

@@ -1,5 +1,23 @@
 # Changelog
 
+## 1.2.0
+
+### Fixed
+
+- Undo and redo of a move, scale or rotation of a straight line (the line tool) put the line at the top left
+  corner of the canvas, where it was not seen. A line has its points relative to its center, and setting them
+  again moved it to the origin. Undo and redo now set the properties which the modification changed, and no
+  others.
+- Undo after the move, scale or rotation of a selection of many objects removed the last object which was drawn,
+  as the modification of a selection was not recorded. It is now one action, which undo and redo apply to all
+  the objects of the selection, and the selection is left when they do.
+- Undo of the move of a rectangle with a label made the rectangle disappear, as it went back to the size it had
+  when the pointer was pressed to draw it, and left its text box behind. They go back together, to where they
+  were.
+- Undo of a modification goes back to the state of the object when the pointer was pressed on it, and not to the
+  one it had when it was added or last modified by the history. A modification which changes nothing is not an
+  action to undo.
+
 ## 1.1.0 (2026-11-04)
 
 The eraser, laser pointer and polygon tools, pinch zoom, and features that forks of react-sketch had, written again for the TypeScript code and fabric 7.

packages/react-sketch/src/components/SketchField/events.ts

@@ -116,6 +116,8 @@ export class FabricCanvasEventsManager
 
   public onMouseDown({ event, tool, props }: EventInput): void {
     this.changed = false;
+    // What is pressed is what the press may move, scale or rotate: its state now is the one undo goes back to
+    if (event.target) this.history.onMouseDown(event.target);
     tool.doMouseDown(event);
     props.onMouseDown?.(sketchEvent(event));
   }

packages/react-sketch/src/components/SketchField/history/SketchHistory.ts

@@ -1,14 +1,25 @@
 import * as fabric from "fabric";
 import { UndoHistory } from "./index";
 import type { Erasure } from "../tools/Eraser";
+import { linkedObjects } from "../tools/RectangleLabel/RectangleLabelObject";
 
 /**
- * One undoable action of the sketch
+ * What a modification did to an object: its state (as JSON) before and after it
+ */
+interface Change {
+  object: fabric.FabricObject;
+  before: string;
+  after: string;
+}
+
+/**
+ * One undoable action of the sketch. A modification can be of many objects, as
+ * the ones of a selection which is moved.
  */
 type HistoryEntry =
   | { action: "add"; object: fabric.FabricObject }
   | { action: "remove"; object: fabric.FabricObject }
-  | { action: "modify"; object: fabric.FabricObject; before: string; after: string }
+  | { action: "modify"; changes: Change[] }
   | { action: "erase"; erasures: Erasure[] };
 
 /**
@@ -21,6 +32,68 @@ const NOT_RESTORED = ["type", "version", "objects", "layoutManager", "clipPath"]
 const stateOf = (object: fabric.FabricObject): string =>
   JSON.stringify(object.toJSON());
 
+/** The properties which say where an object is, and how it is turned and scaled */
+const PLACEMENT = [
+  "left",
+  "top",
+  "scaleX",
+  "scaleY",
+  "angle",
+  "skewX",
+  "skewY",
+  "flipX",
+  "flipY",
+] as const;
+
+/**
+ * State of an object as it is on the canvas on its own. The object of a
+ * selection has its place relative to the selection (its `left` and `top` are
+ * not the ones of the canvas), so the transform of the selection is applied to
+ * it for the state, and then it is put back as it was.
+ */
+const stateOutsideGroup = (object: fabric.FabricObject): string => {
+  const { group } = object;
+  if (!group) return stateOf(object);
+  const own = Object.fromEntries(PLACEMENT.map((key) => [key, object[key]]));
+  fabric.util.addTransformToObject(object, group.calcTransformMatrix());
+  try {
+    return stateOf(object);
+  } finally {
+    object.set(own);
+    object.setCoords();
+  }
+};
+
+/**
+ * The objects which a press or a modification is about: the ones of a selection, or the
+ * one object, and the objects which follow them (the pointer moves the text box of a
+ * label with its rectangle, and nothing tells the history)
+ */
+const objectsOf = (target: fabric.FabricObject): fabric.FabricObject[] => {
+  const objects =
+    target instanceof fabric.ActiveSelection ? target.getObjects() : [target];
+  return [
+    ...new Set(objects.flatMap((object) => [object, ...linkedObjects(object)])),
+  ];
+};
+
+/**
+ * The properties of `state` which are not the ones of `other`: what a modification
+ * changed, as undo and redo set nothing else. Setting all of them is not harmless:
+ * a line has its points relative to its center, and the canvas takes them for the
+ * ones of the canvas when they are set, so the line is moved to the origin.
+ */
+const changedProps = (state: string, other: string): Record<string, unknown> => {
+  const props = JSON.parse(state);
+  const others = JSON.parse(other);
+  NOT_RESTORED.forEach((key) => delete props[key]);
+  Object.keys(props).forEach((key) => {
+    if (JSON.stringify(props[key]) === JSON.stringify(others[key]))
+      delete props[key];
+  });
+  return props;
+};
+
 /**
  * Records what happens to the objects of a canvas (added, modified, removed)
  * and applies undo and redo on them.
@@ -30,8 +103,10 @@ export class SketchHistory {
   private readonly entries: UndoHistory<HistoryEntry>;
   /** Last known state of each object, it is the "before" of its next modification */
   private readonly snapshots = new WeakMap<fabric.FabricObject, string>();
-  /** The object added since the last mouse up, whose final state is not known yet */
-  private pending: fabric.FabricObject | null = null;
+  /** The objects added since the last mouse up, whose final state is not known yet */
+  private readonly pending = new Set<fabric.FabricObject>();
+  /** The state the objects pressed had when they were, which is the "before" of the modification the press makes */
+  private readonly pressed = new Map<fabric.FabricObject, string>();
   private applying = false;
 
   /**
@@ -56,7 +131,7 @@ export class SketchHistory {
    */
   onObjectAdded(object: fabric.FabricObject): void {
     this.snapshots.set(object, stateOf(object));
-    this.pending = object;
+    this.pending.add(object);
     this.entries.keep({ action: "add", object });
   }
 
@@ -72,19 +147,39 @@ export class SketchHistory {
     ) {
       this.entries.pop();
     }
-    if (this.pending && objects.includes(this.pending)) this.pending = null;
+    objects.forEach((object) => this.pending.delete(object));
+  }
+
+  /**
+   * The state of what is pressed, before the press changes it. A modification goes back
+   * to it, whatever the objects were before the press: the snapshots are the state they
+   * had when they were added, which is not the one of an object which something else
+   * changed since.
+   */
+  onMouseDown(target: fabric.FabricObject): void {
+    this.pressed.clear();
+    objectsOf(target).forEach((object) =>
+      this.pressed.set(object, stateOutsideGroup(object))
+    );
   }
 
   /**
-   * Record an object modified on the canvas. Objects which were not added
-   * through {@link onObjectAdded} (for example a multiple selection) are ignored.
+   * Record a modification on the canvas: of an object, or of the objects of a selection
+   * (one action for all of them). The objects which the history does not know, as they
+   * were not added through {@link onObjectAdded}, are ignored, and so is a modification
+   * which changed nothing.
    */
-  onObjectModified(object: fabric.FabricObject): void {
-    const before = this.snapshots.get(object);
-    if (before === undefined) return;
-    const after = stateOf(object);
-    this.snapshots.set(object, after);
-    this.entries.keep({ action: "modify", object, before, after });
+  onObjectModified(target: fabric.FabricObject): void {
+    const changes: Change[] = [];
+    objectsOf(target).forEach((object) => {
+      const before = this.pressed.get(object) ?? this.snapshots.get(object);
+      if (before === undefined) return;
+      const after = stateOutsideGroup(object);
+      this.snapshots.set(object, after);
+      if (after !== before) changes.push({ object, before, after });
+    });
+    this.pressed.clear();
+    if (changes.length) this.entries.keep({ action: "modify", changes });
   }
 
   /**
@@ -100,10 +195,8 @@ export class SketchHistory {
    * the mouse), so its state is only final on mouse up.
    */
   onMouseUp(): void {
-    if (this.pending) {
-      this.snapshots.set(this.pending, stateOf(this.pending));
-      this.pending = null;
-    }
+    this.pending.forEach((object) => this.snapshots.set(object, stateOf(object)));
+    this.pending.clear();
   }
 
   /**
@@ -154,7 +247,8 @@ export class SketchHistory {
    */
   clear(): void {
     this.entries.clear();
-    this.pending = null;
+    this.pending.clear();
+    this.pressed.clear();
   }
 
   /**
@@ -177,34 +271,47 @@ export class SketchHistory {
     object.set("dirty", true);
   }
 
+  /**
+   * Set the objects of a modification to their state before it (undo) or after it
+   * (redo): the properties it changed, and no others
+   */
+  private applyChanges(changes: Change[], forward: boolean): void {
+    // The objects of a selection have a place relative to it: they are given the ones of the canvas
+    const active = this.canvas.getActiveObject();
+    if (
+      active instanceof fabric.ActiveSelection &&
+      changes.some(({ object }) => active.contains(object))
+    ) {
+      this.canvas.discardActiveObject();
+    }
+    changes.forEach(({ object, before, after }) => {
+      const [state, other] = forward ? [after, before] : [before, after];
+      object.set(changedProps(state, other));
+      object.setCoords();
+      this.snapshots.set(object, state);
+    });
+    this.pressed.clear();
+  }
+
   private apply(entry: HistoryEntry, forward: boolean): void {
     this.applying = true;
     try {
-      if (entry.action === "erase") {
-        entry.erasures.forEach((erasure) => this.applyErasure(erasure, forward));
-        this.canvas.renderAll();
-        return;
-      }
-      const { object } = entry;
       switch (entry.action) {
+        case "erase":
+          entry.erasures.forEach((erasure) => this.applyErasure(erasure, forward));
+          break;
+        case "modify":
+          this.applyChanges(entry.changes, forward);
+          break;
         case "add":
         case "remove":
           // redo of an add, and undo of a remove, put the object back
           if ((entry.action === "add") === forward) {
-            this.canvas.add(object);
+            this.canvas.add(entry.object);
           } else {
-            this.canvas.remove(object);
+            this.canvas.remove(entry.object);
           }
           break;
-        case "modify": {
-          const state = forward ? entry.after : entry.before;
-          const props = JSON.parse(state);
-          NOT_RESTORED.forEach((key) => delete props[key]);
-          object.set(props);
-          object.setCoords();
-          this.snapshots.set(object, state);
-          break;
-        }
       }
       this.canvas.renderAll();
     } finally {