Skip to content

Commit

Permalink
[FIX] figure: crash on deleting dragged figure
Browse files Browse the repository at this point in the history
There was a traceback if a figure was deleted while being dragged.

closes #5223

Task: 4329240
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
  • Loading branch information
hokolomopo authored and LucasLefevre committed Dec 6, 2024
1 parent a70d1c0 commit f535004
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 8 deletions.
26 changes: 21 additions & 5 deletions src/components/figures/figure_container/figure_container.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, onMounted, useState } from "@odoo/owl";
import { Component, onMounted, onWillUpdateProps, useState } from "@odoo/owl";
import { MIN_FIG_SIZE } from "../../../constants";
import { figureRegistry } from "../../../registries";
import {
Expand All @@ -21,6 +21,7 @@ interface DndState {
y: Pixel;
width: Pixel;
height: Pixel;
cancelDnd: (() => void) | undefined;
}
interface Props {
sidePanelIsOpen: Boolean;
Expand Down Expand Up @@ -104,6 +105,7 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
y: 0,
width: 0,
height: 0,
cancelDnd: undefined,
});

setup() {
Expand All @@ -117,14 +119,28 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
// new rendering
this.render();
});
onWillUpdateProps(() => {
const sheetId = this.env.model.getters.getActiveSheetId();
if (this.dnd.figId && !this.env.model.getters.getFigure(sheetId, this.dnd.figId)) {
if (this.dnd.cancelDnd) {
this.dnd.cancelDnd();
}
this.dnd.figId = undefined;
this.dnd.cancelDnd = undefined;
}
});
}

private getVisibleFigures(): Figure[] {
const visibleFigures = this.env.model.getters.getVisibleFigures();
if (this.dnd.figId && !visibleFigures.some((figure) => figure.id === this.dnd.figId)) {
visibleFigures.push(
this.env.model.getters.getFigure(this.env.model.getters.getActiveSheetId(), this.dnd.figId)!
const draggedFigure = this.env.model.getters.getFigure(
this.env.model.getters.getActiveSheetId(),
this.dnd.figId
);
if (draggedFigure) {
visibleFigures.push(draggedFigure);
}
}
return visibleFigures;
}
Expand Down Expand Up @@ -256,7 +272,7 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
this.dnd.figId = undefined;
this.env.model.dispatch("UPDATE_FIGURE", { sheetId, id: figure.id, x, y });
};
startDnd(onMouseMove, onMouseUp);
this.dnd.cancelDnd = startDnd(onMouseMove, onMouseUp);
}

startResize(figure: Figure, dirX: ResizeDirection, dirY: ResizeDirection, ev: MouseEvent) {
Expand Down Expand Up @@ -315,7 +331,7 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
...update,
});
};
startDnd(onMouseMove, onMouseUp);
this.dnd.cancelDnd = startDnd(onMouseMove, onMouseUp);
}

private getDndFigure(): Figure {
Expand Down
15 changes: 12 additions & 3 deletions src/components/helpers/drag_and_drop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -26,6 +33,8 @@ export function startDnd(
window.addEventListener("dragstart", _onDragStart);
window.addEventListener("mousemove", onMouseMove);
window.addEventListener("wheel", onMouseMove);

return removeListeners;
}

/**
Expand Down
9 changes: 9 additions & 0 deletions tests/components/figure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,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", 150, 100, 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 () => {
Expand Down

0 comments on commit f535004

Please sign in to comment.