fixup! fix(cdk-experimental/menu): API, code, and docs cleanup pass

Author: mmalerba

src/cdk-experimental/menu/context-menu-trigger.ts

@@ -212,7 +212,8 @@ export class CdkContextMenuTrigger extends CdkMenuTriggerBase implements OnDestr
   private _open(coordinates: ContextMenuCoordinates, ignoreFirstOutsideAuxClick: boolean) {
     if (this.disabled) {
       return;
-    } else if (this.isOpen()) {
+    }
+    if (this.isOpen()) {
       // since we're moving this menu we need to close any submenus first otherwise they end up
       // disconnected from this one.
       this.menuStack.closeSubMenuOf(this.childMenu!);

src/cdk-experimental/menu/menu-bar.ts

@@ -16,7 +16,15 @@ import {
   Self,
 } from '@angular/core';
 import {Directionality} from '@angular/cdk/bidi';
-import {DOWN_ARROW, ESCAPE, LEFT_ARROW, RIGHT_ARROW, TAB, UP_ARROW} from '@angular/cdk/keycodes';
+import {
+  DOWN_ARROW,
+  ESCAPE,
+  hasModifierKey,
+  LEFT_ARROW,
+  RIGHT_ARROW,
+  TAB,
+  UP_ARROW,
+} from '@angular/cdk/keycodes';
 import {takeUntil} from 'rxjs/operators';
 import {CdkMenuGroup} from './menu-group';
 import {CDK_MENU} from './menu-interface';
@@ -82,31 +90,37 @@ export class CdkMenuBar extends CdkMenuBase implements AfterContentInit {
       case DOWN_ARROW:
       case LEFT_ARROW:
       case RIGHT_ARROW:
-        const horizontalArrows = event.keyCode === LEFT_ARROW || event.keyCode === RIGHT_ARROW;
-        // For a horizontal menu if the left/right keys were clicked, or a vertical menu if the
-        // up/down keys were clicked: if the current menu is open, close it then focus and open the
-        // next  menu.
-        if (horizontalArrows) {
-          event.preventDefault();
+        if (!hasModifierKey(event)) {
+          const horizontalArrows = event.keyCode === LEFT_ARROW || event.keyCode === RIGHT_ARROW;
+          // For a horizontal menu if the left/right keys were clicked, or a vertical menu if the
+          // up/down keys were clicked: if the current menu is open, close it then focus and open the
+          // next  menu.
+          if (horizontalArrows) {
+            event.preventDefault();
 
-          const prevIsOpen = keyManager.activeItem?.isMenuOpen();
-          keyManager.activeItem?.getMenuTrigger()?.close();
+            const prevIsOpen = keyManager.activeItem?.isMenuOpen();
+            keyManager.activeItem?.getMenuTrigger()?.close();
 
-          keyManager.setFocusOrigin('keyboard');
-          keyManager.onKeydown(event);
-          if (prevIsOpen) {
-            keyManager.activeItem?.getMenuTrigger()?.open();
+            keyManager.setFocusOrigin('keyboard');
+            keyManager.onKeydown(event);
+            if (prevIsOpen) {
+              keyManager.activeItem?.getMenuTrigger()?.open();
+            }
           }
         }
         break;
 
       case ESCAPE:
-        event.preventDefault();
-        keyManager.activeItem?.getMenuTrigger()?.close();
+        if (!hasModifierKey(event)) {
+          event.preventDefault();
+          keyManager.activeItem?.getMenuTrigger()?.close();
+        }
         break;
 
       case TAB:
-        keyManager.activeItem?.getMenuTrigger()?.close();
+        if (!hasModifierKey(event, 'altKey', 'metaKey', 'ctrlKey')) {
+          keyManager.activeItem?.getMenuTrigger()?.close();
+        }
         break;
 
       default:

src/cdk-experimental/menu/menu-base.ts

@@ -70,8 +70,11 @@ export abstract class CdkMenuBase
   /** The menu's native DOM host element. */
   readonly nativeElement: HTMLElement;
 
-  /** Whether this menu's menu stack has focus. */
-  _tabIndex: number | null;
+  /** The tabindex for this menu. */
+  get _tabIndex() {
+    const tabindexIfInline = this._menuStackHasFocus ? -1 : 0;
+    return this.isInline ? tabindexIfInline : null;
+  }
 
   /** Handles keyboard events for the menu. */
   protected keyManager: FocusKeyManager<CdkMenuItem>;
@@ -85,6 +88,9 @@ export abstract class CdkMenuBase
   /** Tracks the users mouse movements over the menu. */
   protected pointerTracker?: PointerFocusTracker<CdkMenuItem>;
 
+  /** Whether this menu's menu stack has focus. */
+  private _menuStackHasFocus = false;
+
   protected constructor(
     /** The host element. */
     elementRef: ElementRef<HTMLElement>,
@@ -105,7 +111,6 @@ export abstract class CdkMenuBase
     if (!this.isInline) {
       this.menuStack.push(this);
     }
-    this._calculateTabIndex(false);
     this._setKeyManager();
     this._subscribeToMenuStackHasFocus();
     this._subscribeToMenuOpen();
@@ -207,17 +212,11 @@ export abstract class CdkMenuBase
   private _subscribeToMenuStackHasFocus() {
     if (this.isInline) {
       this.menuStack.hasFocus.pipe(takeUntil(this.destroyed)).subscribe(hasFocus => {
-        this._calculateTabIndex(hasFocus);
+        this._menuStackHasFocus = hasFocus;
       });
     }
   }
 
-  /** Calculate the tabindex for the menu host element. */
-  private _calculateTabIndex(menuStackHasFocus: boolean) {
-    const tabindexIfInline = menuStackHasFocus ? -1 : 0;
-    this._tabIndex = this.isInline ? tabindexIfInline : null;
-  }
-
   /**
    * Set the PointerFocusTracker and ensure that when mouse focus changes the key manager is updated
    * with the latest menu item under mouse focus.

src/cdk-experimental/menu/menu-item.spec.ts

@@ -98,7 +98,7 @@ describe('MenuItem', () => {
     it('should get the text for menu item with a single nested mat icon component', () => {
       const fixture = createComponent(MenuItemWithIcon);
       expect(menuItem.getLabel()).toEqual('unicorn Click me!');
-      fixture.componentInstance.typeahead = 'Click me!';
+      fixture.componentInstance.typeaheadLabel = 'Click me!';
       fixture.detectChanges();
       expect(menuItem.getLabel()).toEqual('Click me!');
     });
@@ -109,7 +109,7 @@ describe('MenuItem', () => {
       () => {
         const fixture = createComponent(MenuItemWithIconClass);
         expect(menuItem.getLabel()).toEqual('unicorn Click me!');
-        fixture.componentInstance.typeahead = 'Click me!';
+        fixture.componentInstance.typeaheadLabel = 'Click me!';
         fixture.detectChanges();
         expect(menuItem.getLabel()).toEqual('Click me!');
       },
@@ -126,7 +126,7 @@ describe('MenuItem', () => {
       () => {
         const fixture = createComponent(MenuItemWithMultipleNestings);
         expect(menuItem.getLabel()).toEqual('unicorn Click menume!');
-        fixture.componentInstance.typeahead = 'Click me!';
+        fixture.componentInstance.typeaheadLabel = 'Click me!';
         fixture.detectChanges();
         expect(menuItem.getLabel()).toEqual('Click me!');
       },
@@ -141,7 +141,7 @@ class SingleMenuItem {}
 
 @Component({
   template: `
-    <button cdkMenuItem [cdkMenuItemTypeahead]="typeahead">
+    <button cdkMenuItem [cdkMenuitemTypeaheadLabel]="typeahead">
       <mat-icon>unicorn</mat-icon>
       Click me!
     </button>
@@ -153,7 +153,7 @@ class MenuItemWithIcon {
 
 @Component({
   template: `
-    <button cdkMenuItem [cdkMenuItemTypeahead]="typeahead">
+    <button cdkMenuItem [cdkMenuitemTypeaheadLabel]="typeahead">
       <div class="material-icons">unicorn</div>
       Click me!
     </button>
@@ -170,7 +170,7 @@ class MenuItemWithBoldElement {}
 
 @Component({
   template: `
-    <button cdkMenuItem [cdkMenuItemTypeahead]="typeahead">
+    <button cdkMenuItem [cdkMenuitemTypeaheadLabel]="typeahead">
       <div>
         <div class="material-icons">unicorn</div>
         <div>

src/cdk-experimental/menu/menu-item.ts

@@ -20,7 +20,7 @@ import {
 } from '@angular/core';
 import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion';
 import {FocusableOption} from '@angular/cdk/a11y';
-import {ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE} from '@angular/cdk/keycodes';
+import {ENTER, hasModifierKey, LEFT_ARROW, RIGHT_ARROW, SPACE} from '@angular/cdk/keycodes';
 import {Directionality} from '@angular/cdk/bidi';
 import {fromEvent, Subject} from 'rxjs';
 import {filter, takeUntil} from 'rxjs/operators';
@@ -64,7 +64,7 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
    * The text used to locate this item during menu typeahead. If not specified,
    * the `textContent` of the item will be used.
    */
-  @Input('cdkMenuItemTypeahead') typeahead: string;
+  @Input('cdkMenuitemTypeaheadLabel') typeaheadLabel: string | null;
 
   /**
    * If this MenuItem is a regular MenuItem, outputs when it is triggered by a keyboard or mouse
@@ -113,6 +113,7 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
 
   ngOnDestroy() {
     this.destroyed.next();
+    this.destroyed.complete();
   }
 
   /** Place focus on the element. */
@@ -156,7 +157,7 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
 
   /** Get the label for this element which is required by the FocusableOption interface. */
   getLabel(): string {
-    return this.typeahead || this._elementRef.nativeElement.textContent?.trim() || '';
+    return this.typeaheadLabel || this._elementRef.nativeElement.textContent?.trim() || '';
   }
 
   /** Reset the tabindex to -1. */
@@ -191,26 +192,32 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
     switch (event.keyCode) {
       case SPACE:
       case ENTER:
-        event.preventDefault();
-        this.trigger({keepOpen: event.keyCode === SPACE && !this.closeOnSpacebarTrigger});
+        if (!hasModifierKey(event)) {
+          event.preventDefault();
+          this.trigger({keepOpen: event.keyCode === SPACE && !this.closeOnSpacebarTrigger});
+        }
         break;
 
       case RIGHT_ARROW:
-        if (this._parentMenu && this._isParentVertical()) {
-          if (this._dir?.value !== 'rtl') {
-            this._forwardArrowPressed(event);
-          } else {
-            this._backArrowPressed(event);
+        if (!hasModifierKey(event)) {
+          if (this._parentMenu && this._isParentVertical()) {
+            if (this._dir?.value !== 'rtl') {
+              this._forwardArrowPressed(event);
+            } else {
+              this._backArrowPressed(event);
+            }
           }
         }
         break;
 
       case LEFT_ARROW:
-        if (this._parentMenu && this._isParentVertical()) {
-          if (this._dir?.value !== 'rtl') {
-            this._backArrowPressed(event);
-          } else {
-            this._forwardArrowPressed(event);
+        if (!hasModifierKey(event)) {
+          if (this._parentMenu && this._isParentVertical()) {
+            if (this._dir?.value !== 'rtl') {
+              this._backArrowPressed(event);
+            } else {
+              this._forwardArrowPressed(event);
+            }
           }
         }
         break;

src/cdk-experimental/menu/menu-trigger.ts

@@ -25,7 +25,15 @@ import {
   STANDARD_DROPDOWN_ADJACENT_POSITIONS,
   STANDARD_DROPDOWN_BELOW_POSITIONS,
 } from '@angular/cdk/overlay';
-import {DOWN_ARROW, ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE, UP_ARROW} from '@angular/cdk/keycodes';
+import {
+  DOWN_ARROW,
+  ENTER,
+  hasModifierKey,
+  LEFT_ARROW,
+  RIGHT_ARROW,
+  SPACE,
+  UP_ARROW,
+} from '@angular/cdk/keycodes';
 import {fromEvent} from 'rxjs';
 import {filter, takeUntil} from 'rxjs/operators';
 import {CDK_MENU, Menu} from './menu-interface';
@@ -130,35 +138,43 @@ export class CdkMenuTrigger extends CdkMenuTriggerBase implements OnDestroy {
     switch (keyCode) {
       case SPACE:
       case ENTER:
-        event.preventDefault();
-        this.toggle();
-        this.childMenu?.focusFirstItem('keyboard');
+        if (!hasModifierKey(event)) {
+          event.preventDefault();
+          this.toggle();
+          this.childMenu?.focusFirstItem('keyboard');
+        }
         break;
 
       case RIGHT_ARROW:
-        if (this._parentMenu && isParentVertical && this._directionality?.value !== 'rtl') {
-          event.preventDefault();
-          this.open();
-          this.childMenu?.focusFirstItem('keyboard');
+        if (!hasModifierKey(event)) {
+          if (this._parentMenu && isParentVertical && this._directionality?.value !== 'rtl') {
+            event.preventDefault();
+            this.open();
+            this.childMenu?.focusFirstItem('keyboard');
+          }
         }
         break;
 
       case LEFT_ARROW:
-        if (this._parentMenu && isParentVertical && this._directionality?.value === 'rtl') {
-          event.preventDefault();
-          this.open();
-          this.childMenu?.focusFirstItem('keyboard');
+        if (!hasModifierKey(event)) {
+          if (this._parentMenu && isParentVertical && this._directionality?.value === 'rtl') {
+            event.preventDefault();
+            this.open();
+            this.childMenu?.focusFirstItem('keyboard');
+          }
         }
         break;
 
       case DOWN_ARROW:
       case UP_ARROW:
-        if (!isParentVertical) {
-          event.preventDefault();
-          this.open();
-          keyCode === DOWN_ARROW
-            ? this.childMenu?.focusFirstItem('keyboard')
-            : this.childMenu?.focusLastItem('keyboard');
+        if (!hasModifierKey(event)) {
+          if (!isParentVertical) {
+            event.preventDefault();
+            this.open();
+            keyCode === DOWN_ARROW
+              ? this.childMenu?.focusFirstItem('keyboard')
+              : this.childMenu?.focusLastItem('keyboard');
+          }
         }
         break;
     }

src/cdk-experimental/menu/menu.ts

@@ -103,9 +103,11 @@ export class CdkMenu extends CdkMenuBase implements AfterContentInit, OnDestroy
     switch (event.keyCode) {
       case LEFT_ARROW:
       case RIGHT_ARROW:
-        event.preventDefault();
-        keyManager.setFocusOrigin('keyboard');
-        keyManager.onKeydown(event);
+        if (!hasModifierKey(event)) {
+          event.preventDefault();
+          keyManager.setFocusOrigin('keyboard');
+          keyManager.onKeydown(event);
+        }
         break;
 
       case ESCAPE:
@@ -119,7 +121,9 @@ export class CdkMenu extends CdkMenuBase implements AfterContentInit, OnDestroy
         break;
 
       case TAB:
-        this.menuStack.closeAll({focusParentTrigger: true});
+        if (!hasModifierKey(event, 'altKey', 'metaKey', 'ctrlKey')) {
+          this.menuStack.closeAll({focusParentTrigger: true});
+        }
         break;
 
       default:

src/material-experimental/menubar/menubar-item.ts

@@ -39,8 +39,8 @@ function removeIcons(element: Element) {
 })
 export class MatMenuBarItem extends CdkMenuItem {
   override getLabel(): string {
-    if (this.typeahead !== undefined) {
-      return this.typeahead;
+    if (this.typeaheadLabel !== undefined) {
+      return this.typeaheadLabel || '';
     }
     const clone = this._elementRef.nativeElement.cloneNode(true) as Element;
     removeIcons(clone);