From 7ce24c03174cfc280eeb1d22a473e554fe4b5957 Mon Sep 17 00:00:00 2001
From: Artem Zaiko <102226752+azaiko-akveo@users.noreply.github.com>
Date: Tue, 15 Nov 2022 17:44:05 +0300
Subject: [PATCH] feat(menu component): improve accessibility (#3136)

* feat(menu component): improve accessibility
* fix(menu component): refactor property name
---
 .../components/menu/menu-item.component.html  | 114 ++++++++++--------
 .../theme/components/menu/menu.component.scss |   1 +
 .../theme/components/menu/menu.service.ts     |  50 ++++----
 .../menu/menu-autocollapse.component.ts       |   7 +-
 4 files changed, 97 insertions(+), 75 deletions(-)

diff --git a/src/framework/theme/components/menu/menu-item.component.html b/src/framework/theme/components/menu/menu-item.component.html
index c8d94a9619..d31dc37f71 100644
--- a/src/framework/theme/components/menu/menu-item.component.html
+++ b/src/framework/theme/components/menu/menu-item.component.html
@@ -2,75 +2,91 @@
   <nb-icon class="menu-icon" [config]="menuItem.icon" *ngIf="menuItem.icon"></nb-icon>
   {{ menuItem.title }}
 </span>
-<a *ngIf="menuItem.link && !menuItem.url && !menuItem.children && !menuItem.group"
-   [routerLink]="menuItem.link"
-   [queryParams]="menuItem.queryParams"
-   [fragment]="menuItem.fragment"
-   [queryParamsHandling]="menuItem.queryParamsHandling"
-   [preserveFragment]="menuItem.preserveFragment"
-   [skipLocationChange]="menuItem.skipLocationChange"
-   [attr.target]="menuItem.target"
-   [attr.title]="menuItem.title"
-   [class.active]="menuItem.selected"
-   (mouseenter)="onHoverItem(menuItem)"
-   (click)="onItemClick(menuItem);">
+<a
+  *ngIf="menuItem.link && !menuItem.url && !menuItem.children && !menuItem.group"
+  [routerLink]="menuItem.link"
+  [queryParams]="menuItem.queryParams"
+  [fragment]="menuItem.fragment"
+  [queryParamsHandling]="menuItem.queryParamsHandling"
+  [preserveFragment]="menuItem.preserveFragment"
+  [skipLocationChange]="menuItem.skipLocationChange"
+  [attr.target]="menuItem.target"
+  [attr.title]="menuItem.title"
+  [attr.role]="menuItem.ariaRole"
+  [class.active]="menuItem.selected"
+  (mouseenter)="onHoverItem(menuItem)"
+  (click)="onItemClick(menuItem)"
+>
   <nb-icon class="menu-icon" [config]="menuItem.icon" *ngIf="menuItem.icon"></nb-icon>
   <span class="menu-title">{{ menuItem.title }}</span>
   <ng-container *ngIf="badge" [ngTemplateOutlet]="badgeTemplate"></ng-container>
 </a>
-<a *ngIf="menuItem.url && !menuItem.children && !menuItem.link && !menuItem.group"
-   [attr.href]="menuItem.url"
-   [attr.target]="menuItem.target"
-   [attr.title]="menuItem.title"
-   [class.active]="menuItem.selected"
-   (mouseenter)="onHoverItem(menuItem)"
-   (click)="onSelectItem(menuItem)">
+<a
+  *ngIf="menuItem.url && !menuItem.children && !menuItem.link && !menuItem.group"
+  [attr.href]="menuItem.url"
+  [attr.target]="menuItem.target"
+  [attr.title]="menuItem.title"
+  [attr.role]="menuItem.ariaRole"
+  [class.active]="menuItem.selected"
+  (mouseenter)="onHoverItem(menuItem)"
+  (click)="onSelectItem(menuItem)"
+>
   <nb-icon class="menu-icon" [config]="menuItem.icon" *ngIf="menuItem.icon"></nb-icon>
   <span class="menu-title">{{ menuItem.title }}</span>
   <ng-container *ngIf="badge" [ngTemplateOutlet]="badgeTemplate"></ng-container>
 </a>
-<a *ngIf="!menuItem.children && !menuItem.link && !menuItem.url && !menuItem.group"
-   [attr.target]="menuItem.target"
-   [attr.title]="menuItem.title"
-   [class.active]="menuItem.selected"
-   (mouseenter)="onHoverItem(menuItem)"
-   (click)="$event.preventDefault(); onItemClick(menuItem);">
+<a
+  *ngIf="!menuItem.children && !menuItem.link && !menuItem.url && !menuItem.group"
+  [attr.target]="menuItem.target"
+  [attr.title]="menuItem.title"
+  [attr.role]="menuItem.ariaRole"
+  [class.active]="menuItem.selected"
+  (mouseenter)="onHoverItem(menuItem)"
+  (click)="$event.preventDefault(); onItemClick(menuItem)"
+>
   <nb-icon class="menu-icon" [config]="menuItem.icon" *ngIf="menuItem.icon"></nb-icon>
   <span class="menu-title">{{ menuItem.title }}</span>
   <ng-container *ngIf="badge" [ngTemplateOutlet]="badgeTemplate"></ng-container>
 </a>
-<a *ngIf="menuItem.children"
-   (click)="$event.preventDefault(); onToggleSubMenu(menuItem);"
-   [attr.target]="menuItem.target"
-   [attr.title]="menuItem.title"
-   [class.active]="menuItem.selected"
-   (mouseenter)="onHoverItem(menuItem)"
-   href="#">
+<a
+  *ngIf="menuItem.children"
+  (click)="$event.preventDefault(); onToggleSubMenu(menuItem)"
+  [attr.target]="menuItem.target"
+  [attr.title]="menuItem.title"
+  [attr.aria-expanded]="menuItem.expanded ?? false"
+  [attr.role]="menuItem.ariaRole"
+  [class.active]="menuItem.selected"
+  (mouseenter)="onHoverItem(menuItem)"
+  href="#"
+>
   <nb-icon class="menu-icon" [config]="menuItem.icon" *ngIf="menuItem.icon"></nb-icon>
   <span class="menu-title">{{ menuItem.title }}</span>
   <ng-container *ngIf="badge" [ngTemplateOutlet]="badgeTemplate"></ng-container>
   <nb-icon class="expand-state" [icon]="getExpandStateIcon()" pack="nebular-essentials"></nb-icon>
 </a>
-<ul *ngIf="menuItem.children"
-    [class.collapsed]="!(menuItem.children && menuItem.expanded)"
-    [class.expanded]="menuItem.expanded"
-    [@toggle]="toggleState"
-    class="menu-items">
+<ul
+  *ngIf="menuItem.children"
+  [class.collapsed]="!(menuItem.children && menuItem.expanded)"
+  [class.expanded]="menuItem.expanded"
+  [@toggle]="toggleState"
+  class="menu-items"
+>
   <ng-container *ngFor="let item of menuItem.children">
-    <li nbMenuItem *ngIf="!item.hidden"
-        [menuItem]="item"
-        [badge]="item.badge"
-        [class.menu-group]="item.group"
-        (hoverItem)="onHoverItem($event)"
-        (toggleSubMenu)="onToggleSubMenu($event)"
-        (selectItem)="onSelectItem($event)"
-        (itemClick)="onItemClick($event)"
-        class="menu-item">
-    </li>
+    <li
+      nbMenuItem
+      *ngIf="!item.hidden"
+      [menuItem]="item"
+      [badge]="item.badge"
+      [class.menu-group]="item.group"
+      (hoverItem)="onHoverItem($event)"
+      (toggleSubMenu)="onToggleSubMenu($event)"
+      (selectItem)="onSelectItem($event)"
+      (itemClick)="onItemClick($event)"
+      class="menu-item"
+    ></li>
   </ng-container>
 </ul>
 
 <ng-template #badgeTemplate>
-  <nb-badge [text]="badge.text" [dotMode]="badge.dotMode" [status]="badge.status">
-  </nb-badge>
+  <nb-badge [text]="badge.text" [dotMode]="badge.dotMode" [status]="badge.status"> </nb-badge>
 </ng-template>
diff --git a/src/framework/theme/components/menu/menu.component.scss b/src/framework/theme/components/menu/menu.component.scss
index 94886cf085..3b29ca6c00 100644
--- a/src/framework/theme/components/menu/menu.component.scss
+++ b/src/framework/theme/components/menu/menu.component.scss
@@ -22,6 +22,7 @@
 
     .menu-title {
       flex: 1 0 auto;
+      pointer-events: none;
       @include nb-rtl(text-align, right);
     }
   }
