fix: decouple actionFinalize and actionErase (#4984)

* Update actionCanvas.tsx

* Update actionFinalize.tsx

* lint

* remove Escape trigger from actionErase

* revert to lastActiveTool only if coming from eraser tool

* unrelated: fix restoring `appState.activeTool`

* one more restoring fix

* fix tests

Co-authored-by: dwelle <luzar.david@gmail.com>

src/actions/actionCanvas.tsx

@@ -323,12 +323,7 @@ export const actionErase = register({
       commitToHistory: true,
     };
   },
-  keyTest: (event, appState) => {
-    return (
-      event.key === KEYS.E ||
-      (event.key === KEYS.ESCAPE && isEraserActive(appState))
-    );
-  },
+  keyTest: (event) => event.key === KEYS.E,
   PanelComponent: ({ elements, appState, updateData, data }) => (
     <ToolButton
       type="button"

src/actions/actionFinalize.tsx

@@ -14,7 +14,6 @@ import {
   bindOrUnbindLinearElement,
 } from "../element/binding";
 import { isBindingElement } from "../element/typeChecks";
-import { isEraserActive } from "../appState";
 
 export const actionFinalize = register({
   name: "finalize",
@@ -146,7 +145,14 @@ export const actionFinalize = register({
             appState.activeTool.type === "freedraw") &&
           multiPointElement
             ? appState.activeTool
-            : { ...appState.activeTool, type: "selection" },
+            : {
+                ...appState.activeTool,
+                type:
+                  appState.activeTool.type === "eraser" &&
+                  appState.activeTool.lastActiveToolBeforeEraser
+                    ? appState.activeTool.lastActiveToolBeforeEraser
+                    : "selection",
+              },
         draggingElement: null,
         multiElement: null,
         editingElement: null,
@@ -167,12 +173,11 @@ export const actionFinalize = register({
     };
   },
   keyTest: (event, appState) =>
-    !isEraserActive(appState) &&
-    ((event.key === KEYS.ESCAPE &&
+    (event.key === KEYS.ESCAPE &&
       (appState.editingLinearElement !== null ||
         (!appState.draggingElement && appState.multiElement === null))) ||
     ((event.key === KEYS.ESCAPE || event.key === KEYS.ENTER) &&
-        appState.multiElement !== null)),
+      appState.multiElement !== null),
   PanelComponent: ({ appState, updateData, data }) => (
     <ToolButton
       type="button"

src/data/restore.ts

@@ -252,9 +252,13 @@ export const restoreAppState = (
   }
   return {
     ...nextAppState,
-    activeTool: AllowedExcalidrawActiveTools[nextAppState.activeTool.type]
-      ? nextAppState.activeTool
-      : { ...nextAppState.activeTool, type: "selection" },
+    activeTool: {
+      lastActiveToolBeforeEraser: null,
+      locked: nextAppState.activeTool.locked ?? false,
+      type: AllowedExcalidrawActiveTools[nextAppState.activeTool.type]
+        ? nextAppState.activeTool.type ?? "selection"
+        : "selection",
+    },
     // Migrates from previous version where appState.zoom was a number
     zoom:
       typeof appState.zoom === "number"

src/tests/data/restore.test.ts

@@ -310,7 +310,9 @@ describe("restoreAppState", () => {
       stubImportedAppState,
       stubLocalAppState,
     );
-    expect(restoredAppState.activeTool).toBe(stubImportedAppState.activeTool);
+    expect(restoredAppState.activeTool).toEqual(
+      stubImportedAppState.activeTool,
+    );
     expect(restoredAppState.cursorButton).toBe(
       stubImportedAppState.cursorButton,
     );