Skip to content

Commit 69a2413

Browse files
LidiyaGeorgievaLidiyaGeorgieva
authored
fix(ui5-side-navigation): add parent item to submenu and enhance ACC (#12457)
* fix(ui5-side-navigation): add parent item to submenu and enhance ACC According to the interaction design, clicking on the parent item in the Overflow menu should only open the sub-menu, but should not select the parent item. Clicking on the parent item in the sub-menu selects it and closes the menu - this is the only way to select the parent item in the overflow menu (the menu lacks two-click area functionality for the moment). This is the reason why we add the parent item again in the sub-menu. List of changes: - new behavior for Space/Enter keys - always trigger action (navigation) and do not expand/collapse - labels for menus in the overflow area - dual click item in overflow: - new aria description for parent item for dual click area - duplicate the parent item as a child item - updated aria-describedby for dual click item in expanded mode JIRA: BGSOFUIRODOPI-3443 JIRA: BGSOFUIRODOPI-3515
1 parent 68e6a3e commit 69a2413

15 files changed

+206
-57
lines changed

‎packages/fiori/cypress/specs/SideNavigation.cy.tsx‎

Lines changed: 139 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import SideNavigationSubItem from "../../src/SideNavigationSubItem.js";
55
import group from "@ui5/webcomponents-icons/dist/group.js";
66
import home from "@ui5/webcomponents-icons/dist/home.js";
77
import employeeApprovals from "@ui5/webcomponents-icons/dist/employee-approvals.js";
8+
import { SIDE_NAVIGATION_OVERFLOW_ITEM_LABEL } from "../../src/generated/i18n/i18n-defaults.js";
89
import { NAVIGATION_MENU_POPOVER_HIDDEN_TEXT } from "../../src/generated/i18n/i18n-defaults.js";
10+
import { NAVIGATION_MENU_SELECTABLE_ITEM_HIDDEN_TEXT } from "../../src/generated/i18n/i18n-defaults.js";
911
import Title from "@ui5/webcomponents/dist/Title.js";
1012
import Label from "@ui5/webcomponents/dist/Label.js";
1113
import ResponsivePopover from "@ui5/webcomponents/dist/ResponsivePopover.js";
@@ -208,7 +210,7 @@ describe("Side Navigation interaction", () => {
208210
cy.get("#item1").should("not.have.attr", "expanded");
209211
});
210212

211-
it("Tests expanding and collapsing of unselectable items with Space and Enter", () => {
213+
it("Tests not expanding and collapsing of unselectable items with Space and Enter", () => {
212214
cy.mount(
213215
<SideNavigation>
214216
<SideNavigationItem id="focusStart" text="focus start"></SideNavigationItem>
@@ -221,10 +223,9 @@ describe("Side Navigation interaction", () => {
221223
// act
222224
cy.get("#focusStart").realClick();
223225
cy.realPress("ArrowDown");
224-
cy.realPress("Space");
225226

226227
// assert
227-
cy.get("#unselectableItem").should("be.focused").and("have.attr", "expanded");
228+
cy.get("#unselectableItem").should("be.focused").and("not.have.attr", "expanded");
228229

229230
// act
230231
cy.realPress("Space");
@@ -235,12 +236,6 @@ describe("Side Navigation interaction", () => {
235236
// act
236237
cy.realPress("Enter");
237238

238-
// assert
239-
cy.get("#unselectableItem").should("be.focused").and("have.attr", "expanded");
240-
241-
// act
242-
cy.realPress("Enter");
243-
244239
// assert
245240
cy.get("#unselectableItem").should("be.focused").and("not.have.attr", "expanded");
246241
});
@@ -643,10 +638,10 @@ describe("Side Navigation interaction", () => {
643638

644639
[
645640
{ selector: "#item", expectedCallCount: 2 },
646-
{ selector: "#unselectableItem", expectedCallCount: 2 },
641+
{ selector: "#unselectableItem", expectedCallCount: 1 },
647642
{ selector: "#parentItem", expectedCallCount: 2 },
648643
{ selector: "#childItem", expectedCallCount: 2 },
649-
{ selector: "#unselectableParentItem", expectedCallCount: 2 },
644+
{ selector: "#unselectableParentItem", expectedCallCount: 1 },
650645
].forEach(({ selector, expectedCallCount }) => {
651646
cy.get("#sideNav")
652647
.then(sideNav => {
@@ -697,7 +692,7 @@ describe("Side Navigation interaction", () => {
697692
cy.mount(
698693
<SideNavigation>
699694
<SideNavigationItem id="focusStart" text="focus start" />
700-
<SideNavigationItem text="external link" unselectable={true} href="#test" />
695+
<SideNavigationItem text="link" href="#test" />
701696
</SideNavigation>
702697
);
703698

@@ -1210,21 +1205,65 @@ describe("Side Navigation Accessibility", () => {
12101205

12111206
// assert
12121207
cy.get("@overflowMenu")
1213-
.find("[ui5-navigation-menu-item][text='1']")
1208+
.find("[ui5-navigation-menu-item][text='2']")
12141209
.shadow()
12151210
.find(".ui5-navigation-menu-item-root")
1211+
.should("have.attr", "aria-haspopup", "menu");
1212+
});
1213+
1214+
it("SideNavigationItem description", () => {
1215+
cy.mount(
1216+
<SideNavigation>
1217+
<SideNavigationItem id="item1" text="1" selected={true}/>
1218+
<SideNavigationItem id="item2" text="2" expanded={true}>
1219+
<SideNavigationSubItem id="childItem" text="2.1" />
1220+
</SideNavigationItem>
1221+
</SideNavigation>
1222+
);
1223+
1224+
cy.get("#item1")
1225+
.shadow()
1226+
.find(".ui5-sn-item")
1227+
.should("not.have.attr", "aria-describedby");
1228+
1229+
cy.get("#item2")
1230+
.shadow()
1231+
.find(".ui5-sn-item")
1232+
.should("have.attr", "aria-describedby", "To navigate to navigation item 2, press Spacebar or Enter.");
1233+
1234+
cy.get("#item1")
1235+
.shadow()
1236+
.find(".ui5-sn-item")
1237+
.should("not.have.attr", "aria-expanded");
1238+
1239+
cy.get("#item2")
1240+
.shadow()
1241+
.find(".ui5-sn-item")
1242+
.should("have.attr", "aria-expanded", "true");
1243+
1244+
cy.get("#item1")
1245+
.invoke("prop", "accessibilityAttributes", {
1246+
hasPopup: "dialog",
1247+
});
1248+
1249+
cy.get("#item1")
1250+
.shadow()
1251+
.find(".ui5-sn-item")
12161252
.should("have.attr", "aria-haspopup", "dialog");
12171253

1218-
cy.get("@overflowMenu")
1219-
.find("[ui5-navigation-menu-item][text='2']")
1254+
cy.get("#childItem")
12201255
.shadow()
1221-
.find(".ui5-navigation-menu-item-root")
1222-
.should("have.attr", "aria-haspopup", "menu");
1256+
.find(".ui5-sn-item")
1257+
.should("not.have.attr", "aria-haspopup");
12231258

1224-
cy.get("@overflowMenu")
1225-
.find("[ui5-navigation-menu-item][text='2.1']")
1259+
cy.get("#childItem")
1260+
.invoke("prop", "accessibilityAttributes", {
1261+
hasPopup: "dialog",
1262+
});
1263+
1264+
cy.get("#childItem")
12261265
.shadow()
1227-
.find(".ui5-navigation-menu-item-root")
1266+
.find(".ui5-sn-item")
12281267
.should("have.attr", "aria-haspopup", "dialog");
12291268
});
12301269

@@ -1265,7 +1304,7 @@ describe("Side Navigation Accessibility", () => {
12651304
.find("[ui5-side-navigation-item][is-overflow]")
12661305
.shadow()
12671306
.find(".ui5-sn-item")
1268-
.should("have.attr", "aria-label", "Displays remaining navigation items");
1307+
.should("have.attr", "aria-label", SIDE_NAVIGATION_OVERFLOW_ITEM_LABEL.defaultText);
12691308
});
12701309

12711310
it("SideNavigationItem aria-checked in collapsed SideNavigation", () => {
@@ -1291,10 +1330,13 @@ describe("Side Navigation Accessibility", () => {
12911330
it("Tests accessible-name of overflow menu and sub menu", () => {
12921331
cy.mount(
12931332
<SideNavigation id="sideNav" collapsed={true}>
1294-
<SideNavigationItem text="dummy item"></SideNavigationItem>
1295-
<SideNavigationItem text="1">
1333+
<SideNavigationItem text="No children"></SideNavigationItem>
1334+
<SideNavigationItem text="Parent 1">
12961335
<SideNavigationSubItem text="1.1" />
12971336
</SideNavigationItem>
1337+
<SideNavigationItem text="Parent 2 unselectable" unselectable={true}>
1338+
<SideNavigationSubItem text="2.1" />
1339+
</SideNavigationItem>
12981340
</SideNavigation>
12991341
);
13001342

@@ -1305,7 +1347,8 @@ describe("Side Navigation Accessibility", () => {
13051347
.shadow()
13061348
.find(".ui5-sn-item-overflow")
13071349
.realClick();
1308-
1350+
1351+
// Assert
13091352
cy.get("#sideNav")
13101353
.shadow()
13111354
.find(".ui5-side-navigation-overflow-menu")
@@ -1314,20 +1357,88 @@ describe("Side Navigation Accessibility", () => {
13141357
.invoke("attr", "accessible-name-ref")
13151358
.should("match", /navigationMenuPopoverText$/);
13161359

1360+
13171361
cy.get("#sideNav")
13181362
.shadow()
1319-
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='1']")
1363+
.find(".ui5-side-navigation-overflow-menu")
1364+
.shadow()
1365+
.find(".ui5-menu-rp")
1366+
.find(".ui5-hidden-text")
1367+
.should("have.text", NAVIGATION_MENU_POPOVER_HIDDEN_TEXT.defaultText);
1368+
1369+
cy.get("#sideNav")
1370+
.shadow()
1371+
.find(".ui5-side-navigation-overflow-menu")
1372+
.shadow()
1373+
.find(".ui5-menu-rp")
1374+
.should("have.attr", "accessible-role", "Dialog");
1375+
1376+
cy.get("#sideNav")
1377+
.shadow()
1378+
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='No children']")
1379+
.shadow()
1380+
.find(".ui5-navigation-menu-item-root")
1381+
.should("not.have.attr", "aria-haspopup");
1382+
1383+
cy.get("#sideNav")
1384+
.shadow()
1385+
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='Parent 1']")
1386+
.shadow()
1387+
.find(".ui5-navigation-menu-item-root")
1388+
.should("have.attr", "aria-haspopup", "menu");
1389+
1390+
cy.get("#sideNav")
1391+
.shadow()
1392+
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='Parent 2 unselectable']")
1393+
.shadow()
1394+
.find(".ui5-navigation-menu-item-root")
1395+
.should("have.attr", "aria-haspopup", "menu");
1396+
1397+
cy.get("#sideNav")
1398+
.shadow()
1399+
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='Parent 1']")
1400+
.shadow()
1401+
.find(".ui5-navigation-menu-item-root")
1402+
.invoke("attr", "aria-describedby")
1403+
.should("match", /invisibleText-describedby$/);
1404+
1405+
cy.get("#sideNav")
1406+
.shadow()
1407+
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='Parent 1']")
1408+
.shadow()
1409+
.find(".ui5-navigation-menu-item-root .ui5-hidden-text")
1410+
.next()
1411+
.should("have.text", NAVIGATION_MENU_SELECTABLE_ITEM_HIDDEN_TEXT.defaultText);
1412+
1413+
// Act - open sub menu
1414+
cy.get("#sideNav")
1415+
.shadow()
1416+
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='Parent 1']")
13201417
.realClick();
13211418

1419+
// Assert
13221420
cy.get("#sideNav")
13231421
.shadow()
1324-
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='1']")
1422+
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='Parent 1']")
1423+
.shadow()
1424+
.find(".ui5-menu-rp")
1425+
.should("have.attr", "accessible-name", "Parent 1");
1426+
1427+
cy.get("#sideNav")
1428+
.shadow()
1429+
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='Parent 2 unselectable']")
13251430
.shadow()
13261431
.find(".ui5-menu-rp")
1327-
.should("have.attr", "accessible-name", NAVIGATION_MENU_POPOVER_HIDDEN_TEXT.defaultText);
1432+
.should("have.attr", "accessible-name", "Parent 2 unselectable");
1433+
1434+
cy.get("#sideNav")
1435+
.shadow()
1436+
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='Parent 1']")
1437+
.find("[ui5-navigation-menu-item][text='Parent 1']")
1438+
.should("exist");
13281439
});
13291440

1330-
it("Tests SideNavigationGroup accessibility", () => {
1441+
it("Tests SideNavigationGroup group accessibility attributes", () => {
13311442
cy.mount(
13321443
<SideNavigation>
13331444
<SideNavigationItem text="Home"></SideNavigationItem>

‎packages/fiori/src/NavigationMenuItem.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import NavigationMenuItemTemplate from "./NavigationMenuItemTemplate.js";
2222
import navigationMenuItemCss from "./generated/themes/NavigationMenuItem.css.js";
2323

2424
import {
25-
NAVIGATION_MENU_POPOVER_HIDDEN_TEXT,
25+
NAVIGATION_MENU_SELECTABLE_ITEM_HIDDEN_TEXT,
2626
} from "./generated/i18n/i18n-defaults.js";
2727

2828
/**
@@ -104,10 +104,11 @@ class NavigationMenuItem extends MenuItem {
104104
get _accInfo() {
105105
const accInfo = super._accInfo;
106106

107-
accInfo.role = this.href ? "none" : "treeitem";
107+
accInfo.role = "none";
108108

109-
if (!accInfo.ariaHaspopup) {
110-
accInfo.ariaHaspopup = this.accessibilityAttributes.hasPopup;
109+
if (this.hasSubmenu && this.associatedItem?.isSelectable) {
110+
// For the menu item on first level (parent item)
111+
accInfo.ariaSelectedText = NavigationMenuItem.i18nBundleFiori.getText(NAVIGATION_MENU_SELECTABLE_ITEM_HIDDEN_TEXT);
111112
}
112113

113114
return accInfo;
@@ -204,8 +205,9 @@ class NavigationMenuItem extends MenuItem {
204205
}
205206
}
206207

207-
get acessibleNameText() {
208-
return NavigationMenuItem.i18nBundleFiori.getText(NAVIGATION_MENU_POPOVER_HIDDEN_TEXT);
208+
get accessibleNameText() {
209+
// For the submenu's dialog
210+
return this.text ?? "";
209211
}
210212
}
211213

‎packages/fiori/src/NavigationMenuItemTemplate.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export default function NavigationMenuItemTemplate(this: NavigationMenuItem, hoo
1717
return <>
1818
{
1919
this._href ? (
20-
<a role="treeitem"
20+
<a role="menuitem"
2121
class="ui5-navmenu-item-link"
2222
href={this.href}
2323
target={this.target}

‎packages/fiori/src/NavigationMenuTemplate.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export default function NavigationMenuTemplate(this: NavigationMenu) {
4040
{this.items.length > 0 ?
4141
<List
4242
id={`${this._id}-menu-list`}
43-
accessibleRole="Tree"
43+
accessibleRole="Menu"
4444
selectionMode="None"
4545
loading={this.loading}
4646
loadingDelay={this.loadingDelay}

‎packages/fiori/src/SideNavigationGroup.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import jsxRender from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js";
33
import slot from "@ui5/webcomponents-base/dist/decorators/slot.js";
44
import i18n from "@ui5/webcomponents-base/dist/decorators/i18n.js";
55
import {
6+
isSpace,
67
isLeft,
78
isRight,
89
isMinus,
@@ -143,6 +144,10 @@ class SideNavigationGroup extends SideNavigationItemBase {
143144
return;
144145
}
145146

147+
if (isSpace(e)) {
148+
e.preventDefault();
149+
}
150+
146151
const isRTL = this.effectiveDir === "rtl";
147152

148153
if (isLeft(e)) {

0 commit comments

Comments
 (0)