diff --git a/src/helpers/misc.ts b/src/helpers/misc.ts index 735d3e9fb1..db927a4673 100644 --- a/src/helpers/misc.ts +++ b/src/helpers/misc.ts @@ -462,6 +462,11 @@ export function deepEquals(o1: T, o2: T): boolean { return true; } +/** Check if the given array contains all the values of the other array. */ +export function includesAll(arr: T[], values: T[]): boolean { + return values.every((value) => arr.includes(value)); +} + /** * Return an object with all the keys in the object that have a falsy value removed. */ diff --git a/src/plugins/core/header_visibility.ts b/src/plugins/core/header_visibility.ts index 5a34ed4191..75c9821136 100644 --- a/src/plugins/core/header_visibility.ts +++ b/src/plugins/core/header_visibility.ts @@ -1,10 +1,11 @@ -import { deepCopy, getAddHeaderStartIndex } from "../../helpers"; +import { deepCopy, getAddHeaderStartIndex, includesAll, range } from "../../helpers"; import { Command, CommandResult, ExcelWorkbookData, WorkbookData } from "../../types"; import { ConsecutiveIndexes, Dimension, HeaderIndex, UID } from "../../types/misc"; import { CorePlugin } from "../core_plugin"; export class HeaderVisibilityPlugin extends CorePlugin { static getters = [ + "canRemoveHeaders", "getHiddenColsGroups", "getHiddenRowsGroups", "isRowHiddenByUser", @@ -16,7 +17,7 @@ export class HeaderVisibilityPlugin extends CorePlugin { allowDispatch(cmd: Command) { switch (cmd.type) { case "HIDE_COLUMNS_ROWS": { - if (!this.hiddenHeaders[cmd.sheetId]) { + if (!this.getters.tryGetSheet(cmd.sheetId)) { return CommandResult.InvalidSheetId; } const hiddenGroup = @@ -31,6 +32,14 @@ export class HeaderVisibilityPlugin extends CorePlugin { ? CommandResult.Success : CommandResult.TooManyHiddenElements; } + case "REMOVE_COLUMNS_ROWS": + if (!this.getters.tryGetSheet(cmd.sheetId)) { + return CommandResult.InvalidSheetId; + } + if (!this.canRemoveHeaders(cmd.sheetId, cmd.dimension, cmd.elements)) { + return CommandResult.NotEnoughElements; + } + return CommandResult.Success; } return CommandResult.Success; } @@ -83,6 +92,11 @@ export class HeaderVisibilityPlugin extends CorePlugin { return; } + canRemoveHeaders(sheetId: UID, dimension: Dimension, elements: HeaderIndex[]): boolean { + const visibleHeaders = this.getAllVisibleHeaders(sheetId, dimension); + return !includesAll(elements, visibleHeaders); + } + isRowHiddenByUser(sheetId: UID, index: HeaderIndex): boolean { return this.hiddenHeaders[sheetId].ROW[index]; } @@ -131,6 +145,12 @@ export class HeaderVisibilityPlugin extends CorePlugin { return consecutiveIndexes; } + private getAllVisibleHeaders(sheetId: UID, dimension: Dimension): HeaderIndex[] { + return range(0, this.hiddenHeaders[sheetId][dimension].length).filter( + (i) => !this.hiddenHeaders[sheetId][dimension][i] + ); + } + import(data: WorkbookData) { for (let sheet of data.sheets) { this.hiddenHeaders[sheet.id] = { COL: [], ROW: [] }; diff --git a/src/plugins/core/sheet.ts b/src/plugins/core/sheet.ts index 998481a127..a2f4ba6d5e 100644 --- a/src/plugins/core/sheet.ts +++ b/src/plugins/core/sheet.ts @@ -118,15 +118,6 @@ export class SheetPlugin extends CorePlugin implements SheetState { return this.orderedSheetIds.length > 1 ? CommandResult.Success : CommandResult.NotEnoughSheets; - case "REMOVE_COLUMNS_ROWS": { - const length = - cmd.dimension === "COL" - ? this.getNumberCols(cmd.sheetId) - : this.getNumberRows(cmd.sheetId); - return length > cmd.elements.length - ? CommandResult.Success - : CommandResult.NotEnoughElements; - } case "FREEZE_ROWS": { return this.checkValidations( cmd, diff --git a/src/registries/menus/col_menu_registry.ts b/src/registries/menus/col_menu_registry.ts index efa6851c68..bdb8c7db40 100644 --- a/src/registries/menus/col_menu_registry.ts +++ b/src/registries/menus/col_menu_registry.ts @@ -72,6 +72,11 @@ colMenuRegistry name: ACTIONS.REMOVE_COLUMNS_NAME, sequence: 90, action: ACTIONS.REMOVE_COLUMNS_ACTION, + isVisible: (env: SpreadsheetChildEnv) => { + const sheetId = env.model.getters.getActiveSheetId(); + const selectedCols = env.model.getters.getElementsFromSelection("COL"); + return env.model.getters.canRemoveHeaders(sheetId, "COL", selectedCols); + }, }) .add("clear_column", { name: ACTIONS.DELETE_CONTENT_COLUMNS_NAME, @@ -84,11 +89,8 @@ colMenuRegistry action: ACTIONS.HIDE_COLUMNS_ACTION, isVisible: (env: SpreadsheetChildEnv) => { const sheetId = env.model.getters.getActiveSheetId(); - const hiddenCols = env.model.getters.getHiddenColsGroups(sheetId).flat(); - return ( - env.model.getters.getNumberCols(sheetId) > - hiddenCols.length + env.model.getters.getElementsFromSelection("COL").length - ); + const selectedCols = env.model.getters.getElementsFromSelection("COL"); + return env.model.getters.canRemoveHeaders(sheetId, "COL", selectedCols); }, separator: true, }) diff --git a/src/registries/menus/row_menu_registry.ts b/src/registries/menus/row_menu_registry.ts index 436f50f343..92eb350fdc 100644 --- a/src/registries/menus/row_menu_registry.ts +++ b/src/registries/menus/row_menu_registry.ts @@ -55,6 +55,11 @@ rowMenuRegistry name: ACTIONS.REMOVE_ROWS_NAME, sequence: 70, action: ACTIONS.REMOVE_ROWS_ACTION, + isVisible: (env: SpreadsheetChildEnv) => { + const sheetId = env.model.getters.getActiveSheetId(); + const selectedRows = env.model.getters.getElementsFromSelection("ROW"); + return env.model.getters.canRemoveHeaders(sheetId, "ROW", selectedRows); + }, }) .add("clear_row", { name: ACTIONS.DELETE_CONTENT_ROWS_NAME, @@ -67,11 +72,8 @@ rowMenuRegistry action: ACTIONS.HIDE_ROWS_ACTION, isVisible: (env: SpreadsheetChildEnv) => { const sheetId = env.model.getters.getActiveSheetId(); - const hiddenRows = env.model.getters.getHiddenRowsGroups(sheetId).flat(); - return ( - env.model.getters.getNumberRows(sheetId) > - hiddenRows.length + env.model.getters.getElementsFromSelection("ROW").length - ); + const selectedRows = env.model.getters.getElementsFromSelection("ROW"); + return env.model.getters.canRemoveHeaders(sheetId, "ROW", selectedRows); }, separator: true, }) diff --git a/tests/components/grid_manipulation.test.ts b/tests/components/grid_manipulation.test.ts index 3cc7903d11..13f27244b3 100644 --- a/tests/components/grid_manipulation.test.ts +++ b/tests/components/grid_manipulation.test.ts @@ -1,7 +1,14 @@ import { Spreadsheet } from "../../src"; import { DEFAULT_CELL_HEIGHT, DEFAULT_CELL_WIDTH } from "../../src/constants"; +import { zoneToXc } from "../../src/helpers"; import { Model } from "../../src/model"; -import { selectColumn, selectRow } from "../test_helpers/commands_helpers"; +import { + hideColumns, + hideRows, + selectColumn, + selectRow, + setSelection, +} from "../test_helpers/commands_helpers"; import { simulateClick, triggerMouseEvent } from "../test_helpers/dom_helper"; import { getSelectionAnchorCellXc } from "../test_helpers/getters_helpers"; import { mountSpreadsheet, nextTick, spyDispatch } from "../test_helpers/helpers"; @@ -95,6 +102,24 @@ describe("Context Menu add/remove row/col", () => { }); }); + test("cannot delete nor hide all cols with contextmenu", async () => { + setSelection(model, [zoneToXc(model.getters.getSheetZone(model.getters.getActiveSheetId()))]); + simulateContextMenu(".o-col-resizer", COLUMN_D); + await nextTick(); + expect(fixture.querySelector(".o-menu div[data-name='delete_column']")).toBeNull(); + expect(fixture.querySelector(".o-menu div[data-name='hide_columns']")).toBeNull(); + }); + + test("cannot delete nor hide all non-hidden cols with contextmenu", async () => { + const sheetZone = model.getters.getSheetZone(model.getters.getActiveSheetId()); + setSelection(model, [zoneToXc({ ...sheetZone, left: sheetZone.left + 1 })]); + hideColumns(model, ["A"]); + simulateContextMenu(".o-col-resizer", COLUMN_D); + await nextTick(); + expect(fixture.querySelector(".o-menu div[data-name='delete_column']")).toBeNull(); + expect(fixture.querySelector(".o-menu div[data-name='hide_columns']")).toBeNull(); + }); + test("can delete rows with contextmenu", async () => { simulateContextMenu(".o-row-resizer", ROW_5); await nextTick(); @@ -107,6 +132,24 @@ describe("Context Menu add/remove row/col", () => { }); }); + test("cannot delete nor hide all rows with contextmenu", async () => { + setSelection(model, [zoneToXc(model.getters.getSheetZone(model.getters.getActiveSheetId()))]); + simulateContextMenu(".o-row-resizer", ROW_5); + await nextTick(); + expect(fixture.querySelector(".o-menu div[data-name='delete_row']")).toBeNull(); + expect(fixture.querySelector(".o-menu div[data-name='hide_rows']")).toBeNull(); + }); + + test("cannot delete nor hide all non-hidden rows with contextmenu", async () => { + const sheetZone = model.getters.getSheetZone(model.getters.getActiveSheetId()); + setSelection(model, [zoneToXc({ ...sheetZone, top: sheetZone.top + 1 })]); + hideRows(model, [0]); + simulateContextMenu(".o-row-resizer", ROW_5); + await nextTick(); + expect(fixture.querySelector(".o-menu div[data-name='delete_row']")).toBeNull(); + expect(fixture.querySelector(".o-menu div[data-name='hide_rows']")).toBeNull(); + }); + test("can add before cols with contextmenu", async () => { simulateContextMenu(".o-col-resizer", COLUMN_D); await nextTick(); diff --git a/tests/plugins/grid_manipulation.test.ts b/tests/plugins/grid_manipulation.test.ts index 558d9c7ea7..c2c5b68e3a 100644 --- a/tests/plugins/grid_manipulation.test.ts +++ b/tests/plugins/grid_manipulation.test.ts @@ -135,6 +135,7 @@ describe("Clear columns", () => { expect(getBorder(model, "C2")).toEqual(border); }); test("cannot delete column in invalid sheet", () => { + model = new Model(); expect(deleteColumns(model, ["A"], "INVALID")).toBeCancelledBecause( CommandResult.InvalidSheetId ); @@ -180,6 +181,7 @@ describe("Clear rows", () => { expect(getCell(model, "C2")).toMatchObject({ style }); }); test("cannot delete row in invalid sheet", () => { + model = new Model(); expect(deleteRows(model, [0], "INVALID")).toBeCancelledBecause(CommandResult.InvalidSheetId); }); }); diff --git a/tests/plugins/sheets.test.ts b/tests/plugins/sheets.test.ts index 00f4c1d401..829042405c 100644 --- a/tests/plugins/sheets.test.ts +++ b/tests/plugins/sheets.test.ts @@ -7,9 +7,11 @@ import { createChart, createSheet, createSheetWithName, + deleteColumns, deleteRows, freezeColumns, freezeRows, + hideColumns, hideRows, hideSheet, merge, @@ -879,15 +881,25 @@ describe("sheets", () => { }); test("Cannot remove more columns/rows than there are inside the sheet", () => { - const model = new Model({ - sheets: [ - { - colNumber: 1, - rowNumber: 3, - }, - ], - }); - expect(deleteRows(model, [1, 2, 3, 4])).toBeCancelledBecause(CommandResult.NotEnoughElements); + const model = new Model({ sheets: [{ colNumber: 3, rowNumber: 3 }] }); + expect(deleteRows(model, [0, 1, 2])).toBeCancelledBecause(CommandResult.NotEnoughElements); + expect(deleteColumns(model, ["A", "B", "C"])).toBeCancelledBecause( + CommandResult.NotEnoughElements + ); + }); + + test("Cannot remove all the non-hidden columns/rows", () => { + const model = new Model({ sheets: [{ colNumber: 4, rowNumber: 4 }] }); + hideRows(model, [1, 3]); + hideColumns(model, ["B", "D"]); + + expect(deleteRows(model, [0, 2])).toBeCancelledBecause(CommandResult.NotEnoughElements); + expect(deleteRows(model, [0, 1, 2])).toBeCancelledBecause(CommandResult.NotEnoughElements); + + expect(deleteColumns(model, ["A", "C"])).toBeCancelledBecause(CommandResult.NotEnoughElements); + expect(deleteColumns(model, ["A", "B", "C"])).toBeCancelledBecause( + CommandResult.NotEnoughElements + ); }); test("Cannot have all rows/columns hidden at once", () => {