From 5868ccd2e028ed9188ff6b54a5e5fa32f5c94b5f Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Tue, 15 Oct 2024 14:52:16 +0300 Subject: [PATCH 01/22] feat(ui5-menu): selectable menu items --- packages/main/src/Menu.ts | 83 +++++++++++++-- packages/main/src/MenuItem.hbs | 30 ++++-- packages/main/src/MenuItem.ts | 100 +++++++++++++++++- packages/main/src/MenuItemGroup.hbs | 1 + packages/main/src/MenuItemGroup.ts | 105 +++++++++++++++++++ packages/main/src/MenuSeparator.ts | 4 + packages/main/src/bundle.esm.ts | 1 + packages/main/src/themes/MenuItem.css | 12 +++ packages/main/src/types/ItemSelectionMode.ts | 25 +++++ packages/main/test/pages/Menu.html | 48 ++++++++- 10 files changed, 390 insertions(+), 19 deletions(-) create mode 100644 packages/main/src/MenuItemGroup.hbs create mode 100644 packages/main/src/MenuItemGroup.ts create mode 100644 packages/main/src/types/ItemSelectionMode.ts diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index ebf71738a395..2fa1fa9a5eea 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -25,6 +25,8 @@ import List from "./List.js"; import BusyIndicator from "./BusyIndicator.js"; import MenuItem from "./MenuItem.js"; import MenuSeparator from "./MenuSeparator.js"; +import type MenuItemGroup from "./MenuItemGroup.js"; +import ItemSelectionMode from "./types/ItemSelectionMode.js"; import type { ListItemClickEventDetail, } from "./List.js"; @@ -47,6 +49,7 @@ const MENU_OPEN_DELAY = 300; */ interface IMenuItem extends UI5Element { isSeparator: boolean; + isGroup: boolean; } type MenuItemClickEventDetail = { @@ -234,6 +237,9 @@ class Menu extends UI5Element { @property({ converter: DOMReferenceConverter }) opener?: HTMLElement | string; + @property({ type: Object }) + _parent?: MenuItem | Menu; + /** * Defines the items of this component. * @@ -266,8 +272,51 @@ class Menu extends UI5Element { return this.shadowRoot!.querySelector("[ui5-responsive-popover]")!; } + get _list() { + return this.shadowRoot!.querySelector("[ui5-list]")!; + } + + get _menuItemGroups() { + return this.items.filter((item) : item is MenuItemGroup => item.isGroup); + } + + // Return only the menu items, excluding separators and items that belong to groups get _menuItems() { - return this.items.filter((item): item is MenuItem => !item.isSeparator); + return this.items.filter((item) : item is MenuItem => !item.isSeparator && !item.isGroup); + } + + get _menuItemsAll() { + const items: MenuItem[] = []; + + this.items.forEach(item => { + if (item.isGroup) { + items.push(...(item as MenuItemGroup)._menuItems); + } else if (!item.isSeparator) { + items.push(item as MenuItem); + } + }); + + return items; + } + + get _menuItemsNavigation() { + const parent = this._parent || this; + + const items: MenuItem[] = []; + const slottedItems = parent.getSlottedNodes("items"); + + slottedItems.forEach(item => { + if (item.isGroup) { + const groupItems = item.getSlottedNodes("items"); + items.push(...groupItems); + } else if (!item.isSeparator) { + items.push(item); + } + }); + + this._parent = undefined; + + return items; } get acessibleNameText() { @@ -275,13 +324,21 @@ class Menu extends UI5Element { } onBeforeRendering() { - const siblingsWithIcon = this._menuItems.some(menuItem => !!menuItem.icon); + const siblingsWithIcon = this._menuItemsAll.some(menuItem => !!menuItem.icon); + + this._setupItemNavigation(); - this._menuItems.forEach(item => { + this._menuItemsAll.forEach(item => { item._siblingsWithIcon = siblingsWithIcon; }); } + _setupItemNavigation() { + if (this._list) { + this._list._itemNavigation._getItems = () => this._menuItemsNavigation; + } + } + _close() { this.open = false; } @@ -296,6 +353,9 @@ class Menu extends UI5Element { this.fireEvent("before-open", { item, }, false, false); + + this._parent = item; + item._setupItemNavigation(); item._popover.opener = item; item._popover.open = true; item.selected = true; @@ -303,7 +363,7 @@ class Menu extends UI5Element { _closeItemSubMenu(item: MenuItem) { if (item && item._popover) { - const openedSibling = item._menuItems.find(menuItem => menuItem._popover && menuItem._popover.open); + const openedSibling = item._menuItemsAll.find(menuItem => menuItem._popover && menuItem._popover.open); if (openedSibling) { this._closeItemSubMenu(openedSibling); } @@ -332,7 +392,8 @@ class Menu extends UI5Element { this._timeout = setTimeout(() => { const opener = item.parentElement as MenuItem | Menu; - const openedSibling = opener && opener._menuItems.find(menuItem => menuItem._popover && menuItem._popover.open); + const menuItems = opener._menuItemsAll; + const openedSibling = opener && menuItems && menuItems.find(menuItem => menuItem._popover && menuItem._popover.open); if (openedSibling) { this._closeItemSubMenu(openedSibling); } @@ -343,6 +404,16 @@ class Menu extends UI5Element { _itemClick(e: CustomEvent) { const item = e.detail.item as MenuItem; + const prevSelected = item.isSelected; + const parent = item.parentElement as MenuItem | Menu | MenuItemGroup; + + if (parent && "isGroup" in parent && parent.isGroup) { + const itemSelectionMode = (parent as MenuItemGroup).itemSelectionMode; + if (itemSelectionMode === ItemSelectionMode.SingleSelect) { + (parent as MenuItemGroup)._clearSelectedItems(); + } + item.isSelected = !prevSelected; + } if (!item._popover) { const prevented = !this.fireEvent("item-click", { @@ -390,7 +461,7 @@ class Menu extends UI5Element { } _afterPopoverOpen() { - this._menuItems[0]?.focus(); + this._menuItemsAll[0]?.focus(); this.fireEvent("open", {}, false, true); } diff --git a/packages/main/src/MenuItem.hbs b/packages/main/src/MenuItem.hbs index 012631ac8e74..7d607411389e 100644 --- a/packages/main/src/MenuItem.hbs +++ b/packages/main/src/MenuItem.hbs @@ -20,14 +20,28 @@ > - {{else if hasEndContent}} - - {{else if additionalText}} - {{additionalText}} + {{else}} + {{#if hasEndContent}} + + {{else if additionalText}} + {{additionalText}} + {{/if}} + {{#if _markSelected}} +
+ + +
+ {{/if}} {{/if}} {{/inline}} diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index f2746f016903..9cacfd97d575 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -21,6 +21,8 @@ import { } from "./generated/i18n/i18n-defaults.js"; import type { ResponsivePopoverBeforeCloseEventDetail } from "./ResponsivePopover.js"; import type { IMenuItem } from "./Menu.js"; +import type MenuItemGroup from "./MenuItemGroup.js"; +import ItemSelectionMode from "./types/ItemSelectionMode.js"; // Styles import menuItemCss from "./generated/themes/MenuItem.css.js"; @@ -100,6 +102,17 @@ class MenuItem extends ListItem implements IMenuItem { @property() icon?: string; + /** + * Defines whether `ui5-menu-item` is in selected state. + * + * **Note:** selected state is only taken into account when `ui5-menu-item` is added to `ui5-menu-item-group` with `itemSelectionMode` other than `None`. + * **Note:** A selected `ui5-menu-item` have selection mark displayed ad its end. + * @default false + * @public + */ + @property({ type: Boolean }) + isSelected = false; + /** * Defines whether `ui5-menu-item` is in disabled state. * @@ -169,6 +182,12 @@ class MenuItem extends ListItem implements IMenuItem { @property({ type: Boolean, noAttribute: true }) _siblingsWithIcon = false; + /** + * Keeps the previous selected state of the item. + */ + @property({ noAttribute: true }) + _prevSelected?: boolean; + /** * Defines the items of this component. * @@ -253,12 +272,68 @@ class MenuItem extends ListItem implements IMenuItem { return false; } + get isGroup(): boolean { + return false; + } + + get _parentGroup() { + const parent = this.parentElement; + return parent && "isGroup" in parent && parent.isGroup ? parent as MenuItemGroup : null; + } + + get _parentItemSelectionMode() { + return this._parentGroup ? this._parentGroup?.itemSelectionMode : ItemSelectionMode.None; + } + + get _list() { + return this.shadowRoot!.querySelector("[ui5-list]")!; + } + + get _menuItemsNavigation() { + const items: MenuItem[] = []; + const slottedItems = this.getSlottedNodes("items"); + + slottedItems.forEach(item => { + if (item.isGroup) { + const groupItems = item.getSlottedNodes("items"); + items.push(...groupItems); + } else if (!item.isSeparator) { + items.push(item); + } + }); + + return items; + } + + get _markSelected() { + return this.isSelected && this._parentItemSelectionMode !== ItemSelectionMode.None; + } + onBeforeRendering() { - const siblingsWithIcon = this._menuItems.some(menuItem => !!menuItem.icon); + const siblingsWithIcon = this._menuItemsAll.some(menuItem => !!menuItem.icon); + const itemSelectionMode = this._parentItemSelectionMode; - this._menuItems.forEach(item => { + this._menuItemsAll.forEach(item => { item._siblingsWithIcon = siblingsWithIcon; }); + + if (this._prevSelected === undefined) { + this._prevSelected = this.isSelected; + } + + if (itemSelectionMode === ItemSelectionMode.None) { + return; + } + if (itemSelectionMode === ItemSelectionMode.SingleSelect && this.isSelected) { + this._parentGroup?._clearSelectedItems(); + this.isSelected = true; + } + } + + _setupItemNavigation() { + if (this._list) { + this._list._itemNavigation._getItems = () => this._menuItemsNavigation; + } } get _focusable() { @@ -280,10 +355,29 @@ class MenuItem extends ListItem implements IMenuItem { return this.shadowRoot!.querySelector("[ui5-responsive-popover]")!; } + get _menuItemGroups() { + return this.items.filter((item) : item is MenuItemGroup => item.isGroup); + } + get _menuItems() { return this.items.filter((item): item is MenuItem => !item.isSeparator); } + // Return only the menu items, including items that belong to groups, but excluding separators + get _menuItemsAll() { + const items: MenuItem[] = []; + + this.items.forEach(item => { + if (item.isGroup) { + items.push(...(item as MenuItemGroup)._menuItems); + } else if (!item.isSeparator) { + items.push(item as MenuItem); + } + }); + + return items; + } + _closeAll() { if (this._popover) { this._popover.open = false; @@ -308,7 +402,7 @@ class MenuItem extends ListItem implements IMenuItem { } _afterPopoverOpen() { - this.items[0]?.focus(); + this._menuItemsAll[0]?.focus(); this.fireEvent("open", {}, false, false); } diff --git a/packages/main/src/MenuItemGroup.hbs b/packages/main/src/MenuItemGroup.hbs new file mode 100644 index 000000000000..a7683262f7ca --- /dev/null +++ b/packages/main/src/MenuItemGroup.hbs @@ -0,0 +1 @@ + diff --git a/packages/main/src/MenuItemGroup.ts b/packages/main/src/MenuItemGroup.ts new file mode 100644 index 000000000000..ac5907ee83df --- /dev/null +++ b/packages/main/src/MenuItemGroup.ts @@ -0,0 +1,105 @@ +import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js"; +import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js"; +import property from "@ui5/webcomponents-base/dist/decorators/property.js"; +import slot from "@ui5/webcomponents-base/dist/decorators/slot.js"; +import litRender from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js"; +import MenuItem from "./MenuItem.js"; +import MenuItemGroupTemplate from "./generated/templates/MenuItemGroupTemplate.lit.js"; +import type ItemSelectionMode from "./types/ItemSelectionMode.js"; +import type { IMenuItem } from "./Menu.js"; + +/** + * @class + * + * ### Overview + * + * `ui5-menu-item-group` is the group of items to use inside a `ui5-menu`. + * Items that belong to the same group should be enclosed by `ui5-menu-item-group`. + * Each group can have itemSelectionMode property, which defines the selection mode of the items inside. + * Possible values are 'None' (default), 'SingleSelect', 'MultiSelect'. + * + * ### Usage + * + * `ui5-menu-item-group` represents a collection of `ui5-menu-item` components that can have the same selection mode. + * The items are addeed to the group's `items` slot. + * + * ### ES6 Module Import + * + * `import "@ui5/webcomponents/dist/MenuItemGroup.js";` + * @constructor + * @extends UI5Element + * @implements {IMenuItem} + * @since 2.3.0 + * @public + */ +@customElement({ + tag: "ui5-menu-item-group", + renderer: litRender, + template: MenuItemGroupTemplate, + dependencies: [MenuItem], +}) +class MenuItemGroup extends UI5Element implements IMenuItem { + /** + * Defines the component selection mode. + * @default "None" + * @public + */ + @property() + itemSelectionMode: `${ItemSelectionMode}` = "None"; + + /** + * Defines the items of this component. + * **Note:** The slot can hold `ui5-menu-item` and `ui5-menu-separator` items. + * @public + */ + @slot({ "default": true, type: HTMLElement, invalidateOnChildChange: true }) + items!: Array; + + get isSeparator(): boolean { + return false; + } + + get isGroup(): boolean { + return true; + } + + // Return only the menu items, excluding separators + get _menuItems() { + return this.items.filter((item) : item is MenuItem => !item.isSeparator); + } + + onBeforeRendering() { + if (this.itemSelectionMode === "SingleSelect") { + this._ensureSingleSelection(); + } + } + + /** + * Sets selected property of all items in the group to false. + * @private + */ + _clearSelectedItems() { + this.items.forEach((item: IMenuItem) => { + if (!item.isSeparator && !item.isGroup) { + (item as MenuItem).isSelected = false; + } + }); + } + + /** + * Ensures that only one item is selected in the group (if there were any selected items). + * @private + */ + _ensureSingleSelection() { + const lastSelectedItem = this.items.findLast((item: IMenuItem) => (item as MenuItem).isSelected); + + this._clearSelectedItems(); + if (lastSelectedItem) { + (lastSelectedItem as MenuItem).isSelected = true; + } + } +} + +MenuItemGroup.define(); + +export default MenuItemGroup; diff --git a/packages/main/src/MenuSeparator.ts b/packages/main/src/MenuSeparator.ts index 097935fbce8d..0dd12a9ab1da 100644 --- a/packages/main/src/MenuSeparator.ts +++ b/packages/main/src/MenuSeparator.ts @@ -30,6 +30,10 @@ class MenuSeparator extends ListItemBase implements IMenuItem { return true; } + get isGroup() { + return false; + } + get classes(): ClassMap { return { main: { diff --git a/packages/main/src/bundle.esm.ts b/packages/main/src/bundle.esm.ts index 8f67245661ac..d7af045339a8 100644 --- a/packages/main/src/bundle.esm.ts +++ b/packages/main/src/bundle.esm.ts @@ -66,6 +66,7 @@ import Menu from "./Menu.js"; import NavigationMenu from "./NavigationMenu.js"; import NavigationMenuItem from "./NavigationMenuItem.js"; import MenuItem from "./MenuItem.js"; +import MenuItemGroup from "./MenuItemGroup.js"; import MenuSeparator from "./MenuSeparator.js"; import Popover from "./Popover.js"; import Panel from "./Panel.js"; diff --git a/packages/main/src/themes/MenuItem.css b/packages/main/src/themes/MenuItem.css index 0d14f2ea3184..f7e9e49377db 100644 --- a/packages/main/src/themes/MenuItem.css +++ b/packages/main/src/themes/MenuItem.css @@ -73,6 +73,18 @@ visibility: hidden; } +.ui5-menu-item-selected { + padding-inline-start: 0.5rem; + padding-inline-end: 0; + font-weight: normal; + text-align: center; +} + +.ui5-menu-item-icon-selected { + color: var(--sapContent_BusyColor); + padding-top: 0.25rem; +} + :host::part(title) { font-size: var(--sapFontSize); padding-top: 0.125rem; diff --git a/packages/main/src/types/ItemSelectionMode.ts b/packages/main/src/types/ItemSelectionMode.ts new file mode 100644 index 000000000000..ff5fe03b27e4 --- /dev/null +++ b/packages/main/src/types/ItemSelectionMode.ts @@ -0,0 +1,25 @@ +/** + * Menu item selection modes. + * @public + */ +enum ItemSelectionMode { + /** + * default type (no item selection) + * @public + */ + None = "None", + + /** + * Single item selection mode + * @public + */ + SingleSelect = "SingleSelect", + + /** + * Multiple items selection mode + * @public + */ + MultiSelect = "MultiSelect", +} + +export default ItemSelectionMode; diff --git a/packages/main/test/pages/Menu.html b/packages/main/test/pages/Menu.html index 5619953ca74d..22b7fda16d49 100644 --- a/packages/main/test/pages/Menu.html +++ b/packages/main/test/pages/Menu.html @@ -112,6 +112,45 @@ + Menu with item groups + Open Menu + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
For more information: @@ -126,12 +165,17 @@ btnOpen.addEventListener("click", function() { menu.opener = "btnOpen"; - menu.open = !menu.open; + menu.open = true; }); btnOpenEndContent.addEventListener("click", function() { menuEndContent.opener = "btnOpenEndContent"; - menuEndContent.open = !menu.open; + menuEndContent.open = true; + }); + + btnOpenGroups.addEventListener("click", function() { + menuGroups.opener = "btnOpenGroups"; + menuGroups.open = true; }); btnAddOpener.addEventListener("click", function() { From 69b4859c78ff6176e747dadf67c07fc8d27a18f5 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Thu, 17 Oct 2024 16:48:14 +0300 Subject: [PATCH 02/22] feat(ui5-menu): add accessibility features and tests --- packages/main/cypress/specs/Menu.cy.ts | 317 +++++++++++++++++++++++++ packages/main/src/MenuItem.ts | 14 +- packages/main/src/MenuItemGroup.hbs | 2 + packages/main/test/pages/Menu.html | 5 +- 4 files changed, 336 insertions(+), 2 deletions(-) diff --git a/packages/main/cypress/specs/Menu.cy.ts b/packages/main/cypress/specs/Menu.cy.ts index e326173a0fef..686daef3cb8a 100644 --- a/packages/main/cypress/specs/Menu.cy.ts +++ b/packages/main/cypress/specs/Menu.cy.ts @@ -2,6 +2,8 @@ import { html } from "lit"; import "../../src/Button.js"; import "../../src/Menu.js"; import "../../src/MenuItem.js"; +import "../../src/MenuSeparator.js"; +import "../../src/MenuItemGroup.js"; import type MenuItem from "../../src/MenuItem.js"; describe("Menu interaction", () => { @@ -426,4 +428,319 @@ describe("Menu interaction", () => { .should("be.equal", "Select an option from the menu"); }); }); + + describe("Check mark is rendered for selectable and selected items", () => { + it("Selected items have check mark rendered when it is necessary", () => { + cy.mount(html`Open Menu + + + + + + + + + + + + + + + + + + `); + + cy.get("[ui5-menu]") + .as("menu"); + + cy.get("@menu") + .find("[text='Item 1']") + .shadow() + .find("[part='selected']") + .should("not.exist"); + + cy.get("@menu") + .find("[id='groupSingle']") + .as("groupSingle"); + + cy.get("@groupSingle") + .find("[text='Item 2']") + .shadow() + .find("[part='selected']") + .should("exist"); + + cy.get("@groupSingle") + .find("[text='Item 3']") + .shadow() + .find("[part='selected']") + .should("not.exist"); + + cy.get("@menu") + .find("[id='groupMulti']") + .as("groupMulti"); + + cy.get("@groupMulti") + .find("[text='Item 4']") + .shadow() + .find("[part='selected']") + .should("exist"); + + cy.get("@groupMulti") + .find("[text='Item 5']") + .shadow() + .find("[part='selected']") + .should("exist"); + + cy.get("@groupMulti") + .find("[text='Item 6']") + .shadow() + .find("[part='selected']") + .should("not.exist"); + + cy.get("@menu") + .find("[id='groupNone']") + .as("groupNone"); + + cy.get("@groupNone") + .find("[text='Item 7']") + .shadow() + .find("[part='selected']") + .should("not.exist"); + + cy.get("@groupNone") + .find("[text='Item 8']") + .shadow() + .find("[part='selected']") + .should("not.exist"); + }); + + it("Select item (outside of any group)", () => { + cy.mount(html`Open Menu + + + `); + + cy.get("[ui5-menu]") + .as("menu"); + + cy.get("@menu") + .find("[text='Item 1']") + .as("item") + .ui5MenuItemClick(); + + cy.get("@menu") + .find("[text='Item 1']") + .shadow() + .find("[part='selected']") + .should("not.exist"); + }); + + it("Select/deselect items (SingleSelect mode)", () => { + cy.mount(html`Open Menu + + + + + + `); + + cy.get("[ui5-menu]") + .as("menu"); + + cy.get("@menu") + .find("[text='Item 2']") + .ui5MenuItemClick(); + + cy.get("@menu") + .find("[text='Item 2']") + .shadow() + .find("[part='selected']") + .should("exist"); + + cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + + cy.get("@menu") + .find("[text='Item 3']") + .ui5MenuItemClick(); + + cy.get("@menu") + .find("[text='Item 2']") + .shadow() + .find("[part='selected']") + .should("not.exist"); + + cy.get("@menu") + .find("[text='Item 3']") + .shadow() + .find("[part='selected']") + .should("exist"); + + cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + + cy.get("@menu") + .find("[text='Item 3']") + .ui5MenuItemClick(); + + cy.get("@menu") + .find("[text='Item 3']") + .shadow() + .find("[part='selected']") + .should("not.exist"); + }); + + it("Select/deselect items (MultiSelect mode) ", () => { + cy.mount(html`Open Menu + + + + + + `); + + cy.get("[ui5-menu]") + .as("menu"); + + cy.get("@menu") + .find("[text='Item 4']") + .ui5MenuItemClick(); + + cy.get("@menu") + .find("[text='Item 4']") + .shadow() + .find("[part='selected']") + .should("exist"); + + cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + + cy.get("@menu") + .find("[text='Item 5']") + .ui5MenuItemClick(); + + cy.get("@menu") + .find("[text='Item 4']") + .shadow() + .find("[part='selected']") + .should("exist"); + + cy.get("@menu") + .find("[text='Item 5']") + .shadow() + .find("[part='selected']") + .should("exist"); + + cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + + cy.get("@menu") + .find("[text='Item 5']") + .ui5MenuItemClick(); + + cy.get("@menu") + .find("[text='Item 5']") + .shadow() + .find("[part='selected']") + .should("not.exist"); + }); + + it("Select item (None mode) ", () => { + cy.mount(html`Open Menu + + + + + `); + + cy.get("[ui5-menu]") + .as("menu"); + + cy.get("@menu") + .find("[text='Item 6']") + .ui5MenuItemClick(); + + cy.get("@menu") + .find("[text='Item 6']") + .shadow() + .find("[part='selected']") + .should("not.exist"); + }); + + it("Accessibility attributes", () => { + it("Selected items have check mark rendered when it is necessary", () => { + cy.mount(html`Open Menu + + + + + + + + + + + + + + + `); + + cy.get("[ui5-menu]") + .as("menu"); + + cy.get("@menu") + .find("[text='Item 1']") + .shadow() + .find("li") + .should("have.attr", "role", "menuitem"); + + cy.get("@menu") + .find("[id='groupSingle']") + .as("groupSingle"); + + cy.get("@groupSingle") + .shadow() + .find("div") + .should("have.attr", "role", "group"); + + cy.get("@groupSingle") + .find("[text='Item 2']") + .shadow() + .find("li") + .should("have.attr", "role", "menuitemradio"); + + cy.get("@menu") + .find("[id='groupMulti']") + .as("groupMulti"); + + cy.get("@groupMulti") + .shadow() + .find("div") + .should("have.attr", "role", "group"); + + cy.get("@groupMulti") + .find("[text='Item 4']") + .shadow() + .find("[part='selected']") + .should("have.attr", "role", "menuitemcheckbox"); + + cy.get("@menu") + .find("[id='groupNone']") + .as("groupNone"); + + cy.get("@groupNone") + .shadow() + .find("div") + .should("have.attr", "role", "group"); + + cy.get("@groupNone") + .find("[text='Item 6']") + .shadow() + .find("[part='selected']") + .should("have.attr", "role", "menuitem"); + }); + }); + }); }); diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 9cacfd97d575..56449357081c 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -340,12 +340,24 @@ class MenuItem extends ListItem implements IMenuItem { return true; } + get _role() { + switch (this._parentItemSelectionMode) { + case ItemSelectionMode.SingleSelect: + return "menuitemradio"; + case ItemSelectionMode.MultiSelect: + return "menuitemcheckbox"; + default: + return "menuitem"; + } + } + get _accInfo() { const accInfoSettings = { - role: this.accessibilityAttributes.role || "menuitem", + role: this.accessibilityAttributes.role || this._role, ariaHaspopup: this.hasSubmenu ? AriaHasPopup.Menu.toLowerCase() as Lowercase : undefined, ariaKeyShortcuts: this.accessibilityAttributes.ariaKeyShortcuts, ariaHidden: !!this.additionalText && !!this.accessibilityAttributes.ariaKeyShortcuts ? true : undefined, + ariaChecked: this._markSelected ? true : undefined, }; return { ...super._accInfo, ...accInfoSettings }; diff --git a/packages/main/src/MenuItemGroup.hbs b/packages/main/src/MenuItemGroup.hbs index a7683262f7ca..4352999996a1 100644 --- a/packages/main/src/MenuItemGroup.hbs +++ b/packages/main/src/MenuItemGroup.hbs @@ -1 +1,3 @@ +
+
\ No newline at end of file diff --git a/packages/main/test/pages/Menu.html b/packages/main/test/pages/Menu.html index 22b7fda16d49..0e3861918e3d 100644 --- a/packages/main/test/pages/Menu.html +++ b/packages/main/test/pages/Menu.html @@ -126,10 +126,13 @@ - + + + + From e91564d8e3a13178557a03afbddf7ee2beba23a4 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Fri, 18 Oct 2024 15:16:40 +0300 Subject: [PATCH 03/22] feat(ui5-menu): fix comments --- packages/main/src/Menu.ts | 6 +++++- packages/main/src/MenuItem.ts | 1 + packages/main/src/MenuItemGroup.ts | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 2fa1fa9a5eea..159e7d4496e1 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -402,12 +402,16 @@ class Menu extends UI5Element { }, MENU_OPEN_DELAY); } + _isGroupParent(parent: MenuItem | Menu | MenuItemGroup): boolean { + return parent && "isGroup" in parent && parent.isGroup; + } + _itemClick(e: CustomEvent) { const item = e.detail.item as MenuItem; const prevSelected = item.isSelected; const parent = item.parentElement as MenuItem | Menu | MenuItemGroup; - if (parent && "isGroup" in parent && parent.isGroup) { + if (this._isGroupParent(parent)) { const itemSelectionMode = (parent as MenuItemGroup).itemSelectionMode; if (itemSelectionMode === ItemSelectionMode.SingleSelect) { (parent as MenuItemGroup)._clearSelectedItems(); diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 56449357081c..afd401e6f3e3 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -109,6 +109,7 @@ class MenuItem extends ListItem implements IMenuItem { * **Note:** A selected `ui5-menu-item` have selection mark displayed ad its end. * @default false * @public + * @since 2.4.0 */ @property({ type: Boolean }) isSelected = false; diff --git a/packages/main/src/MenuItemGroup.ts b/packages/main/src/MenuItemGroup.ts index ac5907ee83df..e6a6d2951969 100644 --- a/packages/main/src/MenuItemGroup.ts +++ b/packages/main/src/MenuItemGroup.ts @@ -29,7 +29,7 @@ import type { IMenuItem } from "./Menu.js"; * @constructor * @extends UI5Element * @implements {IMenuItem} - * @since 2.3.0 + * @since 2.4.0 * @public */ @customElement({ From 5a03a771945fe9f4570daef95ea1c154e91a3ed5 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Fri, 18 Oct 2024 16:10:17 +0300 Subject: [PATCH 04/22] feat(ui5-menu): fix more comments --- packages/main/src/Menu.ts | 1 + packages/main/src/MenuItemGroup.ts | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 159e7d4496e1..c3ba0faf913d 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -50,6 +50,7 @@ const MENU_OPEN_DELAY = 300; interface IMenuItem extends UI5Element { isSeparator: boolean; isGroup: boolean; + isSelected?: boolean; } type MenuItemClickEventDetail = { diff --git a/packages/main/src/MenuItemGroup.ts b/packages/main/src/MenuItemGroup.ts index e6a6d2951969..5b861093d501 100644 --- a/packages/main/src/MenuItemGroup.ts +++ b/packages/main/src/MenuItemGroup.ts @@ -81,7 +81,7 @@ class MenuItemGroup extends UI5Element implements IMenuItem { _clearSelectedItems() { this.items.forEach((item: IMenuItem) => { if (!item.isSeparator && !item.isGroup) { - (item as MenuItem).isSelected = false; + item.isSelected = false; } }); } @@ -91,11 +91,11 @@ class MenuItemGroup extends UI5Element implements IMenuItem { * @private */ _ensureSingleSelection() { - const lastSelectedItem = this.items.findLast((item: IMenuItem) => (item as MenuItem).isSelected); + const lastSelectedItem = this.items.findLast((item: IMenuItem) => item.isSelected); this._clearSelectedItems(); if (lastSelectedItem) { - (lastSelectedItem as MenuItem).isSelected = true; + lastSelectedItem.isSelected = true; } } } From 3dab552044e1af222aefed57954cc4d3822dcc29 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Fri, 1 Nov 2024 09:55:06 +0200 Subject: [PATCH 05/22] feat(ui5-menu): fix comments --- packages/main/src/Menu.ts | 4 ++-- packages/main/src/MenuItem.ts | 10 ---------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index c3ba0faf913d..cb82ca1f643c 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -287,7 +287,7 @@ class Menu extends UI5Element { } get _menuItemsAll() { - const items: MenuItem[] = []; + const items: Array = []; this.items.forEach(item => { if (item.isGroup) { @@ -303,7 +303,7 @@ class Menu extends UI5Element { get _menuItemsNavigation() { const parent = this._parent || this; - const items: MenuItem[] = []; + const items: Array = []; const slottedItems = parent.getSlottedNodes("items"); slottedItems.forEach(item => { diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index afd401e6f3e3..375696d3420d 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -183,12 +183,6 @@ class MenuItem extends ListItem implements IMenuItem { @property({ type: Boolean, noAttribute: true }) _siblingsWithIcon = false; - /** - * Keeps the previous selected state of the item. - */ - @property({ noAttribute: true }) - _prevSelected?: boolean; - /** * Defines the items of this component. * @@ -318,10 +312,6 @@ class MenuItem extends ListItem implements IMenuItem { item._siblingsWithIcon = siblingsWithIcon; }); - if (this._prevSelected === undefined) { - this._prevSelected = this.isSelected; - } - if (itemSelectionMode === ItemSelectionMode.None) { return; } From 7954ca5228a4fc134d5e275eb0fa8778c3e401d7 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Wed, 6 Nov 2024 17:03:39 +0200 Subject: [PATCH 06/22] feat(ui5-menu): fix more comments --- packages/main/src/Menu.ts | 12 +----------- packages/main/src/MenuItem.ts | 3 ++- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 922a2759e5b0..badd930a8e4c 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -238,9 +238,6 @@ class Menu extends UI5Element { @property({ converter: DOMReferenceConverter }) opener?: HTMLElement | string; - @property({ type: Object }) - _parent?: MenuItem | Menu; - /** * Defines the items of this component. * @@ -278,7 +275,6 @@ class Menu extends UI5Element { return this.items.filter((item) : item is MenuItemGroup => item.isGroup); } - // Return only the menu items, excluding separators and items that belong to groups get _menuItems() { return this.items.filter((item) : item is MenuItem => !item.isSeparator && !item.isGroup); } @@ -298,10 +294,8 @@ class Menu extends UI5Element { } get _menuItemsNavigation() { - const parent = this._parent || this; - const items: Array = []; - const slottedItems = parent.getSlottedNodes("items"); + const slottedItems = this.getSlottedNodes("items"); slottedItems.forEach(item => { if (item.isGroup) { @@ -312,8 +306,6 @@ class Menu extends UI5Element { } }); - this._parent = undefined; - return items; } @@ -352,8 +344,6 @@ class Menu extends UI5Element { item, }, false, false); - this._parent = item; - item._setupItemNavigation(); item._popover.opener = item; item._popover.open = true; item.selected = true; diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 310bf7105457..6e69d4c978ae 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -308,6 +308,8 @@ class MenuItem extends ListItem implements IMenuItem { const siblingsWithIcon = this._menuItemsAll.some(menuItem => !!menuItem.icon); const itemSelectionMode = this._parentItemSelectionMode; + this._setupItemNavigation(); + this._menuItemsAll.forEach(item => { item._siblingsWithIcon = siblingsWithIcon; }); @@ -366,7 +368,6 @@ class MenuItem extends ListItem implements IMenuItem { return this.items.filter((item): item is MenuItem => !item.isSeparator); } - // Return only the menu items, including items that belong to groups, but excluding separators get _menuItemsAll() { const items: MenuItem[] = []; From 31de2014a24edc6034308b4430f9b1e74ac19f2f Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Tue, 12 Nov 2024 10:52:06 +0200 Subject: [PATCH 07/22] feat(ui5-menu): fix comments and adapt tests --- packages/main/cypress/specs/Menu.cy.ts | 95 ++++++++++++++++++++------ packages/main/src/Menu.ts | 10 +-- packages/main/src/MenuItem.ts | 72 ++++++++++++------- packages/main/src/MenuItemGroup.hbs | 5 +- packages/main/src/MenuItemGroup.ts | 28 +++++++- packages/main/test/pages/Menu.html | 14 ++-- 6 files changed, 163 insertions(+), 61 deletions(-) diff --git a/packages/main/cypress/specs/Menu.cy.ts b/packages/main/cypress/specs/Menu.cy.ts index 4848d363c0d0..a80b76114ac9 100644 --- a/packages/main/cypress/specs/Menu.cy.ts +++ b/packages/main/cypress/specs/Menu.cy.ts @@ -208,21 +208,36 @@ describe("Menu interaction", () => { }); describe("Event firing", () => { - it("Event firing - 'ui5-item-click' after 'click' on menu item", () => { + it("Event firing - 'ui5-item-click' and 'ui5-item-selection' after 'click' on menu item", () => { cy.mount(html`Open Menu + + + `); cy.get("[ui5-menu]") + .as("menu") .ui5MenuOpen(); - cy.get("[ui5-menu]") + cy.get("@menu") + .find("[ui5-menu-item-group]") + .as("group"); + + cy.get("@menu") .then($item => { $item.get(0).addEventListener("ui5-item-click", cy.stub().as("clicked")); }); - cy.get("[ui5-menu]") + cy.get("@group") + .get("div") + .get("[ui5-menu-item]") + .then($item => { + $item.get(0).addEventListener("ui5-item-selection", cy.stub().as("selected")); + }); + + cy.get("@menu") .ui5MenuOpened(); cy.get("[ui5-menu-item]") @@ -230,6 +245,9 @@ describe("Menu interaction", () => { cy.get("@clicked") .should("have.been.calledOnce"); + + cy.get("@selected") + .should("have.been.calledOnce"); }); it("Event firing - 'ui5-item-click' after 'Space' on menu item", () => { @@ -488,6 +506,7 @@ describe("Menu interaction", () => { cy.get("@menu") .find("[text='Item 1']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("not.exist"); @@ -498,14 +517,16 @@ describe("Menu interaction", () => { cy.get("@groupSingle") .find("[text='Item 2']") .shadow() + .find("[part='content']") .find("[part='selected']") - .should("exist"); + .should("not.exist"); cy.get("@groupSingle") .find("[text='Item 3']") .shadow() + .find("[part='content']") .find("[part='selected']") - .should("not.exist"); + .should("exist"); cy.get("@menu") .find("[id='groupMulti']") @@ -514,18 +535,21 @@ describe("Menu interaction", () => { cy.get("@groupMulti") .find("[text='Item 4']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("exist"); cy.get("@groupMulti") .find("[text='Item 5']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("exist"); cy.get("@groupMulti") .find("[text='Item 6']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("not.exist"); @@ -536,12 +560,14 @@ describe("Menu interaction", () => { cy.get("@groupNone") .find("[text='Item 7']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("not.exist"); cy.get("@groupNone") .find("[text='Item 8']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("not.exist"); }); @@ -567,6 +593,8 @@ describe("Menu interaction", () => { .should("not.exist"); }); + /* === commented out due to unintentional behavior === + it("Select/deselect items (SingleSelect mode)", () => { cy.mount(html`Open Menu @@ -580,44 +608,62 @@ describe("Menu interaction", () => { .as("menu"); cy.get("@menu") + .find("[id='groupSingle']") + .as("groupSingle"); + + cy.get("@groupSingle") + .find("[text='Item 2']") + .shadow() + .find("[part='content']") + .find("[part='selected']") + .should("not.exist"); + + cy.get("@groupSingle") .find("[text='Item 2']") .ui5MenuItemClick(); cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + + cy.get("@groupSingle") .find("[text='Item 2']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("exist"); - cy.get("@menu") - .ui5MenuOpen({ opener: "btnOpen" }); - - cy.get("@menu") + cy.get("@groupSingle") .find("[text='Item 3']") .ui5MenuItemClick(); - cy.get("@menu") + cy.get("@groupSingle") .find("[text='Item 2']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("not.exist"); cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + + cy.get("@groupSingle") .find("[text='Item 3']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("exist"); - cy.get("@menu") - .ui5MenuOpen({ opener: "btnOpen" }); - - cy.get("@menu") + cy.get("@groupSingle") .find("[text='Item 3']") .ui5MenuItemClick(); cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + + cy.get("@groupSingle") .find("[text='Item 3']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("not.exist"); }); @@ -635,47 +681,56 @@ describe("Menu interaction", () => { .as("menu"); cy.get("@menu") + .find("[id='groupMulti']") + .as("groupMulti"); + + cy.get("@groupMulti") .find("[text='Item 4']") .ui5MenuItemClick(); - cy.get("@menu") + cy.get("@groupMulti") .find("[text='Item 4']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("exist"); cy.get("@menu") .ui5MenuOpen({ opener: "btnOpen" }); - cy.get("@menu") + cy.get("@groupMulti") .find("[text='Item 5']") .ui5MenuItemClick(); - cy.get("@menu") + cy.get("@groupMulti") .find("[text='Item 4']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("exist"); - cy.get("@menu") + cy.get("@groupMulti") .find("[text='Item 5']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("exist"); cy.get("@menu") .ui5MenuOpen({ opener: "btnOpen" }); - cy.get("@menu") + cy.get("@groupMulti") .find("[text='Item 5']") .ui5MenuItemClick(); - cy.get("@menu") + cy.get("@groupMulti") .find("[text='Item 5']") .shadow() + .find("[part='content']") .find("[part='selected']") .should("not.exist"); }); + */ it("Select item (None mode) ", () => { cy.mount(html`Open Menu diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index badd930a8e4c..6df3a1759ce6 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -26,7 +26,6 @@ import BusyIndicator from "./BusyIndicator.js"; import MenuItem from "./MenuItem.js"; import MenuSeparator from "./MenuSeparator.js"; import type MenuItemGroup from "./MenuItemGroup.js"; -import ItemSelectionMode from "./types/ItemSelectionMode.js"; import type { ListItemClickEventDetail, } from "./List.js"; @@ -397,15 +396,8 @@ class Menu extends UI5Element { _itemClick(e: CustomEvent) { const item = e.detail.item as MenuItem; const prevSelected = item.isSelected; - const parent = item.parentElement as MenuItem | Menu | MenuItemGroup; - if (this._isGroupParent(parent)) { - const itemSelectionMode = (parent as MenuItemGroup).itemSelectionMode; - if (itemSelectionMode === ItemSelectionMode.SingleSelect) { - (parent as MenuItemGroup)._clearSelectedItems(); - } - item.isSelected = !prevSelected; - } + item.isSelected = !prevSelected; if (!item._popover) { const prevented = !this.fireEvent("item-click", { diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 6e69d4c978ae..c50ec79b955b 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -1,6 +1,7 @@ import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js"; import property from "@ui5/webcomponents-base/dist/decorators/property.js"; import slot from "@ui5/webcomponents-base/dist/decorators/slot.js"; +import event from "@ui5/webcomponents-base/dist/decorators/event.js"; import i18n from "@ui5/webcomponents-base/dist/decorators/i18n.js"; import { isPhone } from "@ui5/webcomponents-base/dist/Device.js"; import AriaHasPopup from "@ui5/webcomponents-base/dist/types/AriaHasPopup.js"; @@ -62,6 +63,13 @@ type MenuItemAccessibilityAttributes = Pick("[ui5-list]")!; } @@ -301,12 +327,12 @@ class MenuItem extends ListItem implements IMenuItem { } get _markSelected() { - return this.isSelected && this._parentItemSelectionMode !== ItemSelectionMode.None; + return this.isSelected && this._itemSelectionMode !== ItemSelectionMode.None; } onBeforeRendering() { const siblingsWithIcon = this._menuItemsAll.some(menuItem => !!menuItem.icon); - const itemSelectionMode = this._parentItemSelectionMode; + const itemSelectionMode = this._itemSelectionMode; this._setupItemNavigation(); @@ -334,7 +360,7 @@ class MenuItem extends ListItem implements IMenuItem { } get _role() { - switch (this._parentItemSelectionMode) { + switch (this._itemSelectionMode) { case ItemSelectionMode.SingleSelect: return "menuitemradio"; case ItemSelectionMode.MultiSelect: @@ -387,7 +413,7 @@ class MenuItem extends ListItem implements IMenuItem { this._popover.open = false; } this.selected = false; - this.fireEvent("close-menu", {}); + this.fireDecoratorEvent("close-menu"); } _close() { @@ -407,7 +433,7 @@ class MenuItem extends ListItem implements IMenuItem { _afterPopoverOpen() { this._menuItemsAll[0]?.focus(); - this.fireEvent("open", {}, false, false); + this.fireDecoratorEvent("open"); } _beforePopoverClose(e: CustomEvent) { @@ -422,13 +448,13 @@ class MenuItem extends ListItem implements IMenuItem { if (e.detail.escPressed) { this.focus(); if (isPhone()) { - this.fireEvent("close-menu", {}); + this.fireDecoratorEvent("close-menu"); } } } _afterPopoverClose() { - this.fireEvent("close", {}, false, false); + this.fireDecoratorEvent("close"); } } diff --git a/packages/main/src/MenuItemGroup.hbs b/packages/main/src/MenuItemGroup.hbs index 4352999996a1..163afe1bd59b 100644 --- a/packages/main/src/MenuItemGroup.hbs +++ b/packages/main/src/MenuItemGroup.hbs @@ -1,3 +1,6 @@ -
+
\ No newline at end of file diff --git a/packages/main/src/MenuItemGroup.ts b/packages/main/src/MenuItemGroup.ts index 5b861093d501..2cc2c21c9e09 100644 --- a/packages/main/src/MenuItemGroup.ts +++ b/packages/main/src/MenuItemGroup.ts @@ -5,7 +5,7 @@ import slot from "@ui5/webcomponents-base/dist/decorators/slot.js"; import litRender from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js"; import MenuItem from "./MenuItem.js"; import MenuItemGroupTemplate from "./generated/templates/MenuItemGroupTemplate.lit.js"; -import type ItemSelectionMode from "./types/ItemSelectionMode.js"; +import ItemSelectionMode from "./types/ItemSelectionMode.js"; import type { IMenuItem } from "./Menu.js"; /** @@ -45,7 +45,7 @@ class MenuItemGroup extends UI5Element implements IMenuItem { * @public */ @property() - itemSelectionMode: `${ItemSelectionMode}` = "None"; + itemSelectionMode: `${ItemSelectionMode}` = ItemSelectionMode.None; /** * Defines the items of this component. @@ -69,11 +69,23 @@ class MenuItemGroup extends UI5Element implements IMenuItem { } onBeforeRendering() { - if (this.itemSelectionMode === "SingleSelect") { + this._updateItemsSelectionMode(); + + if (this.itemSelectionMode === ItemSelectionMode.SingleSelect) { this._ensureSingleSelection(); } } + /** + * Sets _itemSelectionMode property of all menu items in the group. + * @private + */ + _updateItemsSelectionMode() { + this._menuItems.forEach((item: MenuItem) => { + item._itemSelectionMode = this.itemSelectionMode; + }); + } + /** * Sets selected property of all items in the group to false. * @private @@ -98,6 +110,16 @@ class MenuItemGroup extends UI5Element implements IMenuItem { lastSelectedItem.isSelected = true; } } + + /** + * Handles the selection of an item in the group and unselects other items if the item selection mode is SingleSelect. + * @private + */ + _handleItemSelection() { + if (this.itemSelectionMode === ItemSelectionMode.SingleSelect) { + this._clearSelectedItems(); + } + } } MenuItemGroup.define(); diff --git a/packages/main/test/pages/Menu.html b/packages/main/test/pages/Menu.html index 0e3861918e3d..669daa11f489 100644 --- a/packages/main/test/pages/Menu.html +++ b/packages/main/test/pages/Menu.html @@ -130,11 +130,15 @@ - - - - + + + + + + + + @@ -214,7 +218,7 @@ menu.addEventListener("ui5-item-click", function(event) { const item = event.detail.item; selectionInput.value = item.text; - if (item && item.text === "New File(selection prevented)") { + if (item && item.text === "New File (selection prevented)") { event.preventDefault(); } }); From c98c8dc229cc60a4640f116eb117c46262e0a56c Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Tue, 12 Nov 2024 14:58:56 +0200 Subject: [PATCH 08/22] feat(ui5-menu): fix build error --- packages/main/src/MenuItem.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index c50ec79b955b..5180e0983dfd 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -189,6 +189,7 @@ class MenuItem extends ListItem implements IMenuItem { * @default false * @private */ + @property({ type: Boolean, noAttribute: true }) _isSelected = false; /** @@ -241,7 +242,7 @@ class MenuItem extends ListItem implements IMenuItem { this._isSelected = value; } - get isSelected() { + get isSelected(): boolean { return this._isSelected; } From e7a79ca04e5c1da640980057333e5670fae3bab8 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Mon, 18 Nov 2024 12:02:21 +0200 Subject: [PATCH 09/22] feat(ui5-menu): fix comments and tests --- packages/main/cypress/specs/Menu.cy.ts | 104 ++++++++++-------- .../cypress/support/commands/Menu.commands.ts | 2 +- packages/main/src/Menu.ts | 6 +- packages/main/src/MenuItem.ts | 36 +++--- packages/main/src/MenuItemGroup.ts | 23 ++-- packages/main/src/types/ItemSelectionMode.ts | 5 +- packages/main/test/pages/Menu.html | 14 +-- 7 files changed, 96 insertions(+), 94 deletions(-) diff --git a/packages/main/cypress/specs/Menu.cy.ts b/packages/main/cypress/specs/Menu.cy.ts index a80b76114ac9..5a737ab0f218 100644 --- a/packages/main/cypress/specs/Menu.cy.ts +++ b/packages/main/cypress/specs/Menu.cy.ts @@ -212,7 +212,7 @@ describe("Menu interaction", () => { cy.mount(html`Open Menu - + `); @@ -483,11 +483,11 @@ describe("Menu interaction", () => { cy.mount(html`Open Menu - + - + @@ -507,7 +507,7 @@ describe("Menu interaction", () => { .find("[text='Item 1']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("not.exist"); cy.get("@menu") @@ -518,14 +518,14 @@ describe("Menu interaction", () => { .find("[text='Item 2']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("not.exist"); cy.get("@groupSingle") .find("[text='Item 3']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("exist"); cy.get("@menu") @@ -536,21 +536,21 @@ describe("Menu interaction", () => { .find("[text='Item 4']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("exist"); cy.get("@groupMulti") .find("[text='Item 5']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("exist"); cy.get("@groupMulti") .find("[text='Item 6']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("not.exist"); cy.get("@menu") @@ -561,14 +561,14 @@ describe("Menu interaction", () => { .find("[text='Item 7']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("not.exist"); cy.get("@groupNone") .find("[text='Item 8']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("not.exist"); }); @@ -589,17 +589,15 @@ describe("Menu interaction", () => { cy.get("@menu") .find("[text='Item 1']") .shadow() - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("not.exist"); }); - /* === commented out due to unintentional behavior === - - it("Select/deselect items (SingleSelect mode)", () => { + it("Select/deselect items (Single mode)", () => { cy.mount(html`Open Menu - - + + `); @@ -615,8 +613,8 @@ describe("Menu interaction", () => { .find("[text='Item 2']") .shadow() .find("[part='content']") - .find("[part='selected']") - .should("not.exist"); + .find(".ui5-menu-item-selected") + .should("exist"); cy.get("@groupSingle") .find("[text='Item 2']") @@ -629,28 +627,28 @@ describe("Menu interaction", () => { .find("[text='Item 2']") .shadow() .find("[part='content']") - .find("[part='selected']") - .should("exist"); + .find(".ui5-menu-item-selected") + .should("not.exist"); cy.get("@groupSingle") .find("[text='Item 3']") .ui5MenuItemClick(); + cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + cy.get("@groupSingle") .find("[text='Item 2']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("not.exist"); - cy.get("@menu") - .ui5MenuOpen({ opener: "btnOpen" }); - cy.get("@groupSingle") .find("[text='Item 3']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("exist"); cy.get("@groupSingle") @@ -664,16 +662,16 @@ describe("Menu interaction", () => { .find("[text='Item 3']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("not.exist"); }); - it("Select/deselect items (MultiSelect mode) ", () => { + it("Select/deselect items (Multiple mode) ", () => { cy.mount(html`Open Menu - - - + + + `); @@ -688,32 +686,49 @@ describe("Menu interaction", () => { .find("[text='Item 4']") .ui5MenuItemClick(); + cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + cy.get("@groupMulti") .find("[text='Item 4']") .shadow() .find("[part='content']") - .find("[part='selected']") - .should("exist"); - - cy.get("@menu") - .ui5MenuOpen({ opener: "btnOpen" }); + .find(".ui5-menu-item-selected") + .should("not.exist"); cy.get("@groupMulti") .find("[text='Item 5']") .ui5MenuItemClick(); + cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + cy.get("@groupMulti") .find("[text='Item 4']") .shadow() .find("[part='content']") - .find("[part='selected']") - .should("exist"); + .find(".ui5-menu-item-selected") + .should("not.exist"); cy.get("@groupMulti") .find("[text='Item 5']") .shadow() .find("[part='content']") - .find("[part='selected']") + .find(".ui5-menu-item-selected") + .should("not.exist"); + + cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + + cy.get("@groupMulti") + .find("[text='Item 4']") + .ui5MenuItemClick(); + + cy.get("@groupMulti") + .find("[text='Item 4']") + .shadow() + .find("[part='content']") + .find(".ui5-menu-item-selected") .should("exist"); cy.get("@menu") @@ -727,10 +742,9 @@ describe("Menu interaction", () => { .find("[text='Item 5']") .shadow() .find("[part='content']") - .find("[part='selected']") - .should("not.exist"); + .find(".ui5-menu-item-selected") + .should("exist"); }); - */ it("Select item (None mode) ", () => { cy.mount(html`Open Menu @@ -750,7 +764,7 @@ describe("Menu interaction", () => { cy.get("@menu") .find("[text='Item 6']") .shadow() - .find("[part='selected']") + .find(".ui5-menu-item-selected") .should("not.exist"); }); @@ -759,11 +773,11 @@ describe("Menu interaction", () => { cy.mount(html`Open Menu - + - + diff --git a/packages/main/cypress/support/commands/Menu.commands.ts b/packages/main/cypress/support/commands/Menu.commands.ts index 247ffff977cd..0ad3f66dc46c 100644 --- a/packages/main/cypress/support/commands/Menu.commands.ts +++ b/packages/main/cypress/support/commands/Menu.commands.ts @@ -28,7 +28,7 @@ Cypress.Commands.add("ui5MenuOpened", { prevSubject: true }, subject => { .should("have.attr", "open"); cy.get("@menu") - .find(" > [ui5-menu-item]") + .find(" > [ui5-menu-item], > [ui5-menu-item-group]") .first() .should("be.visible"); }); diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 6df3a1759ce6..4fe682a7701e 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -267,7 +267,7 @@ class Menu extends UI5Element { } get _list() { - return this.shadowRoot!.querySelector("[ui5-list]")!; + return this.shadowRoot!.querySelector("[ui5-list]"); } get _menuItemGroups() { @@ -389,10 +389,6 @@ class Menu extends UI5Element { }, MENU_OPEN_DELAY); } - _isGroupParent(parent: MenuItem | Menu | MenuItemGroup): boolean { - return parent && "isGroup" in parent && parent.isGroup; - } - _itemClick(e: CustomEvent) { const item = e.detail.item as MenuItem; const prevSelected = item.isSelected; diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 5180e0983dfd..67804ce41413 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -66,9 +66,12 @@ type MenuItemAccessibilityAttributes = Pick("[ui5-list]")!; } @@ -333,21 +332,12 @@ class MenuItem extends ListItem implements IMenuItem { onBeforeRendering() { const siblingsWithIcon = this._menuItemsAll.some(menuItem => !!menuItem.icon); - const itemSelectionMode = this._itemSelectionMode; this._setupItemNavigation(); this._menuItemsAll.forEach(item => { item._siblingsWithIcon = siblingsWithIcon; }); - - if (itemSelectionMode === ItemSelectionMode.None) { - return; - } - if (itemSelectionMode === ItemSelectionMode.SingleSelect && this.isSelected) { - this._parentGroup?._clearSelectedItems(); - this.isSelected = true; - } } _setupItemNavigation() { @@ -362,9 +352,9 @@ class MenuItem extends ListItem implements IMenuItem { get _role() { switch (this._itemSelectionMode) { - case ItemSelectionMode.SingleSelect: + case ItemSelectionMode.Single: return "menuitemradio"; - case ItemSelectionMode.MultiSelect: + case ItemSelectionMode.Multiple: return "menuitemcheckbox"; default: return "menuitem"; @@ -414,7 +404,7 @@ class MenuItem extends ListItem implements IMenuItem { this._popover.open = false; } this.selected = false; - this.fireDecoratorEvent("close-menu"); + this.fireEvent("close-menu", {}); } _close() { @@ -434,7 +424,7 @@ class MenuItem extends ListItem implements IMenuItem { _afterPopoverOpen() { this._menuItemsAll[0]?.focus(); - this.fireDecoratorEvent("open"); + this.fireEvent("open", {}, false, false); } _beforePopoverClose(e: CustomEvent) { @@ -449,7 +439,7 @@ class MenuItem extends ListItem implements IMenuItem { if (e.detail.escPressed) { this.focus(); if (isPhone()) { - this.fireDecoratorEvent("close-menu"); + this.fireEvent("close-menu", {}); } } } diff --git a/packages/main/src/MenuItemGroup.ts b/packages/main/src/MenuItemGroup.ts index 2cc2c21c9e09..750307379094 100644 --- a/packages/main/src/MenuItemGroup.ts +++ b/packages/main/src/MenuItemGroup.ts @@ -16,7 +16,9 @@ import type { IMenuItem } from "./Menu.js"; * `ui5-menu-item-group` is the group of items to use inside a `ui5-menu`. * Items that belong to the same group should be enclosed by `ui5-menu-item-group`. * Each group can have itemSelectionMode property, which defines the selection mode of the items inside. - * Possible values are 'None' (default), 'SingleSelect', 'MultiSelect'. + * Possible values are 'None' (default), 'Single', 'Multiple'. + * **Note:** If the itemSelectionMode property is set to 'Single', only one item can be selected at a time. + * If there is more than one item is marked as selected, the last one would be considered as the selected one. * * ### Usage * @@ -29,7 +31,7 @@ import type { IMenuItem } from "./Menu.js"; * @constructor * @extends UI5Element * @implements {IMenuItem} - * @since 2.4.0 + * @since 2.5.0 * @public */ @customElement({ @@ -45,7 +47,7 @@ class MenuItemGroup extends UI5Element implements IMenuItem { * @public */ @property() - itemSelectionMode: `${ItemSelectionMode}` = ItemSelectionMode.None; + itemSelectionMode: `${ItemSelectionMode}` = "None"; /** * Defines the items of this component. @@ -63,7 +65,6 @@ class MenuItemGroup extends UI5Element implements IMenuItem { return true; } - // Return only the menu items, excluding separators get _menuItems() { return this.items.filter((item) : item is MenuItem => !item.isSeparator); } @@ -71,7 +72,7 @@ class MenuItemGroup extends UI5Element implements IMenuItem { onBeforeRendering() { this._updateItemsSelectionMode(); - if (this.itemSelectionMode === ItemSelectionMode.SingleSelect) { + if (this.itemSelectionMode === ItemSelectionMode.Single) { this._ensureSingleSelection(); } } @@ -91,9 +92,9 @@ class MenuItemGroup extends UI5Element implements IMenuItem { * @private */ _clearSelectedItems() { - this.items.forEach((item: IMenuItem) => { + this._menuItems.forEach((item: MenuItem) => { if (!item.isSeparator && !item.isGroup) { - item.isSelected = false; + item._isSelected = false; } }); } @@ -103,20 +104,20 @@ class MenuItemGroup extends UI5Element implements IMenuItem { * @private */ _ensureSingleSelection() { - const lastSelectedItem = this.items.findLast((item: IMenuItem) => item.isSelected); + const lastSelectedItem = this._menuItems.findLast((item: MenuItem) => item._isSelected); this._clearSelectedItems(); if (lastSelectedItem) { - lastSelectedItem.isSelected = true; + lastSelectedItem._isSelected = true; } } /** - * Handles the selection of an item in the group and unselects other items if the item selection mode is SingleSelect. + * Handles the selection of an item in the group and unselects other items if the item selection mode is Single. * @private */ _handleItemSelection() { - if (this.itemSelectionMode === ItemSelectionMode.SingleSelect) { + if (this.itemSelectionMode === ItemSelectionMode.Single) { this._clearSelectedItems(); } } diff --git a/packages/main/src/types/ItemSelectionMode.ts b/packages/main/src/types/ItemSelectionMode.ts index ff5fe03b27e4..bd641763109a 100644 --- a/packages/main/src/types/ItemSelectionMode.ts +++ b/packages/main/src/types/ItemSelectionMode.ts @@ -1,5 +1,6 @@ /** * Menu item selection modes. + * @since 2.5.0 * @public */ enum ItemSelectionMode { @@ -13,13 +14,13 @@ enum ItemSelectionMode { * Single item selection mode * @public */ - SingleSelect = "SingleSelect", + Single = "Single", /** * Multiple items selection mode * @public */ - MultiSelect = "MultiSelect", + Multiple = "Multiple", } export default ItemSelectionMode; diff --git a/packages/main/test/pages/Menu.html b/packages/main/test/pages/Menu.html index 669daa11f489..c93ff8562023 100644 --- a/packages/main/test/pages/Menu.html +++ b/packages/main/test/pages/Menu.html @@ -119,20 +119,20 @@ - + - + - + @@ -141,7 +141,7 @@ - + @@ -149,10 +149,10 @@ - + - - + + From be391a09a5fdac34a61886a0d070715dd82ebb43 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Thu, 21 Nov 2024 17:50:19 +0200 Subject: [PATCH 10/22] feat(ui5-menu): fix failing tests --- packages/main/cypress/specs/Menu.cy.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/main/cypress/specs/Menu.cy.ts b/packages/main/cypress/specs/Menu.cy.ts index 5a737ab0f218..ecfae8cdb6fe 100644 --- a/packages/main/cypress/specs/Menu.cy.ts +++ b/packages/main/cypress/specs/Menu.cy.ts @@ -595,7 +595,7 @@ describe("Menu interaction", () => { it("Select/deselect items (Single mode)", () => { cy.mount(html`Open Menu - + @@ -605,6 +605,9 @@ describe("Menu interaction", () => { cy.get("[ui5-menu]") .as("menu"); + cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + cy.get("@menu") .find("[id='groupSingle']") .as("groupSingle"); @@ -668,7 +671,7 @@ describe("Menu interaction", () => { it("Select/deselect items (Multiple mode) ", () => { cy.mount(html`Open Menu - + @@ -678,6 +681,9 @@ describe("Menu interaction", () => { cy.get("[ui5-menu]") .as("menu"); + cy.get("@menu") + .ui5MenuOpen({ opener: "btnOpen" }); + cy.get("@menu") .find("[id='groupMulti']") .as("groupMulti"); From f7f0330c743a91071ea2c89e3efe9222c97959d3 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Tue, 3 Dec 2024 13:44:31 +0200 Subject: [PATCH 11/22] feat(ui5-menu): fix comments --- packages/main/src/Menu.ts | 28 ++++++------ packages/main/src/MenuItem.ts | 80 ++++++++++++++++++----------------- 2 files changed, 58 insertions(+), 50 deletions(-) diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 4fe682a7701e..9b94a917287a 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -270,15 +270,22 @@ class Menu extends UI5Element { return this.shadowRoot!.querySelector("[ui5-list]"); } + get acessibleNameText() { + return Menu.i18nBundle.getText(MENU_POPOVER_ACCESSIBLE_NAME); + } + + /** Returns menu item groups */ get _menuItemGroups() { return this.items.filter((item) : item is MenuItemGroup => item.isGroup); } + /** Returns menu items */ get _menuItems() { return this.items.filter((item) : item is MenuItem => !item.isSeparator && !item.isGroup); } - get _menuItemsAll() { + /** Returns all menu items (including those in groups */ + get _allMenuItems() { const items: Array = []; this.items.forEach(item => { @@ -292,7 +299,8 @@ class Menu extends UI5Element { return items; } - get _menuItemsNavigation() { + /** Returns menu items included in the ItemNavigation */ + get _navigatableMenuItems() { const items: Array = []; const slottedItems = this.getSlottedNodes("items"); @@ -308,23 +316,19 @@ class Menu extends UI5Element { return items; } - get acessibleNameText() { - return Menu.i18nBundle.getText(MENU_POPOVER_ACCESSIBLE_NAME); - } - onBeforeRendering() { - const siblingsWithIcon = this._menuItemsAll.some(menuItem => !!menuItem.icon); + const siblingsWithIcon = this._allMenuItems.some(menuItem => !!menuItem.icon); this._setupItemNavigation(); - this._menuItemsAll.forEach(item => { + this._allMenuItems.forEach(item => { item._siblingsWithIcon = siblingsWithIcon; }); } _setupItemNavigation() { if (this._list) { - this._list._itemNavigation._getItems = () => this._menuItemsNavigation; + this._list._itemNavigation._getItems = () => this._navigatableMenuItems; } } @@ -350,7 +354,7 @@ class Menu extends UI5Element { _closeItemSubMenu(item: MenuItem) { if (item && item._popover) { - const openedSibling = item._menuItemsAll.find(menuItem => menuItem._popover && menuItem._popover.open); + const openedSibling = item._allMenuItems.find(menuItem => menuItem._popover && menuItem._popover.open); if (openedSibling) { this._closeItemSubMenu(openedSibling); } @@ -379,7 +383,7 @@ class Menu extends UI5Element { this._timeout = setTimeout(() => { const opener = item.parentElement as MenuItem | Menu; - const menuItems = opener._menuItemsAll; + const menuItems = opener._allMenuItems; const openedSibling = opener && menuItems && menuItems.find(menuItem => menuItem._popover && menuItem._popover.open); if (openedSibling) { this._closeItemSubMenu(openedSibling); @@ -441,7 +445,7 @@ class Menu extends UI5Element { } _afterPopoverOpen() { - this._menuItemsAll[0]?.focus(); + this._allMenuItems[0]?.focus(); this.fireEvent("open", {}, false, true); } diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 67804ce41413..57c39c5f506d 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -310,42 +310,6 @@ class MenuItem extends ListItem implements IMenuItem { return this.shadowRoot!.querySelector("[ui5-list]")!; } - get _menuItemsNavigation() { - const items: MenuItem[] = []; - const slottedItems = this.getSlottedNodes("items"); - - slottedItems.forEach(item => { - if (item.isGroup) { - const groupItems = item.getSlottedNodes("items"); - items.push(...groupItems); - } else if (!item.isSeparator) { - items.push(item); - } - }); - - return items; - } - - get _markSelected() { - return this.isSelected && this._itemSelectionMode !== ItemSelectionMode.None; - } - - onBeforeRendering() { - const siblingsWithIcon = this._menuItemsAll.some(menuItem => !!menuItem.icon); - - this._setupItemNavigation(); - - this._menuItemsAll.forEach(item => { - item._siblingsWithIcon = siblingsWithIcon; - }); - } - - _setupItemNavigation() { - if (this._list) { - this._list._itemNavigation._getItems = () => this._menuItemsNavigation; - } - } - get _focusable() { return true; } @@ -377,15 +341,22 @@ class MenuItem extends ListItem implements IMenuItem { return this.shadowRoot!.querySelector("[ui5-responsive-popover]")!; } + get _markSelected() { + return this.isSelected && this._itemSelectionMode !== ItemSelectionMode.None; + } + + /** Returns menu item groups */ get _menuItemGroups() { return this.items.filter((item) : item is MenuItemGroup => item.isGroup); } + /** Returns menu items */ get _menuItems() { return this.items.filter((item): item is MenuItem => !item.isSeparator); } - get _menuItemsAll() { + /** Returns all menu items (including those in groups */ + get _allMenuItems() { const items: MenuItem[] = []; this.items.forEach(item => { @@ -399,6 +370,39 @@ class MenuItem extends ListItem implements IMenuItem { return items; } + /** Returns menu items included in the ItemNavigation */ + get _navigatableMenuItems() { + const items: MenuItem[] = []; + const slottedItems = this.getSlottedNodes("items"); + + slottedItems.forEach(item => { + if (item.isGroup) { + const groupItems = item.getSlottedNodes("items"); + items.push(...groupItems); + } else if (!item.isSeparator) { + items.push(item); + } + }); + + return items; + } + + onBeforeRendering() { + const siblingsWithIcon = this._allMenuItems.some(menuItem => !!menuItem.icon); + + this._setupItemNavigation(); + + this._allMenuItems.forEach(item => { + item._siblingsWithIcon = siblingsWithIcon; + }); + } + + _setupItemNavigation() { + if (this._list) { + this._list._itemNavigation._getItems = () => this._navigatableMenuItems; + } + } + _closeAll() { if (this._popover) { this._popover.open = false; @@ -423,7 +427,7 @@ class MenuItem extends ListItem implements IMenuItem { } _afterPopoverOpen() { - this._menuItemsAll[0]?.focus(); + this._allMenuItems[0]?.focus(); this.fireEvent("open", {}, false, false); } From c39cfd2a864b3ae770e7c189dcfe095110332a0b Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Wed, 7 May 2025 15:02:44 +0300 Subject: [PATCH 12/22] feat(ui5-menu): fix merge conflicts --- packages/main/cypress/support/commands/Menu.commands.ts | 9 --------- packages/main/src/MenuItemGroup.hbs | 6 ------ 2 files changed, 15 deletions(-) delete mode 100644 packages/main/src/MenuItemGroup.hbs diff --git a/packages/main/cypress/support/commands/Menu.commands.ts b/packages/main/cypress/support/commands/Menu.commands.ts index c755d7f9e3df..587cf7a0dd87 100644 --- a/packages/main/cypress/support/commands/Menu.commands.ts +++ b/packages/main/cypress/support/commands/Menu.commands.ts @@ -25,21 +25,12 @@ Cypress.Commands.add("ui5MenuOpened", { prevSubject: true }, subject => { cy.get("@menu") .shadow() .find("[ui5-responsive-popover]") -<<<<<<< HEAD - .should("have.attr", "open"); - - cy.get("@menu") - .find(" > [ui5-menu-item], > [ui5-menu-item-group]") - .first() - .should("be.visible"); -======= .should($rp => { expect($rp.is(":popover-open")).to.be.true; expect($rp.width()).to.not.equal(0); expect($rp.height()).to.not.equal(0); }) .and("have.attr", "open"); ->>>>>>> main }); Cypress.Commands.add("ui5MenuItemClick", { prevSubject: true }, subject => { diff --git a/packages/main/src/MenuItemGroup.hbs b/packages/main/src/MenuItemGroup.hbs deleted file mode 100644 index 163afe1bd59b..000000000000 --- a/packages/main/src/MenuItemGroup.hbs +++ /dev/null @@ -1,6 +0,0 @@ -
- -
\ No newline at end of file From 2ed057e7e687d7ac77fea25d616a5ec945211ee1 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Wed, 7 May 2025 15:13:43 +0300 Subject: [PATCH 13/22] feat(ui5-menu): fix lint errors --- packages/main/src/Menu.ts | 2 - packages/main/src/MenuItem.ts | 2 - packages/main/src/MenuItemGroup.ts | 2 +- packages/main/src/MenuItemGroupTemplate.tsx | 24 +++++----- packages/main/src/types/ItemCheckMode.ts | 50 ++++++++++----------- 5 files changed, 38 insertions(+), 42 deletions(-) diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 9bc9e27516de..937c12c4e2ca 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -308,8 +308,6 @@ class Menu extends UI5Element { } }); - console.warn(items); - return items; } diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index bbe5a7fca9a7..6a92ed52b78b 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -447,8 +447,6 @@ class MenuItem extends ListItem implements IMenuItem { } }); - console.warn(items); - return items; } diff --git a/packages/main/src/MenuItemGroup.ts b/packages/main/src/MenuItemGroup.ts index c9ff5b6c3075..1f12b547f695 100644 --- a/packages/main/src/MenuItemGroup.ts +++ b/packages/main/src/MenuItemGroup.ts @@ -126,4 +126,4 @@ class MenuItemGroup extends UI5Element implements IMenuItem { MenuItemGroup.define(); -export default MenuItemGroup; \ No newline at end of file +export default MenuItemGroup; diff --git a/packages/main/src/MenuItemGroupTemplate.tsx b/packages/main/src/MenuItemGroupTemplate.tsx index 8d3683d85ff1..ddc0c853bd2b 100644 --- a/packages/main/src/MenuItemGroupTemplate.tsx +++ b/packages/main/src/MenuItemGroupTemplate.tsx @@ -1,12 +1,12 @@ -import type MenuItemGroup from "./MenuItemGroup.js"; - -export default function MenuItemGroupTemplate(this: MenuItemGroup) { - return ( -
- -
- ); -} +import type MenuItemGroup from "./MenuItemGroup.js"; + +export default function MenuItemGroupTemplate(this: MenuItemGroup) { + return ( +
+ +
+ ); +} diff --git a/packages/main/src/types/ItemCheckMode.ts b/packages/main/src/types/ItemCheckMode.ts index 762eeaf43206..a40f7075541f 100644 --- a/packages/main/src/types/ItemCheckMode.ts +++ b/packages/main/src/types/ItemCheckMode.ts @@ -1,26 +1,26 @@ -/** - * Menu item group check modes. - * @since 2.11.0 - * @public - */ -enum ItemCheckMode { - /** - * default type (items in a group cannot be checked) - * @public - */ - None = "None", - - /** - * Single item check mode (only one item in a group can be checked at a time) - * @public - */ - Single = "Single", - - /** - * Multiple items check mode (multiple items in a group can be checked at a time) - * @public - */ - Multiple = "Multiple", -} - +/** + * Menu item group check modes. + * @since 2.11.0 + * @public + */ +enum ItemCheckMode { + /** + * default type (items in a group cannot be checked) + * @public + */ + None = "None", + + /** + * Single item check mode (only one item in a group can be checked at a time) + * @public + */ + Single = "Single", + + /** + * Multiple items check mode (multiple items in a group can be checked at a time) + * @public + */ + Multiple = "Multiple", +} + export default ItemCheckMode; \ No newline at end of file From 4debe933f6eec8ad373377e0ec1ce97073dec51b Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Wed, 7 May 2025 15:20:10 +0300 Subject: [PATCH 14/22] feat(ui5-menu): fix lint errors --- packages/main/src/types/ItemCheckMode.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/src/types/ItemCheckMode.ts b/packages/main/src/types/ItemCheckMode.ts index a40f7075541f..fd63904bdeb0 100644 --- a/packages/main/src/types/ItemCheckMode.ts +++ b/packages/main/src/types/ItemCheckMode.ts @@ -23,4 +23,4 @@ enum ItemCheckMode { Multiple = "Multiple", } -export default ItemCheckMode; \ No newline at end of file +export default ItemCheckMode; From 32c1976eb7be0f1630e0cd390736afe7ca8bd6d5 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Wed, 7 May 2025 16:56:18 +0300 Subject: [PATCH 15/22] feat(ui5-menu): fix failing test --- packages/main/src/Menu.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 937c12c4e2ca..822356bd547f 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -390,7 +390,7 @@ class Menu extends UI5Element { this._timeout = setTimeout(() => { const opener = item.parentElement as MenuItem | Menu; - const menuItems = opener._allMenuItems; + const menuItems = opener && opener._allMenuItems; const openedSibling = opener && menuItems && menuItems.find(menuItem => menuItem._popover && menuItem._popover.open); if (openedSibling) { From 9d7783f43b5d6e934a850613d3668aeff518a02c Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Mon, 12 May 2025 16:18:39 +0300 Subject: [PATCH 16/22] feat(ui5-menu): fix comments --- packages/fiori/src/UserMenuItem.ts | 3 +- packages/main/src/Menu.ts | 45 +++++++++++------- packages/main/src/MenuItem.ts | 33 +++++-------- packages/main/src/MenuItemGroup.ts | 36 +++++++------- packages/main/src/MenuSeparator.ts | 4 -- packages/main/src/types/ItemSelectionMode.ts | 26 ---------- packages/main/test/pages/Menu.html | 6 +-- .../docs/_components_pages/main/Menu/Menu.mdx | 7 ++- .../main/Menu/MenuItemGroup.mdx | 8 ++++ .../main/Menu/ItemGroups/ItemGroups.md | 4 ++ .../_samples/main/Menu/ItemGroups/main.js | 24 ++++++++++ .../_samples/main/Menu/ItemGroups/sample.html | 47 +++++++++++++++++++ 12 files changed, 152 insertions(+), 91 deletions(-) delete mode 100644 packages/main/src/types/ItemSelectionMode.ts create mode 100644 packages/website/docs/_components_pages/main/Menu/MenuItemGroup.mdx create mode 100644 packages/website/docs/_samples/main/Menu/ItemGroups/ItemGroups.md create mode 100644 packages/website/docs/_samples/main/Menu/ItemGroups/main.js create mode 100644 packages/website/docs/_samples/main/Menu/ItemGroups/sample.html diff --git a/packages/fiori/src/UserMenuItem.ts b/packages/fiori/src/UserMenuItem.ts index 655cd54087e5..26f2195cb4d3 100644 --- a/packages/fiori/src/UserMenuItem.ts +++ b/packages/fiori/src/UserMenuItem.ts @@ -1,5 +1,6 @@ import { customElement, slot } from "@ui5/webcomponents-base/dist/decorators.js"; import MenuItem from "@ui5/webcomponents/dist/MenuItem.js"; +import { isInstanceOfMenuItem } from "@ui5/webcomponents/dist/Menu.js"; import UserMenuItemTemplate from "./UserMenuItemTemplate.js"; @@ -45,7 +46,7 @@ class UserMenuItem extends MenuItem { declare items: Array; get _menuItems() { - return this.items.filter(item => !item.isSeparator); + return this.items.filter(isInstanceOfMenuItem); } } diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 822356bd547f..b89cbb626363 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -27,6 +27,7 @@ import type List from "./List.js"; import type ResponsivePopover from "./ResponsivePopover.js"; import type PopoverHorizontalAlign from "./types/PopoverHorizontalAlign.js"; import type MenuItem from "./MenuItem.js"; +import type MenuSeparator from "./MenuSeparator.js"; import "./MenuItem.js"; import "./MenuSeparator.js"; import type MenuItemGroup from "./MenuItemGroup.js"; @@ -51,8 +52,9 @@ const MENU_OPEN_DELAY = 300; * @public */ interface IMenuItem extends UI5Element { - isSeparator: boolean; - isGroup: boolean; + isMenuItem?: boolean; + isSeparator?: boolean; + isGroup?: boolean; } type MenuItemClickEventDetail = { @@ -271,22 +273,22 @@ class Menu extends UI5Element { /** Returns menu item groups */ get _menuItemGroups() { - return this.items.filter((item) : item is MenuItemGroup => item.isGroup); + return this.items.filter(isInstanceOfMenuItemGroup); } /** Returns menu items */ get _menuItems() { - return this.items.filter((item) : item is MenuItem => !item.isSeparator && !item.isGroup); + return this.items.filter(isInstanceOfMenuItem); } /** Returns all menu items (including those in groups */ get _allMenuItems() { - const items: Array = []; + const items: MenuItem[] = []; this.items.forEach(item => { - if (item.isGroup) { + if (isInstanceOfMenuItemGroup(item)) { items.push(...(item as MenuItemGroup)._menuItems); - } else if (!item.isSeparator) { + } else if (!isInstanceOfMenuSeparator(item)) { items.push(item as MenuItem); } }); @@ -296,14 +298,14 @@ class Menu extends UI5Element { /** Returns menu items included in the ItemNavigation */ get _navigatableMenuItems() { - const items: Array = []; + const items: MenuItem[] = []; const slottedItems = this.getSlottedNodes("items"); slottedItems.forEach(item => { - if (item.isGroup) { + if (isInstanceOfMenuItemGroup(item)) { const groupItems = item.getSlottedNodes("items"); items.push(...groupItems); - } else if (!item.isSeparator) { + } else if (!isInstanceOfMenuSeparator(item)) { items.push(item); } }); @@ -376,7 +378,7 @@ class Menu extends UI5Element { // respect mouseover only on desktop const item = e.target as MenuItem; - if (this._isInstanceOfMenuItem(item)) { + if (isInstanceOfMenuItem(item)) { item.focus(); // Opens submenu with 300ms delay @@ -426,9 +428,9 @@ class Menu extends UI5Element { const parentElement = item.parentElement as MenuItem; const shouldItemNavigation = isUp(e) || isDown(e); const shouldOpenMenu = this.isRtl ? isLeft(e) : isRight(e); - const shouldCloseMenu = !shouldItemNavigation && !shouldOpenMenu && this._isInstanceOfMenuItem(parentElement); + const shouldCloseMenu = !shouldItemNavigation && !shouldOpenMenu && isInstanceOfMenuItem(parentElement); - if (this._isInstanceOfMenuItem(item)) { + if (isInstanceOfMenuItem(item)) { if (isEnter(e) || isTabNextPrevious) { e.preventDefault(); } @@ -487,15 +489,24 @@ class Menu extends UI5Element { this.open = false; this.fireDecoratorEvent("close"); } - - _isInstanceOfMenuItem(object: any): object is MenuItem { - return "isMenuItem" in object; - } } +const isInstanceOfMenuItem = (object: any): object is MenuItem => { + return "isMenuItem" in object; +}; + +const isInstanceOfMenuSeparator = (object: any): object is MenuSeparator => { + return "isSeparator" in object; +}; + +const isInstanceOfMenuItemGroup = (object: any): object is MenuItemGroup => { + return "isGroup" in object; +}; + Menu.define(); export default Menu; +export { isInstanceOfMenuItem, isInstanceOfMenuSeparator, isInstanceOfMenuItemGroup }; export type { MenuItemClickEventDetail, MenuBeforeCloseEventDetail, diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 6a92ed52b78b..2223690f5718 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -19,6 +19,7 @@ import type List from "./List.js"; import ListItem from "./ListItem.js"; import type ResponsivePopover from "./ResponsivePopover.js"; import type PopoverPlacement from "./types/PopoverPlacement.js"; +import { isInstanceOfMenuItem, isInstanceOfMenuSeparator, isInstanceOfMenuItemGroup } from "./Menu.js"; import MenuItemTemplate from "./MenuItemTemplate.js"; import { MENU_BACK_BUTTON_ARIA_LABEL, @@ -109,8 +110,8 @@ type MenuItemAccessibilityAttributes = Pick("[ui5-list]")!; + return this.shadowRoot && this.shadowRoot.querySelector("[ui5-list]")!; } get _popover() { - return this.shadowRoot!.querySelector("[ui5-responsive-popover]")!; + return this.shadowRoot && this.shadowRoot.querySelector("[ui5-responsive-popover]")!; } get isMenuItem(): boolean { return true; } - get isGroup(): boolean { - return false; - } - - get isSeparator(): boolean { - return false; - } - get isRtl() { return this.effectiveDir === "rtl"; } @@ -410,12 +403,12 @@ class MenuItem extends ListItem implements IMenuItem { /** Returns menu item groups */ get _menuItemGroups() { - return this.items.filter((item) : item is MenuItemGroup => item.isGroup); + return this.items.filter(isInstanceOfMenuItemGroup); } /** Returns menu items */ get _menuItems() { - return this.items.filter((item): item is MenuItem => !item.isSeparator); + return this.items.filter(isInstanceOfMenuItem); } /** Returns all menu items (including those in groups */ @@ -423,9 +416,9 @@ class MenuItem extends ListItem implements IMenuItem { const items: MenuItem[] = []; this.items.forEach(item => { - if (item.isGroup) { + if (isInstanceOfMenuItemGroup(item)) { items.push(...(item as MenuItemGroup)._menuItems); - } else if (!item.isSeparator) { + } else if (!isInstanceOfMenuSeparator(item)) { items.push(item as MenuItem); } }); @@ -439,10 +432,10 @@ class MenuItem extends ListItem implements IMenuItem { const slottedItems = this.getSlottedNodes("items"); slottedItems.forEach(item => { - if (item.isGroup) { + if (isInstanceOfMenuItemGroup(item)) { const groupItems = item.getSlottedNodes("items"); items.push(...groupItems); - } else if (!item.isSeparator) { + } else if (!isInstanceOfMenuSeparator(item)) { items.push(item); } }); diff --git a/packages/main/src/MenuItemGroup.ts b/packages/main/src/MenuItemGroup.ts index 1f12b547f695..8447634014a9 100644 --- a/packages/main/src/MenuItemGroup.ts +++ b/packages/main/src/MenuItemGroup.ts @@ -5,6 +5,7 @@ import slot from "@ui5/webcomponents-base/dist/decorators/slot.js"; import jsxRenderer from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js"; import MenuItem from "./MenuItem.js"; import MenuItemGroupTemplate from "./MenuItemGroupTemplate.js"; +import { isInstanceOfMenuItem } from "./Menu.js"; import ItemCheckMode from "./types/ItemCheckMode.js"; import type { IMenuItem } from "./Menu.js"; @@ -13,12 +14,16 @@ import type { IMenuItem } from "./Menu.js"; * * ### Overview * - * `ui5-menu-item-group` is the group of items to use inside a `ui5-menu`. - * Items that belong to the same group should be enclosed by `ui5-menu-item-group`. - * Each group can have itemCheckMode property, which defines the check mode of the items inside. - * Possible values are 'None' (default), 'Single', 'Multiple'. - * **Note:** If the itemCheckMode property is set to 'Single', only one item can be checked at a time. - * If there is more than one item is marked as checked, the last one would be considered as the checked one. + * The `ui5-menu-item-group` component represents a group of items designed for use inside a `ui5-menu`. + * Items belonging to the same group should be wrapped by a `ui5-menu-item-group`. + * Each group can have an `itemCheckMode` property, which defines the check mode for the items within the group. + * The possible values for `itemCheckMode` are: + * - 'None' (default) - no items can be checked + * - 'Single' - Only one item can be checked at a time + * - 'Multiple' - Multiple items can be checked simultaneously + * + * **Note:** If the `itemCheckMode` property is set to 'Single', only one item can remain checked at any given time. + * If multiple items are marked as checked, the last checked item will take precedence. * * ### Usage * @@ -42,7 +47,7 @@ import type { IMenuItem } from "./Menu.js"; }) class MenuItemGroup extends UI5Element implements IMenuItem { /** - * Defines the component check mode. + * Defines the component's check mode. * @default "None" * @public */ @@ -51,22 +56,18 @@ class MenuItemGroup extends UI5Element implements IMenuItem { /** * Defines the items of this component. - * **Note:** The slot can hold `ui5-menu-item` and `ui5-menu-separator` items. + * **Note:** The slot can hold any combination of components of type `ui5-menu-item` or `ui5-menu-separator` or both. * @public */ @slot({ "default": true, type: HTMLElement, invalidateOnChildChange: true }) items!: Array; - get isSeparator(): boolean { - return false; - } - get isGroup(): boolean { return true; } get _menuItems() { - return this.items.filter((item) : item is MenuItem => !item.isSeparator); + return this.items.filter(isInstanceOfMenuItem); } onBeforeRendering() { @@ -92,15 +93,12 @@ class MenuItemGroup extends UI5Element implements IMenuItem { * @private */ _clearCheckedItems() { - this._menuItems.forEach((item: MenuItem) => { - if (!item.isSeparator && !item.isGroup) { - item.checked = false; - } - }); + this._menuItems.forEach((item: MenuItem) => item.checked = false); } /** - * Ensures that only one item is checked in the group (if there were any checked items). + * Ensures that only one item can remain checked at any given time. If multiple items are marked as checked, + * the last checked item will take precedence. * @private */ _ensureSingleItemIsChecked() { diff --git a/packages/main/src/MenuSeparator.ts b/packages/main/src/MenuSeparator.ts index 82fe8a23e732..8cfce33c26c5 100644 --- a/packages/main/src/MenuSeparator.ts +++ b/packages/main/src/MenuSeparator.ts @@ -28,10 +28,6 @@ class MenuSeparator extends ListItemBase implements IMenuItem { return true; } - get isGroup() { - return false; - } - get classes(): ClassMap { return { main: { diff --git a/packages/main/src/types/ItemSelectionMode.ts b/packages/main/src/types/ItemSelectionMode.ts deleted file mode 100644 index bd641763109a..000000000000 --- a/packages/main/src/types/ItemSelectionMode.ts +++ /dev/null @@ -1,26 +0,0 @@ -/** - * Menu item selection modes. - * @since 2.5.0 - * @public - */ -enum ItemSelectionMode { - /** - * default type (no item selection) - * @public - */ - None = "None", - - /** - * Single item selection mode - * @public - */ - Single = "Single", - - /** - * Multiple items selection mode - * @public - */ - Multiple = "Multiple", -} - -export default ItemSelectionMode; diff --git a/packages/main/test/pages/Menu.html b/packages/main/test/pages/Menu.html index cc10a8f02d6c..f8097f27e446 100644 --- a/packages/main/test/pages/Menu.html +++ b/packages/main/test/pages/Menu.html @@ -84,7 +84,7 @@ Menu with items that have endContent - Open Menu + Open Menu

@@ -117,7 +117,7 @@ Menu with item groups - Open Menu + Open Menu

@@ -161,7 +161,7 @@ -

+
For more information: diff --git a/packages/website/docs/_components_pages/main/Menu/Menu.mdx b/packages/website/docs/_components_pages/main/Menu/Menu.mdx index 6bafaf0a0018..0654df0045c8 100644 --- a/packages/website/docs/_components_pages/main/Menu/Menu.mdx +++ b/packages/website/docs/_components_pages/main/Menu/Menu.mdx @@ -7,6 +7,7 @@ import SubMenu from "../../../_samples/main/Menu/SubMenu/SubMenu.md"; import AditionalText from "../../../_samples/main/Menu/AditionalText/AditionalText.md"; import MenuEndContent from "../../../_samples/main/Menu/MenuEndContent/MenuEndContent.md"; import DynamicallyAddedItems from "../../../_samples/main/Menu/DynamicallyAddedItems/DynamicallyAddedItems.md"; +import ItemGroups from "../../../_samples/main/Menu/ItemGroups/ItemGroups.md"; <%COMPONENT_OVERVIEW%> @@ -32,4 +33,8 @@ You can nest menu items to create sub menus. ### Handling focus behaviour when adding items dynamically - \ No newline at end of file + + +### Item Groups with Checkable Menu Items + + \ No newline at end of file diff --git a/packages/website/docs/_components_pages/main/Menu/MenuItemGroup.mdx b/packages/website/docs/_components_pages/main/Menu/MenuItemGroup.mdx new file mode 100644 index 000000000000..79c54601029f --- /dev/null +++ b/packages/website/docs/_components_pages/main/Menu/MenuItemGroup.mdx @@ -0,0 +1,8 @@ +--- +slug: ../../MenuItemGroup +sidebar_class_name: newComponentBadge +--- + +<%COMPONENT_OVERVIEW%> + +<%COMPONENT_METADATA%> \ No newline at end of file diff --git a/packages/website/docs/_samples/main/Menu/ItemGroups/ItemGroups.md b/packages/website/docs/_samples/main/Menu/ItemGroups/ItemGroups.md new file mode 100644 index 000000000000..17798ecc59ab --- /dev/null +++ b/packages/website/docs/_samples/main/Menu/ItemGroups/ItemGroups.md @@ -0,0 +1,4 @@ +import html from '!!raw-loader!./sample.html'; +import js from '!!raw-loader!./main.js'; + + diff --git a/packages/website/docs/_samples/main/Menu/ItemGroups/main.js b/packages/website/docs/_samples/main/Menu/ItemGroups/main.js new file mode 100644 index 000000000000..543f38ffd20a --- /dev/null +++ b/packages/website/docs/_samples/main/Menu/ItemGroups/main.js @@ -0,0 +1,24 @@ +import "@ui5/webcomponents/dist/Menu.js"; +import "@ui5/webcomponents/dist/MenuItem.js"; +import "@ui5/webcomponents/dist/MenuSeparator.js"; +import "@ui5/webcomponents/dist/MenuItemGroup.js"; +import "@ui5/webcomponents/dist/Button.js"; + +import "@ui5/webcomponents-icons/dist/add-document.js"; +import "@ui5/webcomponents-icons/dist/slim-arrow-down.js"; +import "@ui5/webcomponents-icons/dist/text-align-left.js"; +import "@ui5/webcomponents-icons/dist/text-align-center.js"; +import "@ui5/webcomponents-icons/dist/text-align-right.js"; +import "@ui5/webcomponents-icons/dist/bold-text.js"; +import "@ui5/webcomponents-icons/dist/italic-text.js"; +import "@ui5/webcomponents-icons/dist/underline-text.js"; +import "@ui5/webcomponents-icons/dist/locked.js"; + +const btnOpenGroups = document.getElementById("btnOpenGroups"); +const menuGroups = document.getElementById("menuGroups"); + +btnOpenGroups.addEventListener("click", function(event) { + menuGroups.opener = btnOpenGroups; + menuGroups.open = !menuGroups.open; +}); + diff --git a/packages/website/docs/_samples/main/Menu/ItemGroups/sample.html b/packages/website/docs/_samples/main/Menu/ItemGroups/sample.html new file mode 100644 index 000000000000..917d9da24252 --- /dev/null +++ b/packages/website/docs/_samples/main/Menu/ItemGroups/sample.html @@ -0,0 +1,47 @@ + + + + + + + + Sample + + + + + + Open Menu + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 4bb9881550e80deff21dc384873a4bd85a4462bd Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Mon, 12 May 2025 16:49:29 +0300 Subject: [PATCH 17/22] feat(ui5-menu): fix lint errors --- packages/main/src/Menu.ts | 2 +- packages/main/src/MenuItem.ts | 3 +-- packages/main/src/MenuItemGroup.ts | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index b89cbb626363..d01b7967c230 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -287,7 +287,7 @@ class Menu extends UI5Element { this.items.forEach(item => { if (isInstanceOfMenuItemGroup(item)) { - items.push(...(item as MenuItemGroup)._menuItems); + items.push(...item._menuItems); } else if (!isInstanceOfMenuSeparator(item)) { items.push(item as MenuItem); } diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 2223690f5718..687830754e80 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -12,7 +12,6 @@ import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js"; import NavigationMode from "@ui5/webcomponents-base/dist/types/NavigationMode.js"; import ItemNavigation from "@ui5/webcomponents-base/dist/delegate/ItemNavigation.js"; import ItemNavigationBehavior from "@ui5/webcomponents-base/dist/types/ItemNavigationBehavior.js"; -import type MenuItemGroup from "./MenuItemGroup.js"; import ItemCheckMode from "./types/ItemCheckMode.js"; import type { ListItemAccessibilityAttributes } from "./ListItem.js"; import type List from "./List.js"; @@ -417,7 +416,7 @@ class MenuItem extends ListItem implements IMenuItem { this.items.forEach(item => { if (isInstanceOfMenuItemGroup(item)) { - items.push(...(item as MenuItemGroup)._menuItems); + items.push(...item._menuItems); } else if (!isInstanceOfMenuSeparator(item)) { items.push(item as MenuItem); } diff --git a/packages/main/src/MenuItemGroup.ts b/packages/main/src/MenuItemGroup.ts index 8447634014a9..bdd4bae03729 100644 --- a/packages/main/src/MenuItemGroup.ts +++ b/packages/main/src/MenuItemGroup.ts @@ -93,7 +93,7 @@ class MenuItemGroup extends UI5Element implements IMenuItem { * @private */ _clearCheckedItems() { - this._menuItems.forEach((item: MenuItem) => item.checked = false); + this._menuItems.forEach((item: MenuItem) => { item.checked = false; }); } /** From 78b8356b590ecebd30a4496c09097e152af8cf3c Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Wed, 14 May 2025 11:43:40 +0300 Subject: [PATCH 18/22] feat(ui5-menu): merge conflicts after another PR is merged and adjust functionality --- packages/fiori/src/UserMenuItem.ts | 2 +- packages/main/src/Menu.ts | 7 ++----- packages/main/src/MenuItem.ts | 8 ++++---- packages/main/src/MenuItemGroup.ts | 13 ++++++++++--- packages/main/src/MenuSeparator.ts | 9 +++++++++ packages/main/test/pages/Menu.html | 2 +- 6 files changed, 27 insertions(+), 14 deletions(-) diff --git a/packages/fiori/src/UserMenuItem.ts b/packages/fiori/src/UserMenuItem.ts index 26f2195cb4d3..ec315f94a4ed 100644 --- a/packages/fiori/src/UserMenuItem.ts +++ b/packages/fiori/src/UserMenuItem.ts @@ -1,6 +1,6 @@ import { customElement, slot } from "@ui5/webcomponents-base/dist/decorators.js"; import MenuItem from "@ui5/webcomponents/dist/MenuItem.js"; -import { isInstanceOfMenuItem } from "@ui5/webcomponents/dist/Menu.js"; +import { isInstanceOfMenuItem } from "@ui5/webcomponents/dist/MenuItem.js"; import UserMenuItemTemplate from "./UserMenuItemTemplate.js"; diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index a0fcf3b6278c..3343f4c2c711 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -27,14 +27,11 @@ import DOMReferenceConverter from "@ui5/webcomponents-base/dist/converters/DOMRe import type List from "./List.js"; import type ResponsivePopover from "./ResponsivePopover.js"; import type MenuItem from "./MenuItem.js"; -import type MenuSeparator from "./MenuSeparator.js"; // The import below should be kept, as MenuItem is part of the Menu component. import { isInstanceOfMenuItem } from "./MenuItem.js"; import { isInstanceOfMenuItemGroup } from "./MenuItemGroup.js"; import { isInstanceOfMenuSeparator } from "./MenuSeparator.js"; import type PopoverHorizontalAlign from "./types/PopoverHorizontalAlign.js"; -import "./MenuSeparator.js"; -import type MenuItemGroup from "./MenuItemGroup.js"; import type { ListItemClickEventDetail, } from "./List.js"; @@ -443,7 +440,7 @@ class Menu extends UI5Element { return; } - const menuItemInMenu = this._menuItems.includes(item); + const menuItemInMenu = this._allMenuItems.includes(item); const isItemNavigation = isUp(e) || isDown(e); const isItemSelection = isEnter(e) || isSpace(e); const isEndContentNavigation = isRight(e) || isLeft(e); @@ -468,7 +465,7 @@ class Menu extends UI5Element { _navigateOutOfEndContent(e: CustomEvent) { const item = e.target as MenuItem; const shouldNavigateToNextItem = e.detail.shouldNavigateToNextItem; - const menuItems = this._menuItems; + const menuItems = this._allMenuItems; const itemIndex = menuItems.indexOf(item); if (itemIndex > -1) { diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 77bae4d03396..250900f2b432 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -480,7 +480,7 @@ class MenuItem extends ListItem implements IMenuItem { await renderFinished(); if (this.hasSubmenu && this.isSubMenuOpen) { - const menuItems = this._menuItems; + const menuItems = this._allMenuItems; return menuItems[0] && menuItems[0].focus(focusOptions); } @@ -534,7 +534,7 @@ class MenuItem extends ListItem implements IMenuItem { _itemKeyDown(e: KeyboardEvent) { const item = e.target as MenuItem; - const itemInMenuItems = this._menuItems.includes(item); + const itemInMenuItems = this._allMenuItems.includes(item); const isTabNextPrevious = isTabNext(e) || isTabPrevious(e); const isItemNavigation = isUp(e) || isDown(e); const isItemSelection = isSpace(e) || isEnter(e); @@ -559,7 +559,7 @@ class MenuItem extends ListItem implements IMenuItem { _navigateOutOfEndContent(e: CustomEvent) { const item = e.target as MenuItem; const shouldNavigateToNextItem = e.detail.shouldNavigateToNextItem; - const menuItems = this._menuItems; + const menuItems = this._allMenuItems; const itemIndex = menuItems.indexOf(item); if (itemIndex > -1) { @@ -582,7 +582,7 @@ class MenuItem extends ListItem implements IMenuItem { _close() { if (this._popover) { this._popover.open = false; - this._menuItems.forEach(item => item._close()); + this._allMenuItems.forEach(item => item._close()); } this.selected = false; } diff --git a/packages/main/src/MenuItemGroup.ts b/packages/main/src/MenuItemGroup.ts index bdd4bae03729..e961b1f144cf 100644 --- a/packages/main/src/MenuItemGroup.ts +++ b/packages/main/src/MenuItemGroup.ts @@ -3,9 +3,9 @@ import customElement from "@ui5/webcomponents-base/dist/decorators/customElement import property from "@ui5/webcomponents-base/dist/decorators/property.js"; import slot from "@ui5/webcomponents-base/dist/decorators/slot.js"; import jsxRenderer from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js"; -import MenuItem from "./MenuItem.js"; +import type MenuItem from "./MenuItem.js"; import MenuItemGroupTemplate from "./MenuItemGroupTemplate.js"; -import { isInstanceOfMenuItem } from "./Menu.js"; +import { isInstanceOfMenuItem } from "./MenuItem.js"; import ItemCheckMode from "./types/ItemCheckMode.js"; import type { IMenuItem } from "./Menu.js"; @@ -43,7 +43,6 @@ import type { IMenuItem } from "./Menu.js"; tag: "ui5-menu-item-group", renderer: jsxRenderer, template: MenuItemGroupTemplate, - dependencies: [MenuItem], }) class MenuItemGroup extends UI5Element implements IMenuItem { /** @@ -122,6 +121,14 @@ class MenuItemGroup extends UI5Element implements IMenuItem { } } +const isInstanceOfMenuItemGroup = (object: any): object is MenuItemGroup => { + return "isGroup" in object; +}; + MenuItemGroup.define(); export default MenuItemGroup; + +export { + isInstanceOfMenuItemGroup, +}; diff --git a/packages/main/src/MenuSeparator.ts b/packages/main/src/MenuSeparator.ts index 8cfce33c26c5..43b5a74536d3 100644 --- a/packages/main/src/MenuSeparator.ts +++ b/packages/main/src/MenuSeparator.ts @@ -50,6 +50,15 @@ class MenuSeparator extends ListItemBase implements IMenuItem { return false; } } + +const isInstanceOfMenuSeparator = (object: any): object is MenuSeparator => { + return "isSeparator" in object; +}; + MenuSeparator.define(); export default MenuSeparator; + +export { + isInstanceOfMenuSeparator, +}; diff --git a/packages/main/test/pages/Menu.html b/packages/main/test/pages/Menu.html index e7aca221e530..e806eec4c760 100644 --- a/packages/main/test/pages/Menu.html +++ b/packages/main/test/pages/Menu.html @@ -142,7 +142,7 @@ - + From 4263b4a72c276b69f04fba2d24ff9fee0df59552 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Wed, 14 May 2025 13:44:05 +0300 Subject: [PATCH 19/22] feat(ui5-menu): fix lint error --- packages/fiori/src/UserMenuItem.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/fiori/src/UserMenuItem.ts b/packages/fiori/src/UserMenuItem.ts index ec315f94a4ed..aa18ecced610 100644 --- a/packages/fiori/src/UserMenuItem.ts +++ b/packages/fiori/src/UserMenuItem.ts @@ -1,6 +1,5 @@ import { customElement, slot } from "@ui5/webcomponents-base/dist/decorators.js"; -import MenuItem from "@ui5/webcomponents/dist/MenuItem.js"; -import { isInstanceOfMenuItem } from "@ui5/webcomponents/dist/MenuItem.js"; +import MenuItem, { isInstanceOfMenuItem } from "@ui5/webcomponents/dist/MenuItem.js"; import UserMenuItemTemplate from "./UserMenuItemTemplate.js"; From f84c0e741a17e07c84a48812b7bf4aaab2899775 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Tue, 17 Jun 2025 14:20:17 +0300 Subject: [PATCH 20/22] feat(ui5-menu): fix comments --- packages/main/cypress/specs/Menu.cy.tsx | 24 +++++------ packages/main/src/Menu.ts | 13 ------ packages/main/src/MenuItem.ts | 42 +++++++++---------- packages/main/src/MenuItemGroup.ts | 20 ++++----- packages/main/src/MenuItemTemplate.tsx | 4 +- ...CheckMode.ts => MenuItemGroupCheckMode.ts} | 6 +-- packages/main/test/pages/Menu.html | 15 ++++--- .../_samples/main/Menu/ItemGroups/sample.html | 4 +- 8 files changed, 60 insertions(+), 68 deletions(-) rename packages/main/src/types/{ItemCheckMode.ts => MenuItemGroupCheckMode.ts} (82%) diff --git a/packages/main/cypress/specs/Menu.cy.tsx b/packages/main/cypress/specs/Menu.cy.tsx index ccef1b9a77cd..d8ecc0db7a9c 100644 --- a/packages/main/cypress/specs/Menu.cy.tsx +++ b/packages/main/cypress/specs/Menu.cy.tsx @@ -473,18 +473,18 @@ describe("Menu interaction", () => { - + - + - + @@ -586,11 +586,11 @@ describe("Menu interaction", () => { .should("not.exist"); }); - it("Select/deselect items (Single mode)", () => { + it("Select/deselect items (checkMode=Single)", () => { cy.mount(<> - + @@ -664,11 +664,11 @@ describe("Menu interaction", () => { .should("not.exist"); }); - it("Select/deselect items (Multiple mode) ", () => { + it("Select/deselect items (checkMode=Multiple) ", () => { cy.mount(<> - + @@ -749,11 +749,11 @@ describe("Menu interaction", () => { .should("exist"); }); - it("Select item (None mode) ", () => { + it("Select item (checkMode=None) ", () => { cy.mount(<> - + @@ -779,15 +779,15 @@ describe("Menu interaction", () => { - + - + - + diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 3343f4c2c711..915ce9362537 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -361,19 +361,6 @@ class Menu extends UI5Element { item.selected = true; } - // _closeItemSubMenu(item: MenuItem) { - // if (item && item._popover) { - // const openedSibling = item._allMenuItems.find(menuItem => menuItem._popover && menuItem._popover.open); - - // if (openedSibling) { - // this._closeItemSubMenu(openedSibling); - // } - - // item._popover.open = false; - // item.selected = false; - // } - // } - _itemMouseOver(e: MouseEvent) { if (!isDesktop()) { return; diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 250900f2b432..893be9ecaf0f 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -22,7 +22,7 @@ import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js"; import NavigationMode from "@ui5/webcomponents-base/dist/types/NavigationMode.js"; import ItemNavigation from "@ui5/webcomponents-base/dist/delegate/ItemNavigation.js"; import ItemNavigationBehavior from "@ui5/webcomponents-base/dist/types/ItemNavigationBehavior.js"; -import ItemCheckMode from "./types/ItemCheckMode.js"; +import MenuItemGroupCheckMode from "./types/MenuItemGroupCheckMode.js"; import type { ListItemAccessibilityAttributes } from "./ListItem.js"; import type List from "./List.js"; import ListItem from "./ListItem.js"; @@ -84,7 +84,7 @@ type MenuItemAccessibilityAttributes = Pick_itemCheckMode property of all menu items in the group. + * Sets _checkMode property of all menu items in the group. * @private */ _updateItemsCheckMode() { this._menuItems.forEach((item: MenuItem) => { - item._itemCheckMode = this.itemCheckMode; + item._checkMode = this.checkMode; }); } @@ -114,7 +114,7 @@ class MenuItemGroup extends UI5Element implements IMenuItem { * @private */ _handleItemCheck(e: CustomEvent) { - if (this.itemCheckMode === ItemCheckMode.Single) { + if (this.checkMode === MenuItemGroupCheckMode.Single) { this._clearCheckedItems(); (e.target as MenuItem).checked = true; } diff --git a/packages/main/src/MenuItemTemplate.tsx b/packages/main/src/MenuItemTemplate.tsx index 995804623123..7815aeb00322 100644 --- a/packages/main/src/MenuItemTemplate.tsx +++ b/packages/main/src/MenuItemTemplate.tsx @@ -5,6 +5,7 @@ import List from "./List.js"; import BusyIndicator from "./BusyIndicator.js"; import navBackIcon from "@ui5/webcomponents-icons/dist/nav-back.js"; import declineIcon from "@ui5/webcomponents-icons/dist/decline.js"; +import checkIcon from "@ui5/webcomponents-icons/dist/accept.js"; import slimArrowRight from "@ui5/webcomponents-icons/dist/slim-arrow-right.js"; import Icon from "./Icon.js"; import ListItemTemplate from "./ListItemTemplate.js"; @@ -38,8 +39,7 @@ function checkmarkContent(this: MenuItem) { return !this._markChecked ? "" : (
diff --git a/packages/main/src/types/ItemCheckMode.ts b/packages/main/src/types/MenuItemGroupCheckMode.ts similarity index 82% rename from packages/main/src/types/ItemCheckMode.ts rename to packages/main/src/types/MenuItemGroupCheckMode.ts index fd63904bdeb0..5212bf53e49e 100644 --- a/packages/main/src/types/ItemCheckMode.ts +++ b/packages/main/src/types/MenuItemGroupCheckMode.ts @@ -1,9 +1,9 @@ /** * Menu item group check modes. - * @since 2.11.0 + * @since 2.12.0 * @public */ -enum ItemCheckMode { +enum MenuItemGroupCheckMode { /** * default type (items in a group cannot be checked) * @public @@ -23,4 +23,4 @@ enum ItemCheckMode { Multiple = "Multiple", } -export default ItemCheckMode; +export default MenuItemGroupCheckMode; diff --git a/packages/main/test/pages/Menu.html b/packages/main/test/pages/Menu.html index e806eec4c760..ab80cb323910 100644 --- a/packages/main/test/pages/Menu.html +++ b/packages/main/test/pages/Menu.html @@ -123,20 +123,20 @@ - + - + - + @@ -145,7 +145,7 @@ - + @@ -153,7 +153,7 @@ - + @@ -351,6 +351,11 @@ role: "menuitemcheckbox", }; + menuGroups.addEventListener("ui5-item-click", function(event) { + console.warn("Item clicked: " + event.detail.item.text); + console.warn("Item checked: " + event.detail.item.checked); + }); + diff --git a/packages/website/docs/_samples/main/Menu/ItemGroups/sample.html b/packages/website/docs/_samples/main/Menu/ItemGroups/sample.html index 917d9da24252..576e5873afd4 100644 --- a/packages/website/docs/_samples/main/Menu/ItemGroups/sample.html +++ b/packages/website/docs/_samples/main/Menu/ItemGroups/sample.html @@ -18,7 +18,7 @@ - + @@ -26,7 +26,7 @@ - + From 61e097dcfe89902528183fd9c2e31373db817f04 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Thu, 19 Jun 2025 15:52:18 +0300 Subject: [PATCH 21/22] feat(ui5-menu): fix more comments --- packages/main/src/Menu.ts | 37 ++++++----- packages/main/src/MenuItem.ts | 98 +++++++++++++++--------------- packages/main/src/MenuItemGroup.ts | 27 +++++++- packages/main/test/pages/Menu.html | 7 ++- 4 files changed, 98 insertions(+), 71 deletions(-) diff --git a/packages/main/src/Menu.ts b/packages/main/src/Menu.ts index 915ce9362537..5d15c1b1b20f 100644 --- a/packages/main/src/Menu.ts +++ b/packages/main/src/Menu.ts @@ -261,10 +261,6 @@ class Menu extends UI5Element { return isPhone(); } - get acessibleNameText() { - return Menu.i18nBundle.getText(MENU_POPOVER_ACCESSIBLE_NAME); - } - get _popover() { return this.shadowRoot!.querySelector("[ui5-responsive-popover]")!; } @@ -314,6 +310,10 @@ class Menu extends UI5Element { return items; } + get acessibleNameText() { + return Menu.i18nBundle.getText(MENU_POPOVER_ACCESSIBLE_NAME); + } + onBeforeRendering() { const siblingsWithIcon = this._allMenuItems.some(menuItem => !!menuItem.icon); @@ -324,17 +324,6 @@ class Menu extends UI5Element { }); } - async focus(focusOptions?: FocusOptions): Promise { - await renderFinished(); - const firstMenuItem = this._allMenuItems[0]; - - if (firstMenuItem) { - return firstMenuItem.focus(focusOptions); - } - - return super.focus(focusOptions); - } - _setupItemNavigation() { if (this._list) { this._list._itemNavigation._getItems = () => this._navigatableMenuItems; @@ -377,6 +366,17 @@ class Menu extends UI5Element { this._startOpenTimeout(item); } + async focus(focusOptions?: FocusOptions): Promise { + await renderFinished(); + const firstMenuItem = this._allMenuItems[0]; + + if (firstMenuItem) { + return firstMenuItem.focus(focusOptions); + } + + return super.focus(focusOptions); + } + _closeOtherSubMenus(item: MenuItem) { const menuItems = this._allMenuItems; if (!menuItems.includes(item)) { @@ -403,16 +403,15 @@ class Menu extends UI5Element { _itemClick(e: CustomEvent) { const item = e.detail.item as MenuItem; - item._updateCheckedState(); - if (!item._popover) { const prevented = !this.fireDecoratorEvent("item-click", { "item": item, "text": item.text || "", }); - if (!prevented && this._popover) { - item.fireDecoratorEvent("close-menu"); + if (!prevented) { + item._updateCheckedState(); + this._popover && item.fireDecoratorEvent("close-menu"); } } else { this._openItemSubMenu(item); diff --git a/packages/main/src/MenuItem.ts b/packages/main/src/MenuItem.ts index 893be9ecaf0f..809970e0bcfb 100644 --- a/packages/main/src/MenuItem.ts +++ b/packages/main/src/MenuItem.ts @@ -130,7 +130,7 @@ type MenuItemAccessibilityAttributes = Pick("[ui5-list]")!; } - get _popover() { - return this.shadowRoot && this.shadowRoot.querySelector("[ui5-responsive-popover]")!; - } - - get isMenuItem(): boolean { - return true; - } - - get isRtl() { - return this.effectiveDir === "rtl"; - } - get _navigableItems(): Array { return [...this.endContent].filter(item => { return item.hasAttribute("ui5-button") @@ -345,10 +333,26 @@ class MenuItem extends ListItem implements IMenuItem { }); } + _navigateToEndContent(shouldNavigateToPreviousItem: boolean) { + const navigatableItems = this._navigableItems; + const item = shouldNavigateToPreviousItem + ? navigatableItems[navigatableItems.length - 1] + : navigatableItems[0]; + + if (item) { + this._itemNavigation.setCurrentItem(item); + this._itemNavigation._focusCurrentItem(); + } + } + get placement(): `${PopoverPlacement}` { return this.isRtl ? "Start" : "End"; } + get isRtl() { + return this.effectiveDir === "rtl"; + } + get hasSubmenu() { return !!(this.items.length || this.loading) && !this.disabled; } @@ -385,6 +389,29 @@ class MenuItem extends ListItem implements IMenuItem { return MenuItem.i18nBundle.getText(MENU_POPOVER_ACCESSIBLE_NAME); } + onBeforeRendering() { + super.onBeforeRendering(); + + const siblingsWithIcon = this._allMenuItems.some(menuItem => !!menuItem.icon); + + this._setupItemNavigation(); + + this._allMenuItems.forEach(item => { + item._siblingsWithIcon = siblingsWithIcon; + }); + } + + async focus(focusOptions?: FocusOptions): Promise { + await renderFinished(); + + if (this.hasSubmenu && this.isSubMenuOpen) { + const menuItems = this._allMenuItems; + return menuItems[0] && menuItems[0].focus(focusOptions); + } + + return super.focus(focusOptions); + } + get _focusable() { return true; } @@ -418,6 +445,10 @@ class MenuItem extends ListItem implements IMenuItem { return { ...super._accInfo, ...accInfoSettings }; } + get _popover() { + return this.shadowRoot && this.shadowRoot.querySelector("[ui5-responsive-popover]")!; + } + get _markChecked() { return !this.hasSubmenu && this.checked && this._checkMode !== MenuItemGroupCheckMode.None; } @@ -464,41 +495,6 @@ class MenuItem extends ListItem implements IMenuItem { return items; } - onBeforeRendering() { - super.onBeforeRendering(); - - const siblingsWithIcon = this._allMenuItems.some(menuItem => !!menuItem.icon); - - this._setupItemNavigation(); - - this._allMenuItems.forEach(item => { - item._siblingsWithIcon = siblingsWithIcon; - }); - } - - async focus(focusOptions?: FocusOptions): Promise { - await renderFinished(); - - if (this.hasSubmenu && this.isSubMenuOpen) { - const menuItems = this._allMenuItems; - return menuItems[0] && menuItems[0].focus(focusOptions); - } - - return super.focus(focusOptions); - } - - _navigateToEndContent(shouldNavigateToPreviousItem: boolean) { - const navigatableItems = this._navigableItems; - const item = shouldNavigateToPreviousItem - ? navigatableItems[navigatableItems.length - 1] - : navigatableItems[0]; - - if (item) { - this._itemNavigation.setCurrentItem(item); - this._itemNavigation._focusCurrentItem(); - } - } - _setupItemNavigation() { if (this._list) { this._list._itemNavigation._getItems = () => this._navigatableMenuItems; @@ -621,6 +617,10 @@ class MenuItem extends ListItem implements IMenuItem { this.fireDecoratorEvent("close"); } + get isMenuItem(): boolean { + return true; + } + _updateCheckedState() { if (this._checkMode === MenuItemGroupCheckMode.None) { return; @@ -628,8 +628,8 @@ class MenuItem extends ListItem implements IMenuItem { const newState = !this.checked; - this.fireDecoratorEvent("item-check"); this.checked = newState; + this.fireDecoratorEvent("item-check"); } } diff --git a/packages/main/src/MenuItemGroup.ts b/packages/main/src/MenuItemGroup.ts index fdf04eefd00d..3c7aacbbdd22 100644 --- a/packages/main/src/MenuItemGroup.ts +++ b/packages/main/src/MenuItemGroup.ts @@ -2,13 +2,16 @@ import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js"; import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js"; import property from "@ui5/webcomponents-base/dist/decorators/property.js"; import slot from "@ui5/webcomponents-base/dist/decorators/slot.js"; +import event from "@ui5/webcomponents-base/dist/decorators/event-strict.js"; import jsxRenderer from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js"; import type MenuItem from "./MenuItem.js"; -import MenuItemGroupTemplate from "./MenuItemGroupTemplate.js"; import { isInstanceOfMenuItem } from "./MenuItem.js"; +import MenuItemGroupTemplate from "./MenuItemGroupTemplate.js"; import MenuItemGroupCheckMode from "./types/MenuItemGroupCheckMode.js"; import type { IMenuItem } from "./Menu.js"; +type MenuItemGroupCheckChangeEventDetail = { checkedItems: Array; } + /** * @class * @@ -44,7 +47,20 @@ import type { IMenuItem } from "./Menu.js"; renderer: jsxRenderer, template: MenuItemGroupTemplate, }) + +/** + * Fired when an item in the group is checked or unchecked. + * @public + * @since 2.12.0 + */ +@event("check-change", { + bubbles: true, +}) class MenuItemGroup extends UI5Element implements IMenuItem { + eventDetails!: UI5Element["eventDetails"] & { + "check-change": MenuItemGroupCheckChangeEventDetail + } + /** * Defines the component's check mode. * @default "None" @@ -114,10 +130,17 @@ class MenuItemGroup extends UI5Element implements IMenuItem { * @private */ _handleItemCheck(e: CustomEvent) { + const item = e.target as MenuItem; + const isChecked = item.checked; + if (this.checkMode === MenuItemGroupCheckMode.Single) { this._clearCheckedItems(); - (e.target as MenuItem).checked = true; + item.checked = isChecked; } + + this.fireDecoratorEvent("check-change", { + checkedItems: this._menuItems.filter((item: MenuItem) => item.checked), + }); } } diff --git a/packages/main/test/pages/Menu.html b/packages/main/test/pages/Menu.html index ab80cb323910..62e599bee87b 100644 --- a/packages/main/test/pages/Menu.html +++ b/packages/main/test/pages/Menu.html @@ -353,7 +353,12 @@ menuGroups.addEventListener("ui5-item-click", function(event) { console.warn("Item clicked: " + event.detail.item.text); - console.warn("Item checked: " + event.detail.item.checked); + + }); + + menuGroups.addEventListener("ui5-check-change", function(event) { + console.warn(event.target); + console.warn("Checked items: " + event.detail.checkedItems.map(item => item.text).join(", ")); }); From bba2c67ced734a5128c80eec530fbeb94c9eb7b6 Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Thu, 19 Jun 2025 16:03:25 +0300 Subject: [PATCH 22/22] feat(ui5-menu): fix lint error --- packages/main/src/MenuItemGroup.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/main/src/MenuItemGroup.ts b/packages/main/src/MenuItemGroup.ts index 3c7aacbbdd22..55f60cdcb917 100644 --- a/packages/main/src/MenuItemGroup.ts +++ b/packages/main/src/MenuItemGroup.ts @@ -130,12 +130,12 @@ class MenuItemGroup extends UI5Element implements IMenuItem { * @private */ _handleItemCheck(e: CustomEvent) { - const item = e.target as MenuItem; - const isChecked = item.checked; + const clickedItem = e.target as MenuItem; + const isChecked = clickedItem.checked; if (this.checkMode === MenuItemGroupCheckMode.Single) { this._clearCheckedItems(); - item.checked = isChecked; + clickedItem.checked = isChecked; } this.fireDecoratorEvent("check-change", {