diff --git a/src/framework/theme/components/menu/menu.service.ts b/src/framework/theme/components/menu/menu.service.ts
index 1c35294113..6b669a7f2a 100644
--- a/src/framework/theme/components/menu/menu.service.ts
+++ b/src/framework/theme/components/menu/menu.service.ts
@@ -13,13 +13,15 @@ import { isFragmentContain, isFragmentEqual, isUrlPathContain, isUrlPathEqual }
 import { NbIconConfig } from '../icon/icon.component';
 import { NbBadge } from '../badge/badge.component';
 
-export interface NbMenuBag { tag: string; item: NbMenuItem }
+export interface NbMenuBag {
+  tag: string;
+  item: NbMenuItem;
+}
 
 const itemClick$ = new Subject<NbMenuBag>();
 const addItems$ = new ReplaySubject<{ tag: string; items: NbMenuItem[] }>(1);
 const navigateHome$ = new ReplaySubject<{ tag: string }>(1);
-const getSelectedItem$
-  = new ReplaySubject<{ tag: string; listener: BehaviorSubject<NbMenuBag> }>(1);
+const getSelectedItem$ = new ReplaySubject<{ tag: string; listener: BehaviorSubject<NbMenuBag> }>(1);
 const itemSelect$ = new ReplaySubject<NbMenuBag>(1);
 const itemHover$ = new ReplaySubject<NbMenuBag>(1);
 const submenuToggle$ = new ReplaySubject<NbMenuBag>(1);
