fix(cdk/menu): unable to close child menu with disabled items when us…

…ing keyboard Fixes the issue where the child menu remained open when all of its items were disabled.
behzadmehrabi · Nov 10, 2023 · b5129f4 · b5129f4
1 parent d902f8d
commit b5129f4
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 1 deletion.
diff --git a/src/cdk/menu/menu-base.ts b/src/cdk/menu/menu-base.ts
@@ -123,6 +123,13 @@ export abstract class CdkMenuBase
   focusFirstItem(focusOrigin: FocusOrigin = 'program') {
     this.keyManager.setFocusOrigin(focusOrigin);
     this.keyManager.setFirstItemActive();
+
+    // If there's no active item at this point, it means that all the items are disabled.
+    // Move focus to the menuPanel panel so keyboard events like Escape and closing child menus using arrow keys still works.
+    // Also, this will give _some_ feedback to screen readers.
+    if (!this.nativeElement.contains(document.activeElement)) {
+      this.nativeElement.focus();
+    }
   }
 
   /**

diff --git a/src/cdk/menu/menu.spec.ts b/src/cdk/menu/menu.spec.ts
@@ -7,7 +7,7 @@ import {
   waitForAsync,
 } from '@angular/core/testing';
 import {Component, ElementRef, QueryList, ViewChild, ViewChildren} from '@angular/core';
-import {TAB} from '@angular/cdk/keycodes';
+import {LEFT_ARROW, RIGHT_ARROW, TAB} from '@angular/cdk/keycodes';
 import {
   createMouseEvent,
   dispatchEvent,
@@ -135,6 +135,8 @@ describe('Menu', () => {
 
       let nativeShareTrigger: HTMLElement | undefined;
 
+      let nativeDownloadTrigger: HTMLElement | undefined;
+
       let nativeMenus: HTMLElement[];
 
       beforeEach(waitForAsync(() => {
@@ -163,6 +165,8 @@ describe('Menu', () => {
 
         nativeShareTrigger = fixture.componentInstance.nativeShareTrigger?.nativeElement;
 
+        nativeDownloadTrigger = fixture.componentInstance.nativeDownloadTrigger?.nativeElement;
+
         nativeMenus = fixture.componentInstance.menus.map(m => m.nativeElement);
       }
 
@@ -315,6 +319,20 @@ describe('Menu', () => {
         expect(numEnters).toBeGreaterThan(2);
         expect(nativeMenus.length).toBe(1);
       }));
+
+      it('should close the download menu when it is open and right arrow key is pressed', () => {
+        openFileMenu();
+        if (nativeDownloadTrigger) {
+          dispatchKeyboardEvent(nativeDownloadTrigger, 'keydown', RIGHT_ARROW);
+          detectChanges();
+
+          const downloadMenu = nativeMenus[1];
+          dispatchKeyboardEvent(downloadMenu, 'keydown', LEFT_ARROW);
+          detectChanges();
+
+          expect(nativeMenus.length).toBe(1);
+        }
+      });
     });
 
     describe('with rtl layout and menu at bottom of page moving up and left', () => {
@@ -327,6 +345,8 @@ describe('Menu', () => {
 
       let nativeShareTrigger: HTMLElement | undefined;
 
+      let nativeDownloadTrigger: HTMLElement | undefined;
+
       let nativeMenus: HTMLElement[];
 
       beforeEach(waitForAsync(() => {
@@ -363,6 +383,8 @@ describe('Menu', () => {
 
         nativeShareTrigger = fixture.componentInstance.nativeShareTrigger?.nativeElement;
 
+        nativeDownloadTrigger = fixture.componentInstance.nativeDownloadTrigger?.nativeElement;
+
         nativeMenus = fixture.componentInstance.menus.map(m => m.nativeElement);
       }
 
@@ -504,6 +526,20 @@ describe('Menu', () => {
         expect(nativeMenus.length).toBe(2);
         expect(nativeMenus[1].id).toBe('edit_menu');
       }));
+
+      it('should close the download menu when it is open and right arrow key is pressed in rtl', () => {
+        openFileMenu();
+        if (nativeDownloadTrigger) {
+          dispatchKeyboardEvent(nativeDownloadTrigger, 'keydown', LEFT_ARROW);
+          detectChanges();
+
+          const downloadMenu = nativeMenus[1];
+          dispatchKeyboardEvent(downloadMenu, 'keydown', RIGHT_ARROW);
+          detectChanges();
+
+          expect(nativeMenus.length).toBe(1);
+        }
+      });
     });
   });
 });
@@ -554,6 +590,7 @@ class InlineMenu {}
       >
         <button #edit_trigger cdkMenuItem [cdkMenuTriggerFor]="edit">Edit</button>
         <button #share_trigger cdkMenuItem [cdkMenuTriggerFor]="share">Share</button>
+        <button #download_trigger cdkMenuItem [cdkMenuTriggerFor]="download">Download</button>
         <button cdkMenuItem>Open</button>
         <button cdkMenuItem>Rename</button>
         <button cdkMenuItem>Print</button>
@@ -587,12 +624,26 @@ class InlineMenu {}
         <button cdkMenuItem>Twitter</button>
       </div>
     </ng-template>
+
+    <ng-template #download>
+      <div
+        id="download_menu"
+        style="display: flex; flex-direction: column;"
+        cdkMenu
+        cdkTargetMenuAim
+      >
+        <button cdkMenuItem disabled>Via Server 1</button>
+        <button cdkMenuItem disabled>Via Server 2</button>
+      </div>
+    </ng-template>
   `,
 })
 class WithComplexNestedMenus {
   @ViewChild('file_trigger', {read: ElementRef}) nativeFileTrigger: ElementRef<HTMLElement>;
   @ViewChild('edit_trigger', {read: ElementRef}) nativeEditTrigger?: ElementRef<HTMLElement>;
   @ViewChild('share_trigger', {read: ElementRef}) nativeShareTrigger?: ElementRef<HTMLElement>;
+  @ViewChild('download_trigger', {read: ElementRef})
+  nativeDownloadTrigger?: ElementRef<HTMLElement>;
 
   @ViewChildren(CdkMenu) menus: QueryList<CdkMenu>;
 }
@@ -615,6 +666,7 @@ class WithComplexNestedMenus {
         <button cdkMenuItem>Open</button>
         <button #share_trigger cdkMenuItem [cdkMenuTriggerFor]="share">Share</button>
         <button #edit_trigger cdkMenuItem [cdkMenuTriggerFor]="edit">Edit</button>
+        <button #download_trigger cdkMenuItem [cdkMenuTriggerFor]="download">Download</button>
       </div>
     </ng-template>
 
@@ -646,12 +698,25 @@ class WithComplexNestedMenus {
         <button cdkMenuItem>Facebook</button>
       </div>
     </ng-template>
+    <ng-template #download>
+      <div
+        id="download_menu"
+        style="display: flex; flex-direction: column;"
+        cdkMenu
+        cdkTargetMenuAim
+      >
+        <button cdkMenuItem disabled>Via Server 1</button>
+        <button cdkMenuItem disabled>Via Server 2</button>
+      </div>
+    </ng-template>
   `,
 })
 class WithComplexNestedMenusOnBottom {
   @ViewChild('file_trigger', {read: ElementRef}) nativeFileTrigger: ElementRef<HTMLElement>;
   @ViewChild('edit_trigger', {read: ElementRef}) nativeEditTrigger?: ElementRef<HTMLElement>;
   @ViewChild('share_trigger', {read: ElementRef}) nativeShareTrigger?: ElementRef<HTMLElement>;
+  @ViewChild('download_trigger', {read: ElementRef})
+  nativeDownloadTrigger?: ElementRef<HTMLElement>;
 
   @ViewChildren(CdkMenu) menus: QueryList<CdkMenu>;
 }
diff --git a/src/cdk/menu/menu.ts b/src/cdk/menu/menu.ts
@@ -74,9 +74,42 @@ export class CdkMenu extends CdkMenuBase implements AfterContentInit, OnDestroy
     const keyManager = this.keyManager;
     switch (event.keyCode) {
       case LEFT_ARROW:
+        if (!hasModifierKey(event)) {
+          event.preventDefault();
+
+          if (
+            this._parentTrigger &&
+            this.nativeElement === document.activeElement &&
+            this.dir?.value !== 'rtl'
+          ) {
+            this.menuStack.close(this, {
+              focusNextOnEmpty: FocusNext.currentItem,
+              focusParentTrigger: true,
+            });
+            return;
+          }
+
+          keyManager.setFocusOrigin('keyboard');
+          keyManager.onKeydown(event);
+        }
+        break;
+
       case RIGHT_ARROW:
         if (!hasModifierKey(event)) {
           event.preventDefault();
+
+          if (
+            this._parentTrigger &&
+            this.nativeElement === document.activeElement &&
+            this.dir?.value === 'rtl'
+          ) {
+            this.menuStack.close(this, {
+              focusNextOnEmpty: FocusNext.currentItem,
+              focusParentTrigger: true,
+            });
+            return;
+          }
+
           keyManager.setFocusOrigin('keyboard');
           keyManager.onKeydown(event);
         }