Fix some issues with undo/redo in mask editor (#2649)

Issues:
- The `change` event was not dispatched in MaskEditor.svelte when an
undo/redo was performed. Therefore, if the user then closed the editor
or switched to another note without performing an operation that would
cause the `change` event to be dispatched, the undone or redone changes
were not saved to DB.
- When `IOMode.kind === "edit"` (i.e., Edit Current or Browse), the
beginning of the undo history was a blank canvas, not a canvas with
existing masks. Therefore, if you continued to undo to the beginning of
the history, the masks that existed when you opened the editor would be
lost, and they would not be restored even when you performed a redo.
- In the 'Add' dialog, the undo history was not reset when starting to
create a new IO note after adding an IO note.

Also add a small UI improvement:
The undo/redo buttons are now disabled when there is no action to
undo/redo.

ts/image-occlusion/MaskEditor.svelte

@@ -2,6 +2,16 @@
 Copyright: Ankitects Pty Ltd and contributors
 License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
+<script context="module" lang="ts">
+    import { writable } from "svelte/store";
+
+    const changeSignal = writable(Symbol());
+
+    export function emitChangeSignal() {
+        changeSignal.set(Symbol());
+    }
+</script>
+
 <script lang="ts">
     import type { PanZoom } from "panzoom";
     import panzoom from "panzoom";
@@ -28,6 +38,8 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         dispatch("change", { canvas });
     }
 
