[FIX] figure: crash on deleting dragged figure

There was a traceback if a figure was deleted while being dragged. closes #5328 Task: 4329240 X-original-commit: f535004 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Adrien Minne (adrm) <[email protected]>
odoo · Dec 9, 2024 · 434bbae · 434bbae
1 parent 4ec1734
commit 434bbae
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 11 deletions.
diff --git a/src/components/figures/figure_container/figure_container.ts b/src/components/figures/figure_container/figure_container.ts
@@ -1,4 +1,4 @@
-import { Component, onMounted, useState } from "@odoo/owl";
+import { Component, onMounted, onWillUpdateProps, useState } from "@odoo/owl";
 import { ComponentsImportance, MIN_FIG_SIZE } from "../../../constants";
 import { isDefined } from "../../../helpers";
 import { rectIntersection, rectUnion } from "../../../helpers/rectangle";
@@ -43,6 +43,7 @@ interface DndState {
   draggedFigure?: Figure;
   horizontalSnap?: Snap<HFigureAxisType>;
   verticalSnap?: Snap<VFigureAxisType>;
+  cancelDnd: (() => void) | undefined;
 }
 
 css/*SCSS*/ `
@@ -132,6 +133,7 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
     draggedFigure: undefined,
     horizontalSnap: undefined,
     verticalSnap: undefined,
+    cancelDnd: undefined,
   });
 
   setup() {
@@ -145,6 +147,19 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
       // new rendering
       this.render();
     });
+    onWillUpdateProps(() => {
+      const sheetId = this.env.model.getters.getActiveSheetId();
+      const draggedFigureId = this.dnd.draggedFigure?.id;
+      if (draggedFigureId && !this.env.model.getters.getFigure(sheetId, draggedFigureId)) {
+        if (this.dnd.cancelDnd) {
+          this.dnd.cancelDnd();
+        }
+        this.dnd.draggedFigure = undefined;
+        this.dnd.horizontalSnap = undefined;
+        this.dnd.verticalSnap = undefined;
+        this.dnd.cancelDnd = undefined;
+      }
+    });
   }
 
   private getVisibleFigures(): Figure[] {
@@ -153,12 +168,13 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
       this.dnd.draggedFigure &&
       !visibleFigures.some((figure) => figure.id === this.dnd.draggedFigure?.id)
     ) {
-      visibleFigures.push(
-        this.env.model.getters.getFigure(
-          this.env.model.getters.getActiveSheetId(),
-          this.dnd.draggedFigure?.id
-        )!
+      const draggedFigure = this.env.model.getters.getFigure(
+        this.env.model.getters.getActiveSheetId(),
+        this.dnd.draggedFigure?.id
       );
+      if (draggedFigure) {
+        visibleFigures.push(draggedFigure);
+      }
     }
     return visibleFigures;
   }
@@ -311,7 +327,7 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
       this.dnd.verticalSnap = undefined;
       this.env.model.dispatch("UPDATE_FIGURE", { sheetId, id: figure.id, x, y });
     };
-    startDnd(onMouseMove, onMouseUp);
+    this.dnd.cancelDnd = startDnd(onMouseMove, onMouseUp);
   }
 
   /**
@@ -379,7 +395,7 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
       this.dnd.horizontalSnap = undefined;
       this.dnd.verticalSnap = undefined;
     };
-    startDnd(onMouseMove, onMouseUp);
+    this.dnd.cancelDnd = startDnd(onMouseMove, onMouseUp);
   }
 
   private getOtherFigures(figId: UID): Figure[] {

diff --git a/src/components/helpers/drag_and_drop.ts b/src/components/helpers/drag_and_drop.ts
@@ -4,20 +4,27 @@ import { HeaderIndex } from "../../types/misc";
 import { gridOverlayPosition } from "./dom_helpers";
 type EventFn = (ev: MouseEvent) => void;
 
+/**
+ * Start listening to pointer events and apply the given callbacks.
+ *
+ * @returns A function to remove the listeners.
+ */
 export function startDnd(
   onMouseMove: EventFn,
   onMouseUp: EventFn,
   onMouseDown: EventFn = () => {}
 ) {
-  const _onMouseUp = (ev: MouseEvent) => {
-    onMouseUp(ev);
-
+  const removeListeners = () => {
     window.removeEventListener("mousedown", onMouseDown);
     window.removeEventListener("mouseup", _onMouseUp);
     window.removeEventListener("dragstart", _onDragStart);
     window.removeEventListener("mousemove", onMouseMove);
     window.removeEventListener("wheel", onMouseMove);
   };
+  const _onMouseUp = (ev: MouseEvent) => {
+    onMouseUp(ev);
+    removeListeners();
+  };
   function _onDragStart(ev: DragEvent) {
     ev.preventDefault();
   }
@@ -29,6 +36,8 @@ export function startDnd(
   // preventDefault() is not allowed in passive event handler.
   // https://chromestatus.com/feature/6662647093133312
   window.addEventListener("wheel", onMouseMove, { passive: false });
+
+  return removeListeners;
 }
 
 /**

diff --git a/tests/figures/figure_component.test.ts b/tests/figures/figure_component.test.ts
@@ -410,6 +410,15 @@ describe("figures", () => {
         );
       }
     );
+
+    test("Deleting a figure during drag and drop does not crash", async () => {
+      createFigure(model, { id: "someuuid", x: 200, y: 100 });
+      await nextTick();
+      await dragElement(".o-figure", { x: 150, y: 100 }, undefined, false);
+      model.dispatch("DELETE_FIGURE", { id: "someuuid", sheetId });
+      await nextTick();
+      expect(model.getters.getFigure(sheetId, "someuuid")).toEqual(undefined);
+    });
   });
 
   test("Cannot select/move figure in readonly mode", async () => {