From 4476a5109a97bfe13363455464efac57de80ca07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Rahir=20=28rar=29?= Date: Wed, 25 Oct 2023 15:54:27 +0200 Subject: [PATCH] [FIX] Menus: Show Insert col/row menu when full sheet selected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revision a7d7c0e5 was too drastic in the visibility limitation of some menu items. Specifically, a usr should be able to add rows or columns when they select the entire sheet grid. closes odoo/o-spreadsheet#3082 Task: 3450188 Signed-off-by: Lucas Lefèvre (lul) --- src/actions/insert_actions.ts | 32 +++++++------------------------ src/actions/menu_items_actions.ts | 22 ++++++++++++++++++++- tests/menu_items_registry.test.ts | 21 ++++++++++++++++++++ 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/src/actions/insert_actions.ts b/src/actions/insert_actions.ts index fb38dacd11..a0b011da9b 100644 --- a/src/actions/insert_actions.ts +++ b/src/actions/insert_actions.ts @@ -1,25 +1,19 @@ import { functionRegistry } from "../functions"; -import { isConsecutive, isDefined } from "../helpers"; +import { isDefined } from "../helpers"; import { _lt } from "../translation"; import { ActionBuilder, ActionSpec } from "./action"; import * as ACTIONS from "./menu_items_actions"; export const insertRow: ActionSpec = { name: ACTIONS.MENU_INSERT_ROWS_NAME, - isVisible: (env) => - isConsecutive(env.model.getters.getActiveRows()) && - ACTIONS.IS_ONLY_ONE_RANGE(env) && - env.model.getters.getActiveCols().size === 0, + isVisible: (env) => ACTIONS.CAN_INSERT_HEADER(env, "ROW"), icon: "o-spreadsheet-Icon.INSERT_ROW", }; export const rowInsertRowBefore: ActionSpec = { name: ACTIONS.ROW_INSERT_ROWS_BEFORE_NAME, execute: ACTIONS.INSERT_ROWS_BEFORE_ACTION, - isVisible: (env) => - isConsecutive(env.model.getters.getActiveRows()) && - ACTIONS.IS_ONLY_ONE_RANGE(env) && - env.model.getters.getActiveCols().size === 0, + isVisible: (env) => ACTIONS.CAN_INSERT_HEADER(env, "ROW"), icon: "o-spreadsheet-Icon.INSERT_ROW_BEFORE", }; @@ -38,10 +32,7 @@ export const cellInsertRowsBefore: ActionSpec = { export const rowInsertRowsAfter: ActionSpec = { execute: ACTIONS.INSERT_ROWS_AFTER_ACTION, name: ACTIONS.ROW_INSERT_ROWS_AFTER_NAME, - isVisible: (env) => - isConsecutive(env.model.getters.getActiveRows()) && - ACTIONS.IS_ONLY_ONE_RANGE(env) && - env.model.getters.getActiveCols().size === 0, + isVisible: (env) => ACTIONS.CAN_INSERT_HEADER(env, "ROW"), icon: "o-spreadsheet-Icon.INSERT_ROW_AFTER", }; @@ -52,20 +43,14 @@ export const topBarInsertRowsAfter: ActionSpec = { export const insertCol: ActionSpec = { name: ACTIONS.MENU_INSERT_COLUMNS_NAME, - isVisible: (env) => - isConsecutive(env.model.getters.getActiveCols()) && - ACTIONS.IS_ONLY_ONE_RANGE(env) && - env.model.getters.getActiveRows().size === 0, + isVisible: (env) => ACTIONS.CAN_INSERT_HEADER(env, "COL"), icon: "o-spreadsheet-Icon.INSERT_COL", }; export const colInsertColsBefore: ActionSpec = { name: ACTIONS.COLUMN_INSERT_COLUMNS_BEFORE_NAME, execute: ACTIONS.INSERT_COLUMNS_BEFORE_ACTION, - isVisible: (env) => - isConsecutive(env.model.getters.getActiveCols()) && - ACTIONS.IS_ONLY_ONE_RANGE(env) && - env.model.getters.getActiveRows().size === 0, + isVisible: (env) => ACTIONS.CAN_INSERT_HEADER(env, "COL"), icon: "o-spreadsheet-Icon.INSERT_COL_BEFORE", }; @@ -84,10 +69,7 @@ export const cellInsertColsBefore: ActionSpec = { export const colInsertColsAfter: ActionSpec = { name: ACTIONS.COLUMN_INSERT_COLUMNS_AFTER_NAME, execute: ACTIONS.INSERT_COLUMNS_AFTER_ACTION, - isVisible: (env) => - isConsecutive(env.model.getters.getActiveCols()) && - ACTIONS.IS_ONLY_ONE_RANGE(env) && - env.model.getters.getActiveRows().size === 0, + isVisible: (env) => ACTIONS.CAN_INSERT_HEADER(env, "COL"), icon: "o-spreadsheet-Icon.INSERT_COL_AFTER", }; diff --git a/src/actions/menu_items_actions.ts b/src/actions/menu_items_actions.ts index a29d58c786..d2304810ff 100644 --- a/src/actions/menu_items_actions.ts +++ b/src/actions/menu_items_actions.ts @@ -4,7 +4,13 @@ import { getSmartChartDefinition, } from "../helpers/figures/charts"; import { centerFigurePosition, getMaxFigureSize } from "../helpers/figures/figure/figure"; -import { areZonesContinuous, getZoneArea, numberToLetters } from "../helpers/index"; +import { + areZonesContinuous, + getZoneArea, + isConsecutive, + isEqual, + numberToLetters, +} from "../helpers/index"; import { interactiveSortSelection } from "../helpers/sort"; import { interactiveCut } from "../helpers/ui/cut_interactive"; import { interactiveAddFilter } from "../helpers/ui/filter_interactive"; @@ -766,3 +772,17 @@ export const SORT_CELLS_DESCENDING = (env: SpreadsheetChildEnv) => { export const IS_ONLY_ONE_RANGE = (env: SpreadsheetChildEnv): boolean => { return env.model.getters.getSelectedZones().length === 1; }; + +export const CAN_INSERT_HEADER = (env: SpreadsheetChildEnv, dimension: Dimension): boolean => { + if (!IS_ONLY_ONE_RANGE(env)) { + return false; + } + const activeHeaders = + dimension === "COL" ? env.model.getters.getActiveCols() : env.model.getters.getActiveRows(); + const ortogonalActiveHeaders = + dimension === "COL" ? env.model.getters.getActiveRows() : env.model.getters.getActiveCols(); + const sheetId = env.model.getters.getActiveSheetId(); + const zone = env.model.getters.getSelectedZone(); + const allSheetSelected = isEqual(zone, env.model.getters.getSheetZone(sheetId)); + return isConsecutive(activeHeaders) && (ortogonalActiveHeaders.size === 0 || allSheetSelected); +}; diff --git a/tests/menu_items_registry.test.ts b/tests/menu_items_registry.test.ts index 9b6ef320cc..311b8ff546 100644 --- a/tests/menu_items_registry.test.ts +++ b/tests/menu_items_registry.test.ts @@ -12,6 +12,7 @@ import { freezeRows, hideColumns, hideRows, + selectAll, selectCell, selectColumn, selectRow, @@ -415,6 +416,11 @@ describe("Menu Item actions", () => { selectRow(model, 6, "newAnchor"); expect(getNode(addRowBeforePath, rowMenuRegistry).isVisible(env)).toBeFalsy(); }); + + test("Full sheet selected", () => { + selectAll(model); + expect(getNode(addRowBeforePath, rowMenuRegistry).isVisible(env)).toBeTruthy(); + }); }); describe("Insert -> Row below", () => { @@ -503,6 +509,11 @@ describe("Menu Item actions", () => { selectRow(model, 6, "newAnchor"); expect(getNode(addRowAfterPath, rowMenuRegistry).isVisible(env)).toBeFalsy(); }); + + test("Full sheet selected", () => { + selectAll(model); + expect(getNode(addRowAfterPath, rowMenuRegistry).isVisible(env)).toBeTruthy(); + }); }); describe("Insert -> Column left", () => { @@ -591,6 +602,11 @@ describe("Menu Item actions", () => { selectColumn(model, 6, "newAnchor"); expect(getNode(addColBeforePath, colMenuRegistry).isVisible(env)).toBeFalsy(); }); + + test("Full sheet selected", () => { + selectAll(model); + expect(getNode(addColBeforePath, colMenuRegistry).isVisible(env)).toBeTruthy(); + }); }); describe("Insert -> Column right", () => { @@ -679,6 +695,11 @@ describe("Menu Item actions", () => { selectColumn(model, 6, "newAnchor"); expect(getNode(addColAfterPath, colMenuRegistry).isVisible(env)).toBeFalsy(); }); + + test("Full sheet selected", () => { + selectAll(model); + expect(getNode(addColAfterPath, colMenuRegistry).isVisible(env)).toBeTruthy(); + }); }); describe("Insert -> Insert cells and shift down", () => {