+    $: $changeSignal, onChange();
+
     function init(node) {
         instance = panzoom(node, {
             bounds: true,

ts/image-occlusion/Toolbar.svelte

@@ -29,7 +29,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         zoomTools,
     } from "./tools/more-tools";
     import { tools } from "./tools/tool-buttons";
-    import { undoRedoTools } from "./tools/tool-undo-redo";
+    import { undoRedoTools, undoStack } from "./tools/tool-undo-redo";
 
     export let canvas;
     export let instance;
@@ -172,9 +172,10 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
                     ? 'left-border-radius'
                     : 'right-border-radius'}"
                 {iconSize}
-                on:click={() => {
+                on:click={tool.action}
-                    tool.action(canvas);
+                disabled={tool.name === "undo"
-                }}
+                    ? !$undoStack.undoable
+                    : !$undoStack.redoable}
             >
                 {@html tool.icon}
             </IconButton>

ts/image-occlusion/mask-editor.ts

@@ -13,7 +13,7 @@ import { notesDataStore, tagsWritable, zoomResetValue } from "./store";
 import Toast from "./Toast.svelte";
 import { addShapesToCanvasFromCloze } from "./tools/add-from-cloze";
 import { enableSelectable, moveShapeToCanvasBoundaries } from "./tools/lib";
-import { undoRedoInit } from "./tools/tool-undo-redo";
+import { undoStack } from "./tools/tool-undo-redo";
 import type { Size } from "./types";
 
 export const setupMaskEditor = async (
@@ -34,6 +34,7 @@ export const setupMaskEditor = async (
         image.height = size.height;
         image.width = size.width;
         setCanvasZoomRatio(canvas, instance);
+        undoStack.reset();
     };
 
     return canvas;
@@ -75,6 +76,7 @@ export const setupMaskEditorForEdit = async (
         addShapesToCanvasFromCloze(canvas, clozeNote.occlusions);
         enableSelectable(canvas, true);
         addClozeNotesToTextEditor(clozeNote.header, clozeNote.backExtra, clozeNote.tags);
+        undoStack.reset();
         window.requestAnimationFrame(() => {
             image.style.visibility = "visible";
         });
@@ -87,11 +89,11 @@ function initCanvas(onChange: () => void): fabric.Canvas {
     const canvas = new fabric.Canvas("canvas");
     tagsWritable.set([]);
     globalThis.canvas = canvas;
+    undoStack.setCanvas(canvas);
     // enables uniform scaling by default without the need for the Shift key
     canvas.uniformScaling = false;
     canvas.uniScaleKey = "none";
     moveShapeToCanvasBoundaries(canvas);
-    undoRedoInit(canvas);
     canvas.on("object:modified", onChange);
     canvas.on("object:removed", onChange);
     return canvas;

ts/image-occlusion/tools/tool-ellipse.ts

@@ -4,9 +4,7 @@
 import { fabric } from "fabric";
 
 import { BORDER_COLOR, disableRotation, SHAPE_MASK_COLOR, stopDraw } from "./lib";
-import { objectAdded } from "./tool-undo-redo";
+import { undoStack } from "./tool-undo-redo";
-
-const addedEllipseIds: string[] = [];
 
 export const drawEllipse = (canvas: fabric.Canvas): void => {
     canvas.selectionColor = "rgba(0, 0, 0, 0)";
@@ -117,6 +115,6 @@ export const drawEllipse = (canvas: fabric.Canvas): void => {
         }
 
         ellipse.setCoords();
-        objectAdded(canvas, addedEllipseIds, ellipse.id);
+        undoStack.onObjectAdded(ellipse.id);
     });
 };

ts/image-occlusion/tools/tool-polygon.ts

@@ -5,7 +5,7 @@ import { fabric } from "fabric";
 import type { PanZoom } from "panzoom";
 
 import { BORDER_COLOR, disableRotation, SHAPE_MASK_COLOR } from "./lib";
-import { objectAdded, saveCanvasState } from "./tool-undo-redo";
+import { undoStack } from "./tool-undo-redo";
 
 let activeLine;
 let activeShape;
@@ -13,7 +13,6 @@ let linesList: fabric.Line = [];
 let pointsList: fabric.Circle = [];
 let drawMode = false;
 let zoomValue = 1;
-const addedPolygonIds: string[] = [];
 
 export const drawPolygon = (canvas: fabric.Canvas, panzoom: PanZoom): void => {
     canvas.selectionColor = "rgba(0, 0, 0, 0)";
@@ -190,12 +189,12 @@ const generatePolygon = (canvas: fabric.Canvas, pointsList): void => {
         disableRotation(polygon);
         canvas.add(polygon);
         // view undo redo tools
-        objectAdded(canvas, addedPolygonIds, polygon.id);
+        undoStack.onObjectAdded(polygon.id);
     }
 
     polygon.on("modified", () => {
         modifiedPolygon(canvas, polygon);
-        saveCanvasState(canvas);
+        undoStack.onObjectModified();
     });
 
     toggleDrawPolygon(canvas);
@@ -223,7 +222,7 @@ const modifiedPolygon = (canvas: fabric.Canvas, polygon: fabric.Polygon): void =
 
     polygon1.on("modified", () => {
         modifiedPolygon(canvas, polygon1);
-        saveCanvasState(canvas);
+        undoStack.onObjectModified();
     });
 
     canvas.remove(polygon);

ts/image-occlusion/tools/tool-rect.ts

@@ -4,9 +4,7 @@
 import { fabric } from "fabric";
 
 import { BORDER_COLOR, disableRotation, SHAPE_MASK_COLOR, stopDraw } from "./lib";
-import { objectAdded } from "./tool-undo-redo";
+import { undoStack } from "./tool-undo-redo";
-
-const addedRectangleIds: string[] = [];
 
 export const drawRectangle = (canvas: fabric.Canvas): void => {
     canvas.selectionColor = "rgba(0, 0, 0, 0)";
@@ -111,6 +109,6 @@ export const drawRectangle = (canvas: fabric.Canvas): void => {
         }
 
         rect.setCoords();
-        objectAdded(canvas, addedRectangleIds, rect.id);
+        undoStack.onObjectAdded(rect.id);
     });
 };

ts/image-occlusion/tools/tool-undo-redo.ts

@@ -2,89 +2,137 @@
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 import type fabric from "fabric";
+import { writable } from "svelte/store";
 
 import { mdiRedo, mdiUndo } from "../icons";
+import { emitChangeSignal } from "../MaskEditor.svelte";
 
 /**
  * Undo redo for rectangle and ellipse handled here,
  * view tool-polygon for handling undo redo in case of polygon
  */
 
-let lockHistory = false;
+type UndoState = {
-const undoHistory: string[] = [];
+    undoable: boolean;
-const redoHistory: string[] = [];
+    redoable: boolean;
+};
 
 const shapeType = ["rect", "ellipse"];
 
-export const undoRedoInit = (canvas: fabric.Canvas): void => {
-    undoHistory.push(JSON.stringify(canvas));
-
-    canvas.on("object:modified", function(o) {
-        if (lockHistory) return;
-        if (!validShape(o.target as fabric.Object)) return;
-        saveCanvasState(canvas);
-    });
-
-    canvas.on("object:removed", function(o) {
-        if (lockHistory) return;
-        if (!validShape(o.target as fabric.Object)) return;
-        saveCanvasState(canvas);
-    });
-};
-
 const validShape = (shape: fabric.Object): boolean => {
     if (shape.width <= 5 || shape.height <= 5) return false;
     if (shapeType.indexOf(shape.type) === -1) return false;
     return true;
 };
 
-export const undoAction = (canvas: fabric.Canvas): void => {
+class UndoStack {
-    if (undoHistory.length > 0) {
+    private stack: string[] = [];
-        lockHistory = true;
+    private index = -1;
-        if (undoHistory.length > 1) redoHistory.push(undoHistory.pop() as string);
+    private canvas: fabric.Canvas | undefined;
-        const content = undoHistory[undoHistory.length - 1];
+    private locked = false;
-        canvas.loadFromJSON(content, function() {
+    private shapeIds = new Set<string>();
-            canvas.renderAll();
+    /** used to make the toolbar buttons reactive */
-            lockHistory = false;
+    private state = writable<UndoState>({ undoable: false, redoable: false });
+    subscribe: typeof this.state.subscribe;
+
+    constructor() {
+        // allows an instance of the class to act as a store
+        this.subscribe = this.state.subscribe;
+    }
+
+    setCanvas(canvas: fabric.Canvas): void {
+        this.canvas = canvas;
+        this.canvas.on("object:modified", (opts) => this.maybePush(opts));
+        this.canvas.on("object:removed", (opts) => this.maybePush(opts));
+    }
+
+    reset(): void {
+        this.shapeIds.clear();
+        this.stack.length = 0;
+        this.index = -1;
+        this.push();
+        this.updateState();
+    }
+
+    private canUndo(): boolean {
+        return this.index > 0;
+    }
+
+    private canRedo(): boolean {
+        return this.index < this.stack.length - 1;
+    }
+
+    private updateState(): void {
+        this.state.set({
+            undoable: this.canUndo(),
+            redoable: this.canRedo(),
         });
     }
-};
 
-export const redoAction = (canvas: fabric.Canvas): void => {
+    private updateCanvas(): void {
-    if (redoHistory.length > 0) {
+        this.locked = true;
-        lockHistory = true;
+        this.canvas?.loadFromJSON(this.stack[this.index], () => {
-        const content = redoHistory.pop() as string;
+            this.canvas?.renderAll();
-        undoHistory.push(content);
+            emitChangeSignal();
-        canvas.loadFromJSON(content, function() {
+            this.locked = false;
-            canvas.renderAll();
-            lockHistory = false;
         });
     }
-};
 
-export const objectAdded = (canvas: fabric.Canvas, shapeIdList: string[], shapeId: string): void => {
+    onObjectAdded(id: string): void {
-    if (shapeIdList.includes(shapeId)) {
+        if (!this.shapeIds.has(id)) {
-        return;
+            this.push();
+        }
+        this.shapeIds.add(id);
     }
 
-    shapeIdList.push(shapeId);
+    onObjectModified(): void {
-    saveCanvasState(canvas);
+        this.push();
-};
+    }
 
-export const saveCanvasState = (canvas: fabric.Canvas): void => {
+    private maybePush(opts): void {
-    undoHistory.push(JSON.stringify(canvas));
+        if (
-    redoHistory.length = 0;
+            !this.locked
-};
+            && validShape(opts.target as fabric.Object)
+        ) {
+            this.push();
+        }
+    }
+
+    private push(): void {
+        this.stack.length = this.index + 1;
+        this.stack.push(JSON.stringify(this.canvas));
+        this.index++;
+        this.updateState();
+    }
+
+    undo(): void {
+        if (this.canUndo()) {
+            this.index--;
+            this.updateState();
+            this.updateCanvas();
+        }
+    }
+
+    redo(): void {
+        if (this.canRedo()) {
+            this.index++;
+            this.updateState();
+            this.updateCanvas();
+        }
+    }
+}
+
+export const undoStack = new UndoStack();
 
 export const undoRedoTools = [
     {
         name: "undo",
         icon: mdiUndo,
-        action: undoAction,
+        action: () => undoStack.undo(),
     },
     {
         name: "redo",
         icon: mdiRedo,
-        action: redoAction,
+        action: () => undoStack.redo(),
     },
 ];