fix: add history capture for paste and drop of images and embeds (#9605)

packages/excalidraw/components/App.tsx

@@ -3006,6 +3006,7 @@ class App extends React.Component<AppProps, AppState> {
     }
   };
 
+  // TODO: this is so spaghetti, we should refactor it and cover it with tests
   public pasteFromClipboard = withBatchedUpdates(
     async (event: ClipboardEvent) => {
       const isPlainPaste = !!IS_PLAIN_PASTE;
@@ -3070,6 +3071,7 @@ class App extends React.Component<AppProps, AppState> {
         const imageElement = this.createImageElement({ sceneX, sceneY });
         this.insertImageElement(imageElement, file);
         this.initializeImageDimensions(imageElement);
+        this.store.scheduleCapture();
         this.setState({
           selectedElementIds: makeNextSelectedElementIds(
             {
@@ -3180,6 +3182,7 @@ class App extends React.Component<AppProps, AppState> {
             }
           }
           if (embeddables.length) {
+            this.store.scheduleCapture();
             this.setState({
               selectedElementIds: Object.fromEntries(
                 embeddables.map((embeddable) => [embeddable.id, true]),
@@ -3292,11 +3295,10 @@ class App extends React.Component<AppProps, AppState> {
       this.addMissingFiles(opts.files);
     }
 
-    this.store.scheduleCapture();
-
     const nextElementsToSelect =
       excludeElementsInFramesFromSelection(duplicatedElements);
 
+    this.store.scheduleCapture();
     this.setState(
       {
         ...this.state,
@@ -3530,7 +3532,7 @@ class App extends React.Component<AppProps, AppState> {
     }
 
     this.scene.insertElements(textElements);
-
+    this.store.scheduleCapture();
     this.setState({
       selectedElementIds: makeNextSelectedElementIds(
         Object.fromEntries(textElements.map((el) => [el.id, true])),
@@ -3552,8 +3554,6 @@ class App extends React.Component<AppProps, AppState> {
       });
       PLAIN_PASTE_TOAST_SHOWN = true;
     }
-
-    this.store.scheduleCapture();
   }
 
   setAppState: React.Component<any, AppState>["setState"] = (
@@ -8978,6 +8978,7 @@ class App extends React.Component<AppProps, AppState> {
         );
 
         this.store.scheduleCapture();
+
         if (hitLockedElement?.locked) {
           this.setState({
             activeLockedId:
@@ -9947,13 +9948,9 @@ class App extends React.Component<AppProps, AppState> {
     const dataURL =
       this.files[fileId]?.dataURL || (await getDataURL(imageFile));
 
-    const imageElement = this.scene.mutateElement(
-      _imageElement,
-      {
-        fileId,
-      },
-      { informMutation: false, isDragging: false },
-    ) as NonDeleted<InitializedExcalidrawImageElement>;
+    let imageElement = newElementWith(_imageElement, {
+      fileId,
+    }) as NonDeleted<InitializedExcalidrawImageElement>;
 
     return new Promise<NonDeleted<InitializedExcalidrawImageElement>>(
       async (resolve, reject) => {
@@ -9967,20 +9964,38 @@ class App extends React.Component<AppProps, AppState> {
               lastRetrieved: Date.now(),
             },
           ]);
-          const cachedImageData = this.imageCache.get(fileId);
+
+          let cachedImageData = this.imageCache.get(fileId);
+
           if (!cachedImageData) {
             this.addNewImagesToImageCache();
-            await this.updateImageCache([imageElement]);
-          }
-          if (cachedImageData?.image instanceof Promise) {
-            await cachedImageData.image;
+
+            const { updatedFiles } = await this.updateImageCache([
+              imageElement,
+            ]);
+
+            if (updatedFiles.size) {
+              ShapeCache.delete(_imageElement);
+            }
+
+            cachedImageData = this.imageCache.get(fileId);
           }
+
+          const imageHTML = await cachedImageData?.image;
+
           if (
+            imageHTML &&
             this.state.pendingImageElementId !== imageElement.id &&
             this.state.newElement?.id !== imageElement.id
           ) {
-            this.initializeImageDimensions(imageElement, true);
+            const naturalDimensions = this.getImageNaturalDimensions(
+              imageElement,
+              imageHTML,
+            );
+
+            imageElement = newElementWith(imageElement, naturalDimensions);
           }
+
           resolve(imageElement);
         } catch (error: any) {
           console.error(error);
@@ -10012,11 +10027,30 @@ class App extends React.Component<AppProps, AppState> {
     this.scene.insertElement(imageElement);
 
     try {
-      return await this.initializeImage({
+      const image = await this.initializeImage({
         imageFile,
         imageElement,
         showCursorImagePreview,
       });
+
+      const nextElements = this.scene
+        .getElementsIncludingDeleted()
+        .map((element) => {
+          if (element.id === image.id) {
+            return image;
+          }
+
+          return element;
+        });
+
+      // schedules an immediate micro action, which will update snapshot,
+      // but won't be undoable, which is what we want!
+      this.updateScene({
+        captureUpdate: CaptureUpdateAction.NEVER,
+        elements: nextElements,
+      });
+
+      return image;
     } catch (error: any) {
       this.scene.mutateElement(imageElement, {
         isDeleted: true,
@@ -10106,6 +10140,7 @@ class App extends React.Component<AppProps, AppState> {
       if (insertOnCanvasDirectly) {
         this.insertImageElement(imageElement, imageFile);
         this.initializeImageDimensions(imageElement);
+        this.store.scheduleCapture();
         this.setState(
           {
             selectedElementIds: makeNextSelectedElementIds(
@@ -10150,20 +10185,18 @@ class App extends React.Component<AppProps, AppState> {
     }
   };
 
-  initializeImageDimensions = (
-    imageElement: ExcalidrawImageElement,
-    forceNaturalSize = false,
-  ) => {
-    const image =
+  initializeImageDimensions = (imageElement: ExcalidrawImageElement) => {
+    const imageHTML =
       isInitializedImageElement(imageElement) &&
       this.imageCache.get(imageElement.fileId)?.image;
 
-    if (!image || image instanceof Promise) {
+    if (!imageHTML || imageHTML instanceof Promise) {
       if (
         imageElement.width < DRAGGING_THRESHOLD / this.state.zoom.value &&
         imageElement.height < DRAGGING_THRESHOLD / this.state.zoom.value
       ) {
         const placeholderSize = 100 / this.state.zoom.value;
+
         this.scene.mutateElement(imageElement, {
           x: imageElement.x - placeholderSize / 2,
           y: imageElement.y - placeholderSize / 2,
@@ -10175,37 +10208,48 @@ class App extends React.Component<AppProps, AppState> {
       return;
     }
 
+    // if user-created bounding box is below threshold, assume the
+    // intention was to click instead of drag, and use the image's
+    // intrinsic size
     if (
-      forceNaturalSize ||
-      // if user-created bounding box is below threshold, assume the
-      // intention was to click instead of drag, and use the image's
-      // intrinsic size
-      (imageElement.width < DRAGGING_THRESHOLD / this.state.zoom.value &&
-        imageElement.height < DRAGGING_THRESHOLD / this.state.zoom.value)
+      imageElement.width < DRAGGING_THRESHOLD / this.state.zoom.value &&
+      imageElement.height < DRAGGING_THRESHOLD / this.state.zoom.value
     ) {
-      const minHeight = Math.max(this.state.height - 120, 160);
-      // max 65% of canvas height, clamped to <300px, vh - 120px>
-      const maxHeight = Math.min(
-        minHeight,
-        Math.floor(this.state.height * 0.5) / this.state.zoom.value,
+      const naturalDimensions = this.getImageNaturalDimensions(
+        imageElement,
+        imageHTML,
       );
 
-      const height = Math.min(image.naturalHeight, maxHeight);
-      const width = height * (image.naturalWidth / image.naturalHeight);
+      this.scene.mutateElement(imageElement, naturalDimensions);
+    }
+  };
+
+  private getImageNaturalDimensions = (
+    imageElement: ExcalidrawImageElement,
+    imageHTML: HTMLImageElement,
+  ) => {
+    const minHeight = Math.max(this.state.height - 120, 160);
+    // max 65% of canvas height, clamped to <300px, vh - 120px>
+    const maxHeight = Math.min(
+      minHeight,
+      Math.floor(this.state.height * 0.5) / this.state.zoom.value,
+    );
+
+    const height = Math.min(imageHTML.naturalHeight, maxHeight);
+    const width = height * (imageHTML.naturalWidth / imageHTML.naturalHeight);
 
-      // add current imageElement width/height to account for previous centering
-      // of the placeholder image
-      const x = imageElement.x + imageElement.width / 2 - width / 2;
-      const y = imageElement.y + imageElement.height / 2 - height / 2;
+    // add current imageElement width/height to account for previous centering
+    // of the placeholder image
+    const x = imageElement.x + imageElement.width / 2 - width / 2;
+    const y = imageElement.y + imageElement.height / 2 - height / 2;
 
-      this.scene.mutateElement(imageElement, {
-        x,
-        y,
-        width,
-        height,
-        crop: null,
-      });
-    }
+    return {
+      x,
+      y,
+      width,
+      height,
+      crop: null,
+    };
   };
 
   /** updates image cache, refreshing updated elements and/or setting status
@@ -10219,13 +10263,7 @@ class App extends React.Component<AppProps, AppState> {
       fileIds: elements.map((element) => element.fileId),
       files,
     });
-    if (updatedFiles.size || erroredFiles.size) {
-      for (const element of elements) {
-        if (updatedFiles.has(element.fileId)) {
-          ShapeCache.delete(element);
-        }
-      }
-    }
+
     if (erroredFiles.size) {
       this.scene.replaceAllElements(
         this.scene.getElementsIncludingDeleted().map((element) => {
@@ -10261,6 +10299,15 @@ class App extends React.Component<AppProps, AppState> {
         uncachedImageElements,
         files,
       );
+
+      if (updatedFiles.size) {
+        for (const element of uncachedImageElements) {
+          if (updatedFiles.has(element.fileId)) {
+            ShapeCache.delete(element);
+          }
+        }
+      }
+
       if (updatedFiles.size) {
         this.scene.triggerUpdate();
       }
@@ -10444,6 +10491,7 @@ class App extends React.Component<AppProps, AppState> {
         const imageElement = this.createImageElement({ sceneX, sceneY });
         this.insertImageElement(imageElement, file);
         this.initializeImageDimensions(imageElement);
+        this.store.scheduleCapture();
         this.setState({
           selectedElementIds: makeNextSelectedElementIds(
             { [imageElement.id]: true },
@@ -10494,6 +10542,7 @@ class App extends React.Component<AppProps, AppState> {
           link: normalizeLink(text),
         });
         if (embeddable) {
+          this.store.scheduleCapture();
           this.setState({ selectedElementIds: { [embeddable.id]: true } });
         }
       }

packages/excalidraw/tests/helpers/api.ts

@@ -499,13 +499,21 @@ export class API {
       value: {
         files,
         getData: (type: string) => {
-          if (type === blob.type) {
+          if (type === blob.type || type === "text") {
             return text;
           }
           return "";
         },
+        types: [blob.type],
       },
     });
+    Object.defineProperty(fileDropEvent, "clientX", {
+      value: 0,
+    });
+    Object.defineProperty(fileDropEvent, "clientY", {
+      value: 0,
+    });
+    
     await fireEvent(GlobalTestState.interactiveCanvas, fileDropEvent);
   };
 

packages/excalidraw/tests/helpers/mocks.ts

@@ -31,3 +31,30 @@ export const mockMermaidToExcalidraw = (opts: {
     });
   }
 };
+
+// Mock for HTMLImageElement (use with `vi.unstubAllGlobals()`)
+// as jsdom.resources: "usable" throws an error on image load
+export const mockHTMLImageElement = (
+  naturalWidth: number,
+  naturalHeight: number,
+) => {
+  vi.stubGlobal(
+    "Image",
+    class extends Image {
+      constructor() {
+        super();
+
+        Object.defineProperty(this, "naturalWidth", {
+          value: naturalWidth,
+        });
+        Object.defineProperty(this, "naturalHeight", {
+          value: naturalHeight,
+        });
+
+        queueMicrotask(() => {
+          this.onload?.({} as Event);
+        });
+      }
+    },
+  );
+};

packages/excalidraw/tests/history.test.tsx

@@ -19,6 +19,7 @@ import {
   COLOR_PALETTE,
   DEFAULT_ELEMENT_BACKGROUND_COLOR_INDEX,
   DEFAULT_ELEMENT_STROKE_COLOR_INDEX,
+  reseed,
 } from "@excalidraw/common";
 
 import "@excalidraw/utils/test-utils";
@@ -35,6 +36,7 @@ import type {
   ExcalidrawGenericElement,
   ExcalidrawLinearElement,
   ExcalidrawTextElement,
+  FileId,
   FixedPointBinding,
   FractionalIndex,
   SceneElementsMap,
@@ -49,12 +51,16 @@ import {
 } from "../actions";
 import { createUndoAction, createRedoAction } from "../actions/actionHistory";
 import { actionToggleViewMode } from "../actions/actionToggleViewMode";
+import * as StaticScene from "../renderer/staticScene";
 import { getDefaultAppState } from "../appState";
 import { Excalidraw } from "../index";
-import * as StaticScene from "../renderer/staticScene";
+import { createPasteEvent } from "../clipboard";
+
+import * as blobModule from "../data/blob";
 
 import { API } from "./helpers/api";
 import { Keyboard, Pointer, UI } from "./helpers/ui";
+import { mockHTMLImageElement } from "./helpers/mocks";
 import {
   GlobalTestState,
   act,
@@ -63,6 +69,7 @@ import {
   togglePopover,
   getCloneByOrigId,
   checkpointHistory,
+  unmountComponent,
 } from "./test-utils";
 
 import type { AppState } from "../types";
@@ -106,7 +113,22 @@ const violet = COLOR_PALETTE.violet[DEFAULT_ELEMENT_BACKGROUND_COLOR_INDEX];
 
 describe("history", () => {
   beforeEach(() => {
+    unmountComponent();
     renderStaticScene.mockClear();
+    vi.clearAllMocks();
+    vi.unstubAllGlobals();
+
+    reseed(7);
+
+    const generateIdSpy = vi.spyOn(blobModule, "generateIdFromFile");
+    const resizeFileSpy = vi.spyOn(blobModule, "resizeImageFile");
+
+    generateIdSpy.mockImplementation(() => Promise.resolve("fileId" as FileId));
+    resizeFileSpy.mockImplementation((file: File) => Promise.resolve(file));
+
+    Object.assign(document, {
+      elementFromPoint: () => GlobalTestState.canvas,
+    });
   });
 
   afterEach(() => {
@@ -559,6 +581,227 @@ describe("history", () => {
       ]);
     });
 
+    it("should create new history entry on image drag&drop", async () => {
+      await render(<Excalidraw handleKeyboardGlobally={true} />);
+
+      // it's necessary to specify the height in order to calculate natural dimensions of the image
+      h.state.height = 1000;
+
+      const deerImageDimensions = {
+        width: 318,
+        height: 335,
+      };
+
+      mockHTMLImageElement(
+        deerImageDimensions.width,
+        deerImageDimensions.height,
+      );
+
+      await API.drop(await API.loadFile("./fixtures/deer.png"));
+
+      await waitFor(() => {
+        expect(API.getUndoStack().length).toBe(1);
+        expect(API.getRedoStack().length).toBe(0);
+        expect(h.elements).toEqual([
+          expect.objectContaining({
+            type: "image",
+            fileId: expect.any(String),
+            x: expect.toBeNonNaNNumber(),
+            y: expect.toBeNonNaNNumber(),
+            ...deerImageDimensions,
+          }),
+        ]);
+      });
+
+      Keyboard.undo();
+      expect(API.getUndoStack().length).toBe(0);
+      expect(API.getRedoStack().length).toBe(1);
+      expect(h.elements).toEqual([
+        expect.objectContaining({
+          type: "image",
+          fileId: expect.any(String),
+          x: expect.toBeNonNaNNumber(),
+          y: expect.toBeNonNaNNumber(),
+          isDeleted: true,
+          ...deerImageDimensions,
+        }),
+      ]);
+
+      Keyboard.redo();
+      expect(API.getUndoStack().length).toBe(1);
+      expect(API.getRedoStack().length).toBe(0);
+      expect(h.elements).toEqual([
+        expect.objectContaining({
+          type: "image",
+          fileId: expect.any(String),
+          x: expect.toBeNonNaNNumber(),
+          y: expect.toBeNonNaNNumber(),
+          isDeleted: false,
+          ...deerImageDimensions,
+        }),
+      ]);
+    });
+
+    it("should create new history entry on embeddable link drag&drop", async () => {
+      await render(<Excalidraw handleKeyboardGlobally={true} />);
+
+      const link = "https://www.youtube.com/watch?v=gkGMXY0wekg";
+      await API.drop(
+        new Blob([link], {
+          type: MIME_TYPES.text,
+        }),
+      );
+
+      await waitFor(() => {
+        expect(API.getUndoStack().length).toBe(1);
+        expect(API.getRedoStack().length).toBe(0);
+        expect(h.elements).toEqual([
+          expect.objectContaining({
+            type: "embeddable",
+            link,
+          }),
+        ]);
+      });
+
+      Keyboard.undo();
+      expect(API.getUndoStack().length).toBe(0);
+      expect(API.getRedoStack().length).toBe(1);
+      expect(h.elements).toEqual([
+        expect.objectContaining({
+          type: "embeddable",
+          link,
+          isDeleted: true,
+        }),
+      ]);
+
+      Keyboard.redo();
+      expect(API.getUndoStack().length).toBe(1);
+      expect(API.getRedoStack().length).toBe(0);
+      expect(h.elements).toEqual([
+        expect.objectContaining({
+          type: "embeddable",
+          link,
+          isDeleted: false,
+        }),
+      ]);
+    });
+
+    it("should create new history entry on image paste", async () => {
+      await render(
+        <Excalidraw autoFocus={true} handleKeyboardGlobally={true} />,
+      );
+
+      // it's necessary to specify the height in order to calculate natural dimensions of the image
+      h.state.height = 1000;
+
+      const smileyImageDimensions = {
+        width: 56,
+        height: 77,
+      };
+
+      mockHTMLImageElement(
+        smileyImageDimensions.width,
+        smileyImageDimensions.height,
+      );
+
+      document.dispatchEvent(
+        createPasteEvent({
+          files: [await API.loadFile("./fixtures/smiley_embedded_v2.png")],
+        }),
+      );
+
+      await waitFor(() => {
+        expect(API.getUndoStack().length).toBe(1);
+        expect(API.getRedoStack().length).toBe(0);
+        expect(h.elements).toEqual([
+          expect.objectContaining({
+            type: "image",
+            fileId: expect.any(String),
+            x: expect.toBeNonNaNNumber(),
+            y: expect.toBeNonNaNNumber(),
+            ...smileyImageDimensions,
+          }),
+        ]);
+      });
+
+      Keyboard.undo();
+      expect(API.getUndoStack().length).toBe(0);
+      expect(API.getRedoStack().length).toBe(1);
+      expect(h.elements).toEqual([
+        expect.objectContaining({
+          type: "image",
+          fileId: expect.any(String),
+          x: expect.toBeNonNaNNumber(),
+          y: expect.toBeNonNaNNumber(),
+          isDeleted: true,
+          ...smileyImageDimensions,
+        }),
+      ]);
+
+      Keyboard.redo();
+      expect(API.getUndoStack().length).toBe(1);
+      expect(API.getRedoStack().length).toBe(0);
+      expect(h.elements).toEqual([
+        expect.objectContaining({
+          type: "image",
+          fileId: expect.any(String),
+          x: expect.toBeNonNaNNumber(),
+          y: expect.toBeNonNaNNumber(),
+          isDeleted: false,
+          ...smileyImageDimensions,
+        }),
+      ]);
+    });
+
+    it("should create new history entry on embeddable link paste", async () => {
+      await render(
+        <Excalidraw autoFocus={true} handleKeyboardGlobally={true} />,
+      );
+
+      const link = "https://www.youtube.com/watch?v=gkGMXY0wekg";
+
+      document.dispatchEvent(
+        createPasteEvent({
+          types: {
+            "text/plain": link,
+          },
+        }),
+      );
+
+      await waitFor(() => {
+        expect(API.getUndoStack().length).toBe(1);
+        expect(API.getRedoStack().length).toBe(0);
+        expect(h.elements).toEqual([
+          expect.objectContaining({
+            type: "embeddable",
+            link,
+          }),
+        ]);
+      });
+
+      Keyboard.undo();
+      expect(API.getUndoStack().length).toBe(0);
+      expect(API.getRedoStack().length).toBe(1);
+      expect(h.elements).toEqual([
+        expect.objectContaining({
+          type: "embeddable",
+          link,
+          isDeleted: true,
+        }),
+      ]);
+
+      Keyboard.redo();
+      expect(API.getUndoStack().length).toBe(1);
+      expect(API.getRedoStack().length).toBe(0);
+      expect(h.elements).toEqual([
+        expect.objectContaining({
+          type: "embeddable",
+          link,
+          isDeleted: false,
+        }),
+      ]);
+    });
+
     it("should support appstate name or viewBackgroundColor change", async () => {
       await render(
         <Excalidraw