Skip to content

Commit

Permalink
[FIX] FigureComponent: Undo/Redo broken after pasting figure
Browse files Browse the repository at this point in the history
Before, the Undo/Redo feature didn't work properly after pasting a figure.
This happened because the focus stayed on the pasted figure instead of
shifting to the grid or composer, where Undo/Redo commands are processed.

This commit addresses the issue by propagating the event handling to
the grid level, where the Undo/Redo actions are appropriately managed.

Task ID: 3659930

closes #3830

Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
  • Loading branch information
dhrp-odoo committed Mar 19, 2024
1 parent d86234a commit affc4b3
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/components/figures/figure/figure.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, useEffect, useRef, useState } from "@odoo/owl";
import { Component, onWillUnmount, useEffect, useRef, useState } from "@odoo/owl";
import {
ComponentsImportance,
FIGURE_BORDER_COLOR,
Expand Down Expand Up @@ -206,6 +206,10 @@ export class FigureComponent extends Component<Props, SpreadsheetChildEnv> {
},
() => [this.env.model.getters.getSelectedFigureId(), this.props.figure.id, this.figureRef.el]
);

onWillUnmount(() => {
this.props.onFigureDeleted();
});
}

clickAnchor(dirX: ResizeDirection, dirY: ResizeDirection, ev: MouseEvent) {
Expand All @@ -228,6 +232,7 @@ export class FigureComponent extends Component<Props, SpreadsheetChildEnv> {
this.props.onFigureDeleted();
ev.stopPropagation();
ev.preventDefault();
ev.stopPropagation();
break;
case "ArrowDown":
case "ArrowLeft":
Expand All @@ -248,6 +253,7 @@ export class FigureComponent extends Component<Props, SpreadsheetChildEnv> {
});
ev.stopPropagation();
ev.preventDefault();
ev.stopPropagation();
break;
}
}
Expand Down
1 change: 1 addition & 0 deletions src/plugins/ui_stateful/selection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ export class GridSelectionPlugin extends UIPlugin {
this.gridSelection.anchor.zone
);
this.setSelectionMixin(this.gridSelection.anchor, this.gridSelection.zones);
this.selectedFigureId = null;
break;
}
/** Any change to the selection has to be reflected in the selection processor. */
Expand Down
31 changes: 31 additions & 0 deletions tests/components/charts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,25 @@ import { ChartDefinition } from "../../src/types";
import { BarChartDefinition } from "../../src/types/chart/bar_chart";
import { LineChartDefinition } from "../../src/types/chart/line_chart";
import {
copy,
createChart,
createGaugeChart,
createScorecardChart,
createSheet,
paste,
setCellContent,
setStyle,
updateChart,
} from "../test_helpers/commands_helpers";
import { TEST_CHART_DATA } from "../test_helpers/constants";
import {
click,
keyDown,
setInputValueAndTrigger,
simulateClick,
triggerMouseEvent,
} from "../test_helpers/dom_helper";
import { getCellContent } from "../test_helpers/getters_helpers";
import {
mockChart,
mountSpreadsheet,
Expand Down Expand Up @@ -1115,6 +1120,32 @@ describe("charts", () => {
]);
});
});

test("Can undo multiple times after pasting figure", async () => {
setCellContent(model, "D6", "HELLO");
createTestChart("gauge");
await nextTick();
parent.env.model.dispatch("SELECT_FIGURE", { id: chartId });
await nextTick();

copy(model);
await simulateClick(".o-grid-overlay", 0, 0);
paste(model, "A1");
await nextTick();

await keyDown({ key: "Z", ctrlKey: true });
expect(model.getters.getChartIds(sheetId)).toHaveLength(1);

await keyDown({ key: "Y", ctrlKey: true });
expect(model.getters.getChartIds(sheetId)).toHaveLength(2);

await keyDown({ key: "Z", ctrlKey: true });
await keyDown({ key: "Z", ctrlKey: true });
expect(model.getters.getChartIds(sheetId)).toHaveLength(0);

await keyDown({ key: "Z", ctrlKey: true });
expect(getCellContent(model, "D6")).toEqual("");
});
});

describe("charts with multiple sheets", () => {
Expand Down
20 changes: 20 additions & 0 deletions tests/plugins/selection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,26 @@ describe("simple selection", () => {
moveAnchorCell(model, "up");
expect(model.getters.getSelectedZone()).toEqual(toZone("A1:B2"));
});

test("Selecting figure and undo cleanup selectedFigureId in selection plugin", () => {
const model = new Model();
model.dispatch("CREATE_FIGURE", {
sheetId: model.getters.getActiveSheetId(),
figure: {
id: "someuuid",
x: 10,
y: 10,
tag: "hey",
width: 100,
height: 100,
},
});
expect(model.getters.getSelectedFigureId()).toBe(null);
model.dispatch("SELECT_FIGURE", { id: "someuuid" });
expect(model.getters.getSelectedFigureId()).toBe("someuuid");
undo(model);
expect(model.getters.getSelectedFigureId()).toBe(null);
});
});

describe("multiple selections", () => {
Expand Down

0 comments on commit affc4b3

Please sign in to comment.