@@ -111,6 +113,10 @@ export class NbMenuItem {
   data?: any;
   fragment?: string;
   preserveFragment?: boolean;
+  /** The name of a role in the ARIA specification
+   * @type {string}
+   */
+  ariaRole?: string;
 
   /**
    * @returns item parents in top-down order
@@ -128,9 +134,7 @@ export class NbMenuItem {
   }
 
   static isParent(item: NbMenuItem, possibleChild: NbMenuItem): boolean {
-    return possibleChild.parent
-      ? possibleChild.parent === item || this.isParent(item, possibleChild.parent)
-      : false;
+    return possibleChild.parent ? possibleChild.parent === item || this.isParent(item, possibleChild.parent) : false;
   }
 }
 
@@ -146,7 +150,6 @@ export class NbMenuItem {
  */
 @Injectable()
 export class NbMenuService {
-
   /**
    * Add items to the end of the menu items list
    * @param {List<NbMenuItem>} items
@@ -204,12 +207,11 @@ export class NbMenuService {
 
 @Injectable()
 export class NbMenuInternalService {
-
   constructor(private location: Location) {}
 
   prepareItems(items: NbMenuItem[]) {
     const defaultItem = new NbMenuItem();
-    items.forEach(i => {
+    items.forEach((i) => {
       this.applyDefaults(i, defaultItem);
       this.setParent(i);
     });
@@ -284,19 +286,19 @@ export class NbMenuInternalService {
   }
 
   itemHover(item: NbMenuItem, tag?: string) {
-    itemHover$.next({tag, item});
+    itemHover$.next({ tag, item });
   }
 
   submenuToggle(item: NbMenuItem, tag?: string) {
-    submenuToggle$.next({tag, item});
+    submenuToggle$.next({ tag, item });
   }
 
   itemSelect(item: NbMenuItem, tag?: string) {
-    itemSelect$.next({tag, item});
+    itemSelect$.next({ tag, item });
   }
 
   itemClick(item: NbMenuItem, tag?: string) {
-    itemClick$.next({tag, item});
+    itemClick$.next({ tag, item });
   }
 
   /**
@@ -336,7 +338,7 @@ export class NbMenuInternalService {
       }
 
       if (item.expanded) {
-        collapsedItems.push(item)
+        collapsedItems.push(item);
       }
       item.expanded = false;
 
@@ -349,18 +351,20 @@ export class NbMenuInternalService {
   }
 
   private applyDefaults(item, defaultItem) {
-    const menuItem = {...item};
+    const menuItem = { ...item };
     Object.assign(item, defaultItem, menuItem);
-    item.children && item.children.forEach(child => {
-      this.applyDefaults(child, defaultItem);
-    });
+    item.children &&
+      item.children.forEach((child) => {
+        this.applyDefaults(child, defaultItem);
+      });
   }
 
   private setParent(item: NbMenuItem) {
-    item.children && item.children.forEach(child => {
-      child.parent = item;
-      this.setParent(child);
-    });
+    item.children &&
+      item.children.forEach((child) => {
+        child.parent = item;
+        this.setParent(child);
+      });
   }
 
   /**
@@ -371,7 +375,7 @@ export class NbMenuInternalService {
   private findItemByUrl(items: NbMenuItem[]): NbMenuItem | undefined {
     let selectedItem;
 
-    items.some(item => {
+    items.some((item) => {
       if (item.children) {
         selectedItem = this.findItemByUrl(item.children);
       }
diff --git a/src/playground/with-layout/menu/menu-autocollapse.component.ts b/src/playground/with-layout/menu/menu-autocollapse.component.ts
index 41bc45cb54..584496ea46 100644
--- a/src/playground/with-layout/menu/menu-autocollapse.component.ts
+++ b/src/playground/with-layout/menu/menu-autocollapse.component.ts
@@ -8,17 +8,16 @@ import { ChangeDetectionStrategy, Component } from '@angular/core';
 import { NbMenuItem } from '@nebular/theme';
 
 @Component({
-  selector: 'nb-menu-autocollapse',
+  selector: 'npg-menu-autocollapse',
   changeDetection: ChangeDetectionStrategy.OnPush,
   templateUrl: './menu-autocollapse.component.html',
 })
-
 export class MenuAutoCollapseComponent {
-
   items: NbMenuItem[] = [
     {
       title: 'Profile',
       expanded: true,
+      ariaRole: 'button',
       children: [
         {
           title: 'Change Password',
@@ -33,6 +32,7 @@ export class MenuAutoCollapseComponent {
     },
     {
       title: 'Shopping Bag',
+      ariaRole: 'button',
       children: [
         {
           title: 'First Product',
@@ -47,6 +47,7 @@ export class MenuAutoCollapseComponent {
     },
     {
       title: 'Orders',
+      ariaRole: 'button',
       children: [
         {
           title: 'First Order',