diff --git a/src/actions/action.ts b/src/actions/action.ts index 2969834901..f7a6842d2d 100644 --- a/src/actions/action.ts +++ b/src/actions/action.ts @@ -97,7 +97,7 @@ let nextItemId = 1; export function createAction(item: ActionSpec): Action { const name = item.name; - const children = item.children; + const children = item.children || []; const description = item.description; const icon = item.icon; const secondaryIcon = item.secondaryIcon; @@ -117,14 +117,15 @@ export function createAction(item: ActionSpec): Action { return undefined; } : undefined, - children: children - ? (env) => { - return children - .map((child) => (typeof child === "function" ? child(env) : child)) - .flat() - .map(createAction); - } - : () => [], + children: + children.length > 0 + ? (env) => { + return children + .map((child) => (typeof child === "function" ? child(env) : child)) + .flat() + .map(createAction); + } + : () => [], isReadonlyAllowed: item.isReadonlyAllowed || false, separator: item.separator || false, icon: typeof icon === "function" ? icon : () => icon || "", diff --git a/src/actions/view_actions.ts b/src/actions/view_actions.ts index f63d916fb1..0f7a14608b 100644 --- a/src/actions/view_actions.ts +++ b/src/actions/view_actions.ts @@ -131,7 +131,6 @@ export const unFreezeRows: ActionSpec = { env.model.dispatch("UNFREEZE_ROWS", { sheetId: env.model.getters.getActiveSheetId(), }), - isReadonlyAllowed: true, isVisible: (env) => !!env.model.getters.getPaneDivisions(env.model.getters.getActiveSheetId()).ySplit, }; @@ -139,13 +138,11 @@ export const unFreezeRows: ActionSpec = { export const freezeFirstRow: ActionSpec = { name: _t("1 row"), execute: (env) => interactiveFreezeColumnsRows(env, "ROW", 1), - isReadonlyAllowed: true, }; export const freezeSecondRow: ActionSpec = { name: _t("2 rows"), execute: (env) => interactiveFreezeColumnsRows(env, "ROW", 2), - isReadonlyAllowed: true, }; export const freezeCurrentRow: ActionSpec = { @@ -154,7 +151,6 @@ export const freezeCurrentRow: ActionSpec = { const { bottom } = env.model.getters.getSelectedZone(); interactiveFreezeColumnsRows(env, "ROW", bottom + 1); }, - isReadonlyAllowed: true, }; export const unFreezeCols: ActionSpec = { @@ -163,7 +159,6 @@ export const unFreezeCols: ActionSpec = { env.model.dispatch("UNFREEZE_COLUMNS", { sheetId: env.model.getters.getActiveSheetId(), }), - isReadonlyAllowed: true, isVisible: (env) => !!env.model.getters.getPaneDivisions(env.model.getters.getActiveSheetId()).xSplit, }; @@ -171,13 +166,11 @@ export const unFreezeCols: ActionSpec = { export const freezeFirstCol: ActionSpec = { name: _t("1 column"), execute: (env) => interactiveFreezeColumnsRows(env, "COL", 1), - isReadonlyAllowed: true, }; export const freezeSecondCol: ActionSpec = { name: _t("2 columns"), execute: (env) => interactiveFreezeColumnsRows(env, "COL", 2), - isReadonlyAllowed: true, }; export const freezeCurrentCol: ActionSpec = { @@ -186,7 +179,6 @@ export const freezeCurrentCol: ActionSpec = { const { right } = env.model.getters.getSelectedZone(); interactiveFreezeColumnsRows(env, "COL", right + 1); }, - isReadonlyAllowed: true, }; export const viewGridlines: ActionSpec = { diff --git a/src/components/menu/menu.css b/src/components/menu/menu.css index 441e303595..95b1744e47 100644 --- a/src/components/menu/menu.css +++ b/src/components/menu/menu.css @@ -12,15 +12,26 @@ var(--os-desktop-menu-item-height) - 2 * var(--os-menu-item-padding-vertical) ); } - &:not(.disabled) { + & { &:hover, &.o-menu-item-active { background-color: var(--os-button-active-bg); color: var(--os-button-active-text-color); } + .o-menu-item-icon { + .o-icon { + color: var(--os-icons-color); + } + } + .o-menu-item-description { + color: grey; + } } &.disabled { - color: var(--os-disabled-text-color); + &, + .o-menu-item-description { + color: var(--os-disabled-text-color); + } } .o-menu-item-name { min-width: 40%; @@ -29,12 +40,6 @@ display: inline-block; margin: 0px 8px 0px 0px; } - .o-menu-item-description { - color: grey; - } - &.disabled { - cursor: not-allowed; - } } } &.o-spreadsheet-mobile { diff --git a/src/components/menu/menu.ts b/src/components/menu/menu.ts index 6ae91ad42c..7846e3de10 100644 --- a/src/components/menu/menu.ts +++ b/src/components/menu/menu.ts @@ -118,10 +118,15 @@ export class Menu extends Component { } isEnabled(menu: Action) { - if (menu.isEnabled(this.env)) { - return this.env.model.getters.isReadonly() ? menu.isReadonlyAllowed : true; + const children = menu.children?.(this.env); + if (children.length) { + return children.some((child) => this.isEnabled(child)); + } else { + if (menu.isEnabled(this.env)) { + return this.env.model.getters.isReadonly() ? menu.isReadonlyAllowed : true; + } + return false; } - return false; } get menuStyle() { diff --git a/src/components/menu_popover/menu_popover.ts b/src/components/menu_popover/menu_popover.ts index 57c9c3d9d5..53317b3923 100644 --- a/src/components/menu_popover/menu_popover.ts +++ b/src/components/menu_popover/menu_popover.ts @@ -245,17 +245,15 @@ export class MenuPopover extends Component { } onMouseOver(menu: Action, ev: MouseEvent) { - if (this.isEnabled(menu)) { - if (this.isParentMenu(this.subMenu, menu)) { - this.openingTimeOut.clear(); - return; - } - const currentTarget = ev.currentTarget as HTMLElement; - if (this.isRoot(menu)) { - this.openingTimeOut.schedule(() => { - this.openSubMenu(menu, currentTarget); - }, TIMEOUT_DELAY); - } + if (this.isParentMenu(this.subMenu, menu)) { + this.openingTimeOut.clear(); + return; + } + const currentTarget = ev.currentTarget as HTMLElement; + if (this.isRoot(menu)) { + this.openingTimeOut.schedule(() => { + this.openSubMenu(menu, currentTarget); + }, TIMEOUT_DELAY); } } diff --git a/src/registries/menus/topbar_menu_registry.ts b/src/registries/menus/topbar_menu_registry.ts index d120ab823c..d11d449de9 100644 --- a/src/registries/menus/topbar_menu_registry.ts +++ b/src/registries/menus/topbar_menu_registry.ts @@ -22,7 +22,6 @@ topbarMenuRegistry .add("file", { name: _t("File"), sequence: 10, - isReadonlyAllowed: true, }) .addChild("settings", ["file"], { name: _t("Settings"), @@ -39,7 +38,6 @@ topbarMenuRegistry .add("edit", { name: _t("Edit"), sequence: 20, - isReadonlyAllowed: true, }) .addChild("undo", ["edit"], { ...ACTION_EDIT.undo, @@ -126,7 +124,6 @@ topbarMenuRegistry .add("view", { name: _t("View"), sequence: 30, - isReadonlyAllowed: true, }) .addChild("unfreeze_panes", ["view"], { ...ACTION_VIEW.unFreezePane, @@ -225,7 +222,6 @@ topbarMenuRegistry .add("insert", { name: _t("Insert"), sequence: 40, - isReadonlyAllowed: true, }) .addChild("insert_row", ["insert"], { ...ACTION_INSERT.insertRow, @@ -346,7 +342,6 @@ topbarMenuRegistry .add("format", { name: _t("Format"), sequence: 50, - isReadonlyAllowed: true, }) .addChild("format_number", ["format"], { ...formatNumberMenuItemSpec, @@ -441,7 +436,6 @@ topbarMenuRegistry .add("data", { name: _t("Data"), sequence: 60, - isReadonlyAllowed: true, }) .addChild("sort_range", ["data"], { ...ACTION_DATA.sortRange, diff --git a/tests/menus/context_menu_component.test.ts b/tests/menus/context_menu_component.test.ts index d0a5413be2..ee85bf8334 100644 --- a/tests/menus/context_menu_component.test.ts +++ b/tests/menus/context_menu_component.test.ts @@ -449,33 +449,51 @@ describe("Context MenuPopover internal tests", () => { expect(menuItem2?.classList).toContain("o-menu-item-active"); }); - test("submenu does not open when disabled", async () => { - const menuItems: Action[] = createActions([ - makeTestMenuItem("root", { - isEnabled: () => false, - children: [makeTestMenuItem("subMenu")], - }), - ]); - await renderContextMenu(300, 300, { menuItems }); - expect(fixture.querySelector(".o-menu div[data-name='root']")!.classList).toContain("disabled"); - await simulateClick(".o-menu div[data-name='root']"); - expect(fixture.querySelector(".o-menu div[data-name='subMenu']")).toBeFalsy(); - }); + describe("IsEnabled is ignored on parent menu items", () => { + test("submenu opens even if the parent is disabled", async () => { + const menuItems: Action[] = createActions([ + makeTestMenuItem("root", { + isEnabled: () => false, + children: [makeTestMenuItem("subMenu")], + }), + ]); + await renderContextMenu(300, 300, { menuItems }); + expect(fixture.querySelector(".o-menu div[data-name='root']")!).not.toHaveClass("disabled"); + await simulateClick(".o-menu div[data-name='root']"); + expect(fixture.querySelector(".o-menu div[data-name='subMenu']")).toBeTruthy(); + }); - test("submenu does not open when hovering write only parent", async () => { - const menuItems: Action[] = createActions([ - makeTestMenuItem("root", { - isReadonlyAllowed: false, - isEnabled: () => true, - children: [makeTestMenuItem("subMenu")], - }), - ]); - await renderContextMenu(300, 300, { menuItems }); - model.updateMode("readonly"); - await nextTick(); - expect(fixture.querySelector(".o-menu div[data-name='root']")!.classList).toContain("disabled"); - await mouseOverMenuElement(".o-menu div[data-name='root']"); - expect(fixture.querySelector(".o-menu div[data-name='subMenu']")).toBeFalsy(); + test("in readonly, submenu opens if the children are readonly and the parent write only", async () => { + const menuItems: Action[] = createActions([ + makeTestMenuItem("root", { + isReadonlyAllowed: false, + isEnabled: () => false, + children: [makeTestMenuItem("subMenu", { isReadonlyAllowed: true })], + }), + ]); + await renderContextMenu(300, 300, { menuItems }); + model.updateMode("readonly"); + await nextTick(); + expect(fixture.querySelector(".o-menu div[data-name='root']")!).not.toHaveClass("disabled"); + await mouseOverMenuElement(".o-menu div[data-name='root']"); + expect(fixture.querySelector(".o-menu div[data-name='subMenu']")).toBeTruthy(); + }); + + test("in readonly, submenu opens if the children are readonly and the parent write only", async () => { + const menuItems: Action[] = createActions([ + makeTestMenuItem("root", { + isReadonlyAllowed: false, + isEnabled: () => true, + children: [makeTestMenuItem("subMenu")], + }), + ]); + await renderContextMenu(300, 300, { menuItems }); + model.updateMode("readonly"); + await nextTick(); + expect(fixture.querySelector(".o-menu div[data-name='root']")!).toHaveClass("disabled"); + await mouseOverMenuElement(".o-menu div[data-name='root']"); + expect(fixture.querySelector(".o-menu div[data-name='subMenu']")).toBeTruthy(); + }); }); test("submenu does not close when sub item hovered", async () => { diff --git a/tests/menus/menu_component.test.ts b/tests/menus/menu_component.test.ts index 909aad860d..84b0a529c5 100644 --- a/tests/menus/menu_component.test.ts +++ b/tests/menus/menu_component.test.ts @@ -1,3 +1,4 @@ +import { Model } from "@odoo/o-spreadsheet-engine"; import { createActions } from "../../src/actions/action"; import { Menu } from "../../src/components/menu/menu"; import { simulateClick } from "../test_helpers/dom_helper"; @@ -24,4 +25,28 @@ describe("Menu component", () => { await simulateClick(selector); expect(callback).not.toHaveBeenCalled(); }); + + test("Execute is not called when menu item is ont readonly allowed", async () => { + const callback = jest.fn(); + const menuItems = createActions([ + { + id: "test_menu", + name: "Test Menu", + isEnabled: () => true, + execute: () => {}, + }, + ]); + + const selector = ".o-menu div[data-name='test_menu']"; + const model = new Model(); + model.updateMode("readonly"); + const { fixture } = await mountComponent(Menu, { + props: { menuItems, onClose: () => {}, onClickMenu: () => callback() }, + env: { model }, + }); + + expect(fixture.querySelector(selector)!.classList).toContain("disabled"); + await simulateClick(selector); + expect(callback).not.toHaveBeenCalled(); + }); });