[REF] plugin cell: use a new command "CLEAR_CELLS"

for performance and clarification reasons, groups the dispatch commands "CLEAR_CELL" with a single command "CLEAR_CELLS" where possible. closes #4831 Task: 4095732 X-original-commit: a6192dd Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Rémi Rahir (rar) <[email protected]>
odoo · Aug 13, 2024 · 3a2d2da · 3a2d2da
1 parent cc21880
commit 3a2d2da
Show file tree

Hide file tree

Showing 12 changed files with 133 additions and 35 deletions.
diff --git a/src/clipboard_handlers/cell_clipboard.ts b/src/clipboard_handlers/cell_clipboard.ts
@@ -178,13 +178,10 @@ export class CellClipboardHandler extends AbstractCellClipboardHandler<
    * Clear the clipped zones: remove the cells and clear the formatting
    */
   private clearClippedZones(content: ClipboardContent) {
-    for (const row of content.cells) {
-      for (const cell of row) {
-        if (cell.cell) {
-          this.dispatch("CLEAR_CELL", cell.position);
-        }
-      }
-    }
+    this.dispatch("CLEAR_CELLS", {
+      sheetId: content.sheetId,
+      target: content.zones,
+    });
     this.dispatch("CLEAR_FORMATTING", {
       sheetId: content.sheetId,
       target: content.zones,

diff --git a/src/plugins/core/cell.ts b/src/plugins/core/cell.ts
@@ -141,6 +141,10 @@ export class CellPlugin extends CorePlugin<CoreState> implements CoreState {
         });
         break;
 
+      case "CLEAR_CELLS":
+        this.clearCells(cmd.sheetId, cmd.target);
+        break;
+
       case "DELETE_CONTENT":
         this.clearZones(cmd.sheetId, cmd.target);
         break;
@@ -203,6 +207,26 @@ export class CellPlugin extends CorePlugin<CoreState> implements CoreState {
     }
   }
 
+  /**
+   * Clear the styles, the format and the content of zones
+   */
+  private clearCells(sheetId: UID, zones: Zone[]) {
+    for (const zone of zones) {
+      for (let col = zone.left; col <= zone.right; col++) {
+        for (let row = zone.top; row <= zone.bottom; row++) {
+          this.dispatch("UPDATE_CELL", {
+            sheetId: sheetId,
+            col,
+            row,
+            content: "",
+            style: null,
+            format: "",
+          });
+        }
+      }
+    }
+  }
+
   /**
    * Copy the style of the reference column/row to the new columns/rows.
    */

diff --git a/src/plugins/core/sheet.ts b/src/plugins/core/sheet.ts
@@ -870,19 +870,24 @@ export class SheetPlugin extends CorePlugin<SheetState> implements SheetState {
   }
 
   private moveCellOnColumnsDeletion(sheet: Sheet, deletedColumn: number) {
+    this.dispatch("CLEAR_CELLS", {
+      sheetId: sheet.id,
+      target: [
+        {
+          left: deletedColumn,
+          top: 0,
+          right: deletedColumn,
+          bottom: sheet.rows.length - 1,
+        },
+      ],
+    });
+
     for (let rowIndex = 0; rowIndex < sheet.rows.length; rowIndex++) {
       const row = sheet.rows[rowIndex];
       for (let i in row.cells) {
         const colIndex = Number(i);
         const cellId = row.cells[i];
         if (cellId) {
-          if (colIndex === deletedColumn) {
-            this.dispatch("CLEAR_CELL", {
-              sheetId: sheet.id,
-              col: colIndex,
-              row: rowIndex,
-            });
-          }
           if (colIndex > deletedColumn) {
             this.setNewPosition(cellId, sheet.id, colIndex - 1, rowIndex);
           }
@@ -939,22 +944,21 @@ export class SheetPlugin extends CorePlugin<SheetState> implements SheetState {
     deleteFromRow: HeaderIndex,
     deleteToRow: HeaderIndex
   ) {
+    this.dispatch("CLEAR_CELLS", {
+      sheetId: sheet.id,
+      target: [
+        {
+          left: 0,
+          top: deleteFromRow,
+          right: this.getters.getNumberCols(sheet.id),
+          bottom: deleteToRow,
+        },
+      ],
+    });
+
     const numberRows = deleteToRow - deleteFromRow + 1;
     for (let rowIndex = 0; rowIndex < sheet.rows.length; rowIndex++) {
       const row = sheet.rows[rowIndex];
-      if (rowIndex >= deleteFromRow && rowIndex <= deleteToRow) {
-        for (let i in row.cells) {
-          const colIndex = Number(i);
-          const cellId = row.cells[i];
-          if (cellId) {
-            this.dispatch("CLEAR_CELL", {
-              sheetId: sheet.id,
-              col: colIndex,
-              row: rowIndex,
-            });
-          }
-        }
-      }
       if (rowIndex > deleteToRow) {
         for (let i in row.cells) {
           const colIndex = Number(i);

diff --git a/src/plugins/ui_feature/data_cleanup.ts b/src/plugins/ui_feature/data_cleanup.ts
@@ -82,9 +82,7 @@ export class DataCleanupPlugin extends UIPlugin {
       return;
     }
 
-    for (const { col, row } of positions(zone)) {
-      this.dispatch("CLEAR_CELL", { col, row, sheetId });
-    }
+    this.dispatch("CLEAR_CELLS", { target: [zone], sheetId });
 
     const zonePasted: Zone = {
       left: zone.left,

diff --git a/src/plugins/ui_stateful/clipboard.ts b/src/plugins/ui_stateful/clipboard.ts
@@ -3,7 +3,7 @@ import { ClipboardHandler } from "../../clipboard_handlers/abstract_clipboard_ha
 import { cellStyleToCss, cssPropertiesToCss } from "../../components/helpers";
 import { SELECTION_BORDER_COLOR } from "../../constants";
 import { getClipboardDataPositions } from "../../helpers/clipboard/clipboard_helpers";
-import { isZoneValid, positions, union } from "../../helpers/index";
+import { isZoneValid, union } from "../../helpers/index";
 import {
   ClipboardContent,
   ClipboardData,
@@ -198,9 +198,10 @@ export class ClipboardPlugin extends UIPlugin {
       case "DELETE_CELL": {
         const { cut, paste } = this.getDeleteCellsTargets(cmd.zone, cmd.shiftDimension);
         if (!isZoneValid(cut[0])) {
-          for (const { col, row } of positions(cmd.zone)) {
-            this.dispatch("CLEAR_CELL", { col, row, sheetId: this.getters.getActiveSheetId() });
-          }
+          this.dispatch("CLEAR_CELLS", {
+            target: [cmd.zone],
+            sheetId: this.getters.getActiveSheetId(),
+          });
           break;
         }
         const copiedData = this.copy(cut);

diff --git a/src/registries/repeat_commands_registry.ts b/src/registries/repeat_commands_registry.ts
@@ -40,6 +40,7 @@ export const repeatCommandTransformRegistry = new Registry<RepeatTransform>();
 
 repeatCommandTransformRegistry.add("UPDATE_CELL", genericRepeat);
 repeatCommandTransformRegistry.add("CLEAR_CELL", genericRepeat);
+repeatCommandTransformRegistry.add("CLEAR_CELLS", genericRepeat);
 repeatCommandTransformRegistry.add("DELETE_CONTENT", genericRepeat);
 
 repeatCommandTransformRegistry.add("ADD_MERGE", genericRepeat);

diff --git a/src/types/commands.ts b/src/types/commands.ts
@@ -150,6 +150,7 @@ export const coreTypes = new Set<CoreCommandTypes>([
   "UPDATE_CELL",
   "UPDATE_CELL_POSITION",
   "CLEAR_CELL",
+  "CLEAR_CELLS",
   "DELETE_CONTENT",
 
   /** GRID SHAPE */
@@ -719,6 +720,10 @@ export interface ClearCellCommand extends PositionDependentCommand {
   type: "CLEAR_CELL";
 }
 
+export interface ClearCellsCommand extends TargetDependentCommand {
+  type: "CLEAR_CELLS";
+}
+
 export interface UndoCommand {
   type: "UNDO";
   commands: readonly CoreCommand[];
@@ -860,6 +865,7 @@ export type CoreCommand =
   | UpdateCellCommand
   | UpdateCellPositionCommand
   | ClearCellCommand
+  | ClearCellsCommand
   | DeleteContentCommand
 
   /** GRID SHAPE */

diff --git a/tests/cells/cell_plugin.test.ts b/tests/cells/cell_plugin.test.ts
@@ -7,6 +7,7 @@ import {
   addColumns,
   addRows,
   clearCell,
+  clearCells,
   copy,
   createSheet,
   deleteColumns,
@@ -160,20 +161,46 @@ describe("getCellText", () => {
     expect(getCell(model, "A1")).toBeUndefined();
   });
 
-  test("clear style", () => {
+  test("clear some content", () => {
+    const model = new Model();
+    setCellContent(model, "A1", "hello");
+    setCellContent(model, "A2", "there");
+    clearCells(model, ["A1:A2"]);
+    expect(getCell(model, "A1")).toBeUndefined();
+    expect(getCell(model, "A2")).toBeUndefined();
+  });
+
+  test("clear some style", () => {
     const model = new Model();
     setStyle(model, "A1", { bold: true });
     clearCell(model, "A1");
     expect(getCell(model, "A1")).toBeUndefined();
   });
 
+  test("clear some style", () => {
+    const model = new Model();
+    setStyle(model, "A1", { bold: true });
+    setStyle(model, "A2", { italic: true });
+    clearCells(model, ["A1:A2"]);
+    expect(getCell(model, "A1")).toBeUndefined();
+    expect(getCell(model, "A2")).toBeUndefined();
+  });
+
   test("clear format", () => {
     const model = new Model();
     setCellFormat(model, "A1", "#,##0.0");
     clearCell(model, "A1");
     expect(getCell(model, "A1")).toBeUndefined();
   });
 
+  test("clear some format", () => {
+    const model = new Model();
+    setCellFormat(model, "A1", "#,##0.0");
+    setCellFormat(model, "A2", "0%");
+    clearCells(model, ["A1:A2"]);
+    expect(getCell(model, "A1")).toBeUndefined();
+  });
+
   test("clear content, style and format", () => {
     const model = new Model();
     setCellContent(model, "A1", "hello");
@@ -183,6 +210,19 @@ describe("getCellText", () => {
     expect(getCell(model, "A1")).toBeUndefined();
   });
 
+  test("clear some content, style and format", () => {
+    const model = new Model();
+    setCellContent(model, "A1", "hello");
+    setCellContent(model, "A2", "there");
+    setStyle(model, "A1", { bold: true });
+    setStyle(model, "A2", { italic: true });
+    setCellFormat(model, "A1", "#,##0.0");
+    setCellFormat(model, "A2", "0%");
+    clearCells(model, ["A1", "A2"]);
+    expect(getCell(model, "A1")).toBeUndefined();
+    expect(getCell(model, "A2")).toBeUndefined();
+  });
+
   test("clear cell outside of sheet", () => {
     const model = new Model();
     const result = clearCell(model, "AAA999");

diff --git a/tests/collaborative/inverses.test.ts b/tests/collaborative/inverses.test.ts
@@ -4,6 +4,7 @@ import {
   AddColumnsRowsCommand,
   AddMergeCommand,
   ClearCellCommand,
+  ClearCellsCommand,
   ClearFormattingCommand,
   CoreCommand,
   CreateSheetCommand,
@@ -249,6 +250,11 @@ describe("Inverses commands", () => {
       col: 1,
       row: 1,
     };
+    const clearCells: ClearCellsCommand = {
+      type: "CLEAR_CELLS",
+      sheetId: "1",
+      target: [toZone("A1")],
+    };
     const deleteContent: DeleteContentCommand = {
       type: "DELETE_CONTENT",
       sheetId: "1",
@@ -301,6 +307,7 @@ describe("Inverses commands", () => {
       updateCell,
       updateCellPosition,
       clearCell,
+      clearCells,
       deleteContent,
       resizeColumns,
       resizeRows,

diff --git a/tests/repeat_commands_plugin.test.ts b/tests/repeat_commands_plugin.test.ts
@@ -59,6 +59,7 @@ describe("Repeat commands basics", () => {
     const repeatableCommands = [
       "UPDATE_CELL",
       "CLEAR_CELL",
+      "CLEAR_CELLS",
       "DELETE_CONTENT",
       "ADD_MERGE",
       "REMOVE_MERGE",
@@ -135,6 +136,7 @@ describe("Repeat command transform generics", () => {
   test.each([
     TEST_COMMANDS.UPDATE_CELL,
     TEST_COMMANDS.CLEAR_CELL,
+    TEST_COMMANDS.CLEAR_CELLS,
     TEST_COMMANDS.DELETE_CONTENT,
     TEST_COMMANDS.ADD_MERGE,
     TEST_COMMANDS.REMOVE_MERGE,
@@ -166,6 +168,7 @@ describe("Repeat command transform generics", () => {
     TEST_COMMANDS.REMOVE_TABLE,
     TEST_COMMANDS.SET_FORMATTING,
     TEST_COMMANDS.CLEAR_FORMATTING,
+    TEST_COMMANDS.CLEAR_CELLS,
   ])(
     "Target dependant commands have target equal to current current selection",
     (command: CoreCommand) => {

diff --git a/tests/test_helpers/commands_helpers.ts b/tests/test_helpers/commands_helpers.ts
@@ -561,6 +561,17 @@ export function clearCell(
   return model.dispatch("CLEAR_CELL", { col, row, sheetId });
 }
 
+/**
+ * Clear cells in zones
+ */
+export function clearCells(
+  model: Model,
+  xcs: string[],
+  sheetId: UID = model.getters.getActiveSheetId()
+) {
+  return model.dispatch("CLEAR_CELLS", { target: xcs.map(toZone), sheetId });
+}
+
 /**
  * Set the content of a cell
  */

diff --git a/tests/test_helpers/constants.ts b/tests/test_helpers/constants.ts
@@ -71,6 +71,11 @@ export const TEST_COMMANDS: CommandMapping = {
     row: 0,
     sheetId: "sheetId",
   },
+  CLEAR_CELLS: {
+    type: "CLEAR_CELLS",
+    target: target("A1"),
+    sheetId: "sheetId",
+  },
   DELETE_CONTENT: {
     type: "DELETE_CONTENT",
     target: target("A1"),
@@ -380,6 +385,7 @@ export const OT_TESTS_TARGET_DEPENDANT_COMMANDS = [
   TEST_COMMANDS.SET_FORMATTING,
   TEST_COMMANDS.CLEAR_FORMATTING,
   TEST_COMMANDS.REMOVE_TABLE,
+  TEST_COMMANDS.CLEAR_CELLS,
 ];
 
 export const OT_TESTS_ZONE_DEPENDANT_COMMANDS = [