From 0bc4dc9e1192d136c3252615142cbab1187d6dc9 Mon Sep 17 00:00:00 2001 From: Alexis Lacroix Date: Mon, 17 Jun 2024 14:17:45 +0000 Subject: [PATCH] [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. Task: 4095732 X-original-commit: a6192ddec55c64a556af85fc845916ce5d0a87fd --- src/clipboard_handlers/cell_clipboard.ts | 11 ++---- src/plugins/core/cell.ts | 24 ++++++++++++ src/plugins/core/sheet.ts | 44 ++++++++++++---------- src/plugins/ui_feature/data_cleanup.ts | 4 +- src/plugins/ui_stateful/clipboard.ts | 9 +++-- src/registries/repeat_commands_registry.ts | 1 + src/types/commands.ts | 6 +++ tests/cells/cell_plugin.test.ts | 42 ++++++++++++++++++++- tests/collaborative/inverses.test.ts | 7 ++++ tests/repeat_commands_plugin.test.ts | 3 ++ tests/test_helpers/commands_helpers.ts | 11 ++++++ tests/test_helpers/constants.ts | 6 +++ 12 files changed, 133 insertions(+), 35 deletions(-) diff --git a/src/clipboard_handlers/cell_clipboard.ts b/src/clipboard_handlers/cell_clipboard.ts index 7dd08c53fc..90f71b2368 100644 --- 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 index 8f754834f5..7b6921ba85 100644 --- a/src/plugins/core/cell.ts +++ b/src/plugins/core/cell.ts @@ -141,6 +141,10 @@ export class CellPlugin extends CorePlugin 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 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 index c7635501fa..d170eec6fe 100644 --- a/src/plugins/core/sheet.ts +++ b/src/plugins/core/sheet.ts @@ -870,19 +870,24 @@ export class SheetPlugin extends CorePlugin 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 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 index 12a92cf0a9..1ef6616cd4 100644 --- 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 index afdd04e4de..d8f04b2175 100644 --- 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 index 3351c38c0d..8b0e29591d 100644 --- a/src/registries/repeat_commands_registry.ts +++ b/src/registries/repeat_commands_registry.ts @@ -40,6 +40,7 @@ export const repeatCommandTransformRegistry = new Registry(); 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 index 2c33f7f22d..ca561d4182 100644 --- a/src/types/commands.ts +++ b/src/types/commands.ts @@ -150,6 +150,7 @@ export const coreTypes = new Set([ "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 index 9042c66252..b82c8a516c 100644 --- 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,13 +161,31 @@ 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"); @@ -174,6 +193,14 @@ describe("getCellText", () => { 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 index 4c6381f0d9..72f3a21d2a 100644 --- 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 index 387af0c48f..18e2eb61bc 100644 --- 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 index 4bdc7a3d58..2f41fc9dae 100644 --- 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 index 9aa105be01..2dd9ab27d4 100644 --- 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 = [