fix(cdk-experimental/menu): fix issue where left/right on standalone menu closes it (#24696)

* refactor(cdk-experimental/menu): make MenuStack required

* fix(cdk-experimental/menu): fix issue where left/right on standalone menu closes it

also consolidates some shared trigger logic under the common base class

* fixup! fix(cdk-experimental/menu): fix issue where left/right on standalone menu closes it

* fixup! fix(cdk-experimental/menu): fix issue where left/right on standalone menu closes it

* fixup! fix(cdk-experimental/menu): fix issue where left/right on standalone menu closes it

Author: mmalerba

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

@@ -3,11 +3,12 @@ import {CdkMenuModule} from './menu-module';
 import {TestBed, waitForAsync, ComponentFixture} from '@angular/core/testing';
 import {CdkMenu} from './menu';
 import {CdkContextMenuTrigger} from './context-menu';
-import {dispatchMouseEvent} from '../../cdk/testing/private';
+import {dispatchKeyboardEvent, dispatchMouseEvent} from '../../cdk/testing/private';
 import {By} from '@angular/platform-browser';
 import {CdkMenuItem} from './menu-item';
 import {CdkMenuItemTrigger} from './menu-item-trigger';
 import {CdkMenuBar} from './menu-bar';
+import {LEFT_ARROW, RIGHT_ARROW} from '@angular/cdk/keycodes';
 
 describe('CdkContextMenuTrigger', () => {
   describe('with simple context menu trigger', () => {
@@ -126,6 +127,22 @@ describe('CdkContextMenuTrigger', () => {
       openContextMenu();
       expect(document.activeElement!.id).toEqual('first_menu_item');
     });
+
+    it('should not close context menu when pressing left arrow', () => {
+      openContextMenu();
+      expect(document.activeElement!.id).toEqual('first_menu_item');
+      dispatchKeyboardEvent(document.activeElement!, 'keydown', LEFT_ARROW, 'ArrowLeft');
+      fixture.detectChanges();
+      expect(getContextMenu()).toBeDefined();
+    });
+
+    it('should not close context menu when pressing right arrow', () => {
+      openContextMenu();
+      expect(document.activeElement!.id).toEqual('first_menu_item');
+      dispatchKeyboardEvent(document.activeElement!, 'keydown', RIGHT_ARROW, 'ArrowRight');
+      fixture.detectChanges();
+      expect(getContextMenu()).toBeDefined();
+    });
   });
 
   describe('nested context menu triggers', () => {

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

@@ -8,29 +8,24 @@
 
 import {
   Directive,
-  EventEmitter,
   Inject,
   Injectable,
   Injector,
   Input,
   OnDestroy,
   Optional,
-  Output,
-  TemplateRef,
   ViewContainerRef,
 } from '@angular/core';
 import {Directionality} from '@angular/cdk/bidi';
 import {
-  ConnectedPosition,
   FlexibleConnectedPositionStrategy,
   Overlay,
   OverlayConfig,
-  OverlayRef,
   STANDARD_DROPDOWN_BELOW_POSITIONS,
 } from '@angular/cdk/overlay';
 import {Portal, TemplatePortal} from '@angular/cdk/portal';
 import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion';
-import {merge, partition, Subject} from 'rxjs';
+import {merge, partition} from 'rxjs';
 import {skip, takeUntil} from 'rxjs/operators';
 import {MENU_STACK, MenuStack} from './menu-stack';
 import {isClickInsideMenuOverlay} from './menu-item-trigger';
@@ -76,25 +71,14 @@ export type ContextMenuCoordinates = {x: number; y: number};
   host: {
     '(contextmenu)': '_openOnContextMenu($event)',
   },
+  inputs: ['_menuTemplateRef: cdkContextMenuTriggerFor', 'menuPosition: cdkContextMenuPosition'],
+  outputs: ['opened: cdkContextMenuOpened', 'closed: cdkContextMenuClosed'],
   providers: [
     {provide: MENU_TRIGGER, useExisting: CdkContextMenuTrigger},
     {provide: MENU_STACK, useClass: MenuStack},
   ],
 })
 export class CdkContextMenuTrigger extends MenuTrigger implements OnDestroy {
-  /** Template reference variable to the menu to open on right click. */
-  @Input('cdkContextMenuTriggerFor')
-  private _menuTemplateRef: TemplateRef<unknown>;
-
-  /** A list of preferred menu positions to be used when constructing the `FlexibleConnectedPositionStrategy` for this trigger's menu. */
-  @Input('cdkMenuPosition') menuPosition: ConnectedPosition[];
-
-  /** Emits when the attached menu is requested to open. */
-  @Output('cdkContextMenuOpened') readonly opened: EventEmitter<void> = new EventEmitter();
-
-  /** Emits when the attached menu is requested to close. */
-  @Output('cdkContextMenuClosed') readonly closed: EventEmitter<void> = new EventEmitter();
-
   /** Whether the context menu should be disabled. */
   @Input('cdkContextMenuDisabled')
   get disabled(): boolean {
@@ -105,18 +89,6 @@ export class CdkContextMenuTrigger extends MenuTrigger implements OnDestroy {
   }
   private _disabled = false;
 
-  /** A reference to the overlay which manages the triggered menu. */
-  private _overlayRef: OverlayRef | null = null;
-
-  /** The content of the menu panel opened by this trigger. */
-  private _menuPortal: TemplatePortal;
-
-  /** Emits when the element is destroyed. */
-  private readonly _destroyed = new Subject<void>();
-
-  /** Emits when the outside pointer events listener on the overlay should be stopped. */
-  private readonly _stopOutsideClicksListener = merge(this.closed, this._destroyed);
-
   constructor(
     injector: Injector,
     protected readonly _viewContainerRef: ViewContainerRef,
@@ -199,11 +171,6 @@ export class CdkContextMenuTrigger extends MenuTrigger implements OnDestroy {
     }
   }
 
-  /** Whether the attached menu is open. */
-  isOpen() {
-    return !!this._overlayRef?.hasAttached();
-  }
-
   /**
    * Get the configuration object used to create the overlay.
    * @param coordinates the location to place the opened menu
@@ -277,19 +244,4 @@ export class CdkContextMenuTrigger extends MenuTrigger implements OnDestroy {
       });
     }
   }
-
-  ngOnDestroy() {
-    this._destroyOverlay();
-
-    this._destroyed.next();
-    this._destroyed.complete();
-  }
-
-  /** Destroy and unset the overlay reference it if exists. */
-  private _destroyOverlay() {
-    if (this._overlayRef) {
-      this._overlayRef.dispose();
-      this._overlayRef = null;
-    }
-  }
 }

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

@@ -509,13 +509,13 @@ describe('MenuBar', () => {
             .queryAll(By.directive(CdkMenuItem))[0]
             .injector.get(CdkMenuItem)
             .getMenuTrigger()!
-            .openMenu();
+            .open();
           detectChanges();
           fixture.debugElement
             .queryAll(By.directive(CdkMenuItem))[2]
             .injector.get(CdkMenuItem)
             .getMenuTrigger()!
-            .openMenu();
+            .open();
           detectChanges();
 
           fileMenuNativeItems[0].focus();

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

@@ -44,7 +44,7 @@ import {CdkMenuBase} from './menu-base';
   providers: [
     {provide: CdkMenuGroup, useExisting: CdkMenuBar},
     {provide: CDK_MENU, useExisting: CdkMenuBar},
-    {provide: MENU_STACK, useClass: MenuStack},
+    {provide: MENU_STACK, useFactory: () => MenuStack.inline()},
   ],
 })
 export class CdkMenuBar extends CdkMenuBase implements AfterContentInit, OnDestroy {
@@ -97,23 +97,23 @@ export class CdkMenuBar extends CdkMenuBase implements AfterContentInit, OnDestr
           event.preventDefault();
 
           const prevIsOpen = keyManager.activeItem?.isMenuOpen();
-          keyManager.activeItem?.getMenuTrigger()?.closeMenu();
+          keyManager.activeItem?.getMenuTrigger()?.close();
 
           keyManager.setFocusOrigin('keyboard');
           keyManager.onKeydown(event);
           if (prevIsOpen) {
-            keyManager.activeItem?.getMenuTrigger()?.openMenu();
+            keyManager.activeItem?.getMenuTrigger()?.open();
           }
         }
         break;
 
       case ESCAPE:
         event.preventDefault();
-        keyManager.activeItem?.getMenuTrigger()?.closeMenu();
+        keyManager.activeItem?.getMenuTrigger()?.close();
         break;
 
       case TAB:
-        keyManager.activeItem?.getMenuTrigger()?.closeMenu();
+        keyManager.activeItem?.getMenuTrigger()?.close();
         break;
 
       default:
@@ -146,13 +146,13 @@ export class CdkMenuBar extends CdkMenuBase implements AfterContentInit, OnDestr
       case FocusNext.nextItem:
         keyManager.setFocusOrigin('keyboard');
         keyManager.setNextItemActive();
-        keyManager.activeItem?.getMenuTrigger()?.openMenu();
+        keyManager.activeItem?.getMenuTrigger()?.open();
         break;
 
       case FocusNext.previousItem:
         keyManager.setFocusOrigin('keyboard');
         keyManager.setPreviousItemActive();
-        keyManager.activeItem?.getMenuTrigger()?.openMenu();
+        keyManager.activeItem?.getMenuTrigger()?.open();
         break;
 
       case FocusNext.currentItem:

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

@@ -40,7 +40,7 @@ export abstract class CdkMenuBase
    * Sets the aria-orientation attribute and determines where menus will be opened.
    * Does not affect styling/layout.
    */
-  readonly orientation: 'horizontal' | 'vertical' = 'vertical';
+  orientation: 'horizontal' | 'vertical' = 'vertical';
 
   /** All child MenuItem elements nested in this Menu. */
   @ContentChildren(CdkMenuItem, {descendants: true})
@@ -60,7 +60,7 @@ export abstract class CdkMenuBase
 
   protected constructor(
     readonly _elementRef: ElementRef<HTMLElement>,
-    @Optional() @Inject(MENU_STACK) readonly menuStack?: MenuStack,
+    @Inject(MENU_STACK) readonly menuStack: MenuStack,
     @Optional() protected readonly dir?: Directionality,
   ) {
     super();
@@ -109,7 +109,7 @@ export abstract class CdkMenuBase
     const keyManager = this.keyManager;
     const trigger = this.openItem;
     if (menu === trigger?.getMenuTrigger()?.getMenu()) {
-      trigger?.getMenuTrigger()?.closeMenu();
+      trigger?.getMenuTrigger()?.close();
       // If the user has moused over a sibling item we want to focus the element under mouse focus
       // not the trigger which previously opened the now closed menu.
       if (trigger) {
@@ -155,7 +155,7 @@ export abstract class CdkMenuBase
 
   /** Subscribe to the MenuStack close and empty observables. */
   private _subscribeToMenuStackClosed() {
-    this.menuStack?.closed
+    this.menuStack.closed
       .pipe(takeUntil(this.destroyed))
       .subscribe(item => this.closeOpenMenu(item));
   }

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

@@ -5,6 +5,7 @@ import {CdkMenuModule} from './menu-module';
 import {CdkMenuItemCheckbox} from './menu-item-checkbox';
 import {CDK_MENU} from './menu-interface';
 import {CdkMenu} from './menu';
+import {MENU_STACK, MenuStack} from '@angular/cdk-experimental/menu/menu-stack';
 
 describe('MenuItemCheckbox', () => {
   let fixture: ComponentFixture<SingleCheckboxButton>;
@@ -17,6 +18,7 @@ describe('MenuItemCheckbox', () => {
       declarations: [SingleCheckboxButton],
       providers: [
         {provide: CDK_MENU, useClass: CdkMenu},
+        {provide: MENU_STACK, useClass: MenuStack},
         // View engine can't figure out the ElementRef to inject so we need to provide a fake
         {provide: ElementRef, useValue: new ElementRef<null>(null)},
       ],

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

@@ -6,6 +6,7 @@ import {CdkMenuModule} from './menu-module';
 import {CdkMenuItemRadio} from './menu-item-radio';
 import {CDK_MENU} from './menu-interface';
 import {CdkMenu} from './menu';
+import {MENU_STACK, MenuStack} from '@angular/cdk-experimental/menu/menu-stack';
 
 describe('MenuItemRadio', () => {
   let fixture: ComponentFixture<SimpleRadioButton>;
@@ -21,6 +22,7 @@ describe('MenuItemRadio', () => {
       providers: [
         {provide: UniqueSelectionDispatcher, useValue: selectionDispatcher},
         {provide: CDK_MENU, useClass: CdkMenu},
+        {provide: MENU_STACK, useClass: MenuStack},
         // View engine can't figure out the ElementRef to inject so we need to provide a fake
         {provide: ElementRef, useValue: new ElementRef<null>(null)},
       ],

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

@@ -44,7 +44,7 @@ export class CdkMenuItemRadio extends CdkMenuItemSelectable implements OnDestroy
     private readonly _selectionDispatcher: UniqueSelectionDispatcher,
     element: ElementRef<HTMLElement>,
     ngZone: NgZone,
-    @Optional() @Inject(MENU_STACK) menuStack?: MenuStack,
+    @Inject(MENU_STACK) menuStack: MenuStack,
     @Optional() @Inject(CDK_MENU) parentMenu?: Menu,
     @Optional() @Inject(MENU_AIM) menuAim?: MenuAim,
     @Optional() dir?: Directionality,