[FIX] format painter: use a single history step

Since it was transformed into a store, the paint format tool would paste the style across multiple history steps. Oops. closes #5117 Task: 4277362 Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
odoo · Oct 22, 2024 · 5953d29 · 5953d29
1 parent 8ee4266
commit 5953d29
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 14 deletions.
diff --git a/src/components/paint_format_button/paint_format_store.ts b/src/components/paint_format_button/paint_format_store.ts
@@ -7,7 +7,7 @@ import { getClipboardDataPositions } from "../../helpers/clipboard/clipboard_hel
 import { Get } from "../../store_engine";
 import { SpreadsheetStore } from "../../stores";
 import { HighlightStore } from "../../stores/highlight_store";
-import { ClipboardCell, Highlight, UID, Zone } from "../../types";
+import { ClipboardCell, Command, Highlight, UID, Zone } from "../../types";
 
 interface ClipboardContent {
   cells: ClipboardCell[][];
@@ -36,6 +36,14 @@ export class PaintFormatStore extends SpreadsheetStore {
     });
   }
 
+  protected handle(cmd: Command): void {
+    switch (cmd.type) {
+      case "PAINT_FORMAT":
+        this.paintFormat(cmd.sheetId, cmd.target);
+        break;
+    }
+  }
+
   activate(args: { persistent: boolean }) {
     this.copiedData = this.copyFormats();
     this.status = args.persistent ? "persistent" : "oneOff";
@@ -47,18 +55,7 @@ export class PaintFormatStore extends SpreadsheetStore {
   }
 
   pasteFormat(target: Zone[]) {
-    if (this.copiedData) {
-      const sheetId = this.getters.getActiveSheetId();
-      for (const handler of this.clipboardHandlers) {
-        handler.paste({ zones: target, sheetId }, this.copiedData, {
-          isCutOperation: false,
-          pasteOption: "onlyFormat",
-        });
-      }
-    }
-    if (this.status === "oneOff") {
-      this.cancel();
-    }
+    this.model.dispatch("PAINT_FORMAT", { target, sheetId: this.getters.getActiveSheetId() });
   }
 
   get isActive() {
@@ -77,6 +74,20 @@ export class PaintFormatStore extends SpreadsheetStore {
     return copiedData as ClipboardContent;
   }
 
+  private paintFormat(sheetId: UID, target: Zone[]) {
+    if (this.copiedData) {
+      for (const handler of this.clipboardHandlers) {
+        handler.paste({ zones: target, sheetId }, this.copiedData, {
+          isCutOperation: false,
+          pasteOption: "onlyFormat",
+        });
+      }
+    }
+    if (this.status === "oneOff") {
+      this.cancel();
+    }
+  }
+
   get highlights(): Highlight[] {
     const data = this.copiedData;
     if (!data) {

diff --git a/src/types/commands.ts b/src/types/commands.ts
@@ -992,6 +992,10 @@ export interface SplitPivotFormulaCommand extends PositionDependentCommand {
   pivotId: UID;
 }
 
+export interface PaintFormat extends TargetDependentCommand {
+  type: "PAINT_FORMAT";
+}
+
 export type CoreCommand =
   // /** History */
   // | SelectiveUndoCommand
@@ -1139,7 +1143,8 @@ export type LocalCommand =
   | InsertNewPivotCommand
   | DuplicatePivotInNewSheetCommand
   | InsertPivotWithTableCommand
-  | SplitPivotFormulaCommand;
+  | SplitPivotFormulaCommand
+  | PaintFormat;
 
 export type Command = CoreCommand | LocalCommand;
 

diff --git a/tests/grid/grid_component.test.ts b/tests/grid/grid_component.test.ts
@@ -46,6 +46,7 @@ import {
   setCellFormat,
   setSelection,
   setStyle,
+  undo,
   updateFilter,
   updateTableConfig,
 } from "../test_helpers/commands_helpers";
@@ -962,6 +963,23 @@ describe("Grid component", () => {
       gridMouseEvent(model, "pointerup", "D8");
       expect(getCell(model, "D8")?.style).toEqual({ bold: true });
     });
+
+    test("Paint format does a single history step", async () => {
+      selectCell(model, "B2");
+      setStyle(model, "B2", { bold: true });
+      setBorders(model, "B2", { top: DEFAULT_BORDER_DESC });
+
+      paintFormatStore.activate({ persistent: false });
+      gridMouseEvent(model, "pointerdown", "D8");
+      gridMouseEvent(model, "pointerup", "D8");
+
+      expect(getStyle(model, "D8")).toEqual({ bold: true });
+      expect(getBorder(model, "D8")).toEqual({ top: DEFAULT_BORDER_DESC });
+
+      undo(model);
+      expect(getStyle(model, "D8")).toEqual({});
+      expect(getBorder(model, "D8")).toEqual(null);
+    });
   });
 
   test("closing contextmenu focuses the grid", async () => {