From 5868ccd2e028ed9188ff6b54a5e5fa32f5c94b5f Mon Sep 17 00:00:00 2001 From: Nikolay Hristov Date: Tue, 15 Oct 2024 14:52:16 +0300 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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; } } }