Skip to content

Commit

Permalink
[FIX] Menus: Show Insert col/row menu when full sheet selected
Browse files Browse the repository at this point in the history
Revision a7d7c0e 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 #3137

Task: 3450188
X-original-commit: 4476a51
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
Signed-off-by: Rémi Rahir (rar) <[email protected]>
  • Loading branch information
rrahir committed Nov 14, 2023
1 parent 8b58776 commit 647be67
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 26 deletions.
32 changes: 7 additions & 25 deletions src/actions/insert_actions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { functionRegistry } from "../functions";
import { isConsecutive, isDefined } from "../helpers";
import { isDefined } from "../helpers";
import { handlePasteResult } from "../helpers/ui/paste_interactive";
import { _lt } from "../translation";
import { ActionBuilder, ActionSpec } from "./action";
Expand All @@ -10,10 +10,7 @@ export const insertRow: ActionSpec = {
const number = getRowsNumber(env);
return number === 1 ? _lt("Insert row") : _lt("Insert %s rows", number.toString());
},
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",
};

Expand All @@ -23,10 +20,7 @@ export const rowInsertRowBefore: ActionSpec = {
return number === 1 ? _lt("Insert row above") : _lt("Insert %s rows above", number.toString());
},
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",
};

Expand Down Expand Up @@ -60,10 +54,7 @@ export const rowInsertRowsAfter: ActionSpec = {
const number = getRowsNumber(env);
return number === 1 ? _lt("Insert row below") : _lt("Insert %s rows below", number.toString());
},
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",
};

Expand All @@ -83,10 +74,7 @@ export const insertCol: ActionSpec = {
const number = getColumnsNumber(env);
return number === 1 ? _lt("Insert column") : _lt("Insert %s columns", number.toString());
},
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",
};

Expand All @@ -98,10 +86,7 @@ export const colInsertColsBefore: ActionSpec = {
: _lt("Insert %s columns left", number.toString());
},
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",
};

Expand Down Expand Up @@ -137,10 +122,7 @@ export const colInsertColsAfter: ActionSpec = {
: _lt("Insert %s columns right", number.toString());
},
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",
};

Expand Down
16 changes: 15 additions & 1 deletion src/actions/menu_items_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
getSmartChartDefinition,
} from "../helpers/figures/charts";
import { centerFigurePosition, getMaxFigureSize } from "../helpers/figures/figure/figure";
import { getZoneArea, numberToLetters } from "../helpers/index";
import { getZoneArea, isConsecutive, isEqual, numberToLetters } from "../helpers/index";
import { interactivePaste, interactivePasteFromOS } from "../helpers/ui/paste_interactive";
import { _lt } from "../translation";
import { ClipboardMIMEType, ClipboardPasteOptions } from "../types/clipboard";
Expand Down Expand Up @@ -456,3 +456,17 @@ export const SELECTION_CONTAINS_FILTER = (env: SpreadsheetChildEnv): boolean =>
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);
};
21 changes: 21 additions & 0 deletions tests/menu_items_registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
freezeRows,
hideColumns,
hideRows,
selectAll,
selectCell,
selectColumn,
selectRow,
Expand Down Expand Up @@ -468,6 +469,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", () => {
Expand Down Expand Up @@ -556,6 +562,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", () => {
Expand Down Expand Up @@ -644,6 +655,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", () => {
Expand Down Expand Up @@ -732,6 +748,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", () => {
Expand Down

0 comments on commit 647be67

Please sign in to comment.