refactor(cdk-experimental/menu): Break MenuItems into directives

src/cdk-experimental/menu/BUILD.bazel

@@ -11,6 +11,7 @@ ng_module(
     module_name = "@angular/cdk-experimental/menu",
     deps = [
         "//src/cdk/coercion",
+        "//src/cdk/collections",
         "@npm//@angular/core",
         "@npm//rxjs",
     ],
@@ -24,6 +25,7 @@ ng_test_library(
     ),
     deps = [
         ":menu",
+        "//src/cdk/collections",
         "@npm//@angular/platform-browser",
     ],
 )

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

@@ -1,14 +1,14 @@
 import {ComponentFixture, TestBed, async} from '@angular/core/testing';
 import {Component} from '@angular/core';
+import {By} from '@angular/platform-browser';
 import {CdkMenuBar} from './menu-bar';
 import {CdkMenuModule} from './menu-module';
-import {CdkMenuItem} from './menu-item';
-import {By} from '@angular/platform-browser';
+import {CdkMenuItemRadio} from './menu-item-radio';
 
 describe('MenuBar', () => {
   describe('as radio group', () => {
     let fixture: ComponentFixture<MenuBarRadioGroup>;
-    let menuItems: CdkMenuItem[];
+    let menuItems: CdkMenuItemRadio[];
 
     beforeEach(async(() => {
       TestBed.configureTestingModule({
@@ -20,8 +20,8 @@ describe('MenuBar', () => {
       fixture.detectChanges();
 
       menuItems = fixture.debugElement
-        .queryAll(By.directive(CdkMenuItem))
-        .map(element => element.injector.get(CdkMenuItem));
+        .queryAll(By.directive(CdkMenuItemRadio))
+        .map(element => element.injector.get(CdkMenuItemRadio));
     }));
 
     it('should toggle menuitemradio items', () => {
@@ -37,7 +37,7 @@ describe('MenuBar', () => {
 
   describe('radiogroup change events', () => {
     let fixture: ComponentFixture<MenuBarRadioGroup>;
-    let menuItems: CdkMenuItem[];
+    let menuItems: CdkMenuItemRadio[];
 
     beforeEach(async(() => {
       TestBed.configureTestingModule({
@@ -50,8 +50,8 @@ describe('MenuBar', () => {
       fixture.detectChanges();
 
       menuItems = fixture.debugElement
-        .queryAll(By.directive(CdkMenuItem))
-        .map(element => element.injector.get(CdkMenuItem));
+        .queryAll(By.directive(CdkMenuItemRadio))
+        .map(element => element.injector.get(CdkMenuItemRadio));
     }));
 
     it('should emit on click', () => {
@@ -73,12 +73,12 @@ describe('MenuBar', () => {
   template: `
     <ul cdkMenuBar>
       <li role="none">
-        <button checked="true" role="menuitemradio" cdkMenuItem>
+        <button checked="true" cdkMenuItemRadio>
           first
         </button>
       </li>
       <li role="none">
-        <button role="menuitemradio" cdkMenuItem>
+        <button cdkMenuItemRadio>
           second
         </button>
       </li>

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

@@ -11,7 +11,7 @@ import {CdkMenuGroup} from './menu-group';
 
 /**
  * Directive applied to an element which configures it as a MenuBar by setting the appropriate
- * role, aria attributes, and accessable keyboard and mouse handling logic. The component that
+ * role, aria attributes, and accessible keyboard and mouse handling logic. The component that
  * this directive is applied to should contain components marked with CdkMenuItem.
  *
  */

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

@@ -1,15 +1,15 @@
 import {Component} from '@angular/core';
 import {ComponentFixture, TestBed, async} from '@angular/core/testing';
+import {By} from '@angular/platform-browser';
 import {CdkMenuModule} from './menu-module';
 import {CdkMenuGroup} from './menu-group';
-import {CdkMenuItem} from './menu-item';
-import {CdkMenu} from './menu';
-import {By} from '@angular/platform-browser';
+import {CdkMenuItemCheckbox} from './menu-item-checkbox';
+import {CdkMenuItemRadio} from './menu-item-radio';
 
 describe('MenuGroup', () => {
   describe('with MenuItems as checkbox', () => {
     let fixture: ComponentFixture<CheckboxMenu>;
-    let menuItems: CdkMenuItem[];
+    let menuItems: CdkMenuItemCheckbox[];
 
     beforeEach(async(() => {
       TestBed.configureTestingModule({
@@ -21,8 +21,8 @@ describe('MenuGroup', () => {
       fixture.detectChanges();
 
       menuItems = fixture.debugElement
-        .queryAll(By.directive(CdkMenuItem))
-        .map(e => e.injector.get(CdkMenuItem));
+        .queryAll(By.directive(CdkMenuItemCheckbox))
+        .map(e => e.injector.get(CdkMenuItemCheckbox));
     }));
 
     it('should not change state of sibling checked menuitemcheckbox', () => {
@@ -34,7 +34,7 @@ describe('MenuGroup', () => {
 
   describe('with MenuItems as radio button', () => {
     let fixture: ComponentFixture<MenuWithMultipleRadioGroups>;
-    let menuItems: CdkMenuItem[];
+    let menuItems: CdkMenuItemRadio[];
 
     beforeEach(async(() => {
       TestBed.configureTestingModule({
@@ -46,8 +46,8 @@ describe('MenuGroup', () => {
       fixture.detectChanges();
 
       menuItems = fixture.debugElement
-        .queryAll(By.directive(CdkMenuItem))
-        .map(e => e.injector.get(CdkMenuItem));
+        .queryAll(By.directive(CdkMenuItemRadio))
+        .map(e => e.injector.get(CdkMenuItemRadio));
     }));
 
     it('should change state of sibling menuitemradio in same group', () => {
@@ -76,8 +76,7 @@ describe('MenuGroup', () => {
 
   describe('change events', () => {
     let fixture: ComponentFixture<MenuWithMenuItemsAndRadioGroups>;
-    let menu: CdkMenu;
-    let menuItems: CdkMenuItem[];
+    let menuItems: CdkMenuItemRadio[];
 
     beforeEach(async(() => {
       TestBed.configureTestingModule({
@@ -88,22 +87,11 @@ describe('MenuGroup', () => {
       fixture = TestBed.createComponent(MenuWithMenuItemsAndRadioGroups);
       fixture.detectChanges();
 
-      menu = fixture.debugElement.query(By.directive(CdkMenu)).injector.get(CdkMenu);
-
       menuItems = fixture.debugElement
-        .queryAll(By.directive(CdkMenuItem))
-        .map(element => element.injector.get(CdkMenuItem));
+        .queryAll(By.directive(CdkMenuItemRadio))
+        .map(element => element.injector.get(CdkMenuItemRadio));
     }));
 
-    it('should not emit from root menu with nested groups', () => {
-      const spy = jasmine.createSpy('changeSpy for root menu');
-      menu.change.subscribe(spy);
-
-      menuItems.forEach(menuItem => menuItem.trigger());
-
-      expect(spy).toHaveBeenCalledTimes(0);
-    });
-
     it('should emit from enclosing radio group only', () => {
       const spies: jasmine.Spy[] = [];
 
@@ -117,8 +105,8 @@ describe('MenuGroup', () => {
 
       expect(spies[1]).toHaveBeenCalledTimes(1);
       expect(spies[1]).toHaveBeenCalledWith(menuItems[0]);
-      expect(spies[2]).toHaveBeenCalledTimes(0);
-      expect(spies[3]).toHaveBeenCalledTimes(0);
+      expect(spies[2]).not.toHaveBeenCalled();
+      expect(spies[3]).not.toHaveBeenCalled();
     });
 
     it('should not emit with click on disabled button', () => {
@@ -132,21 +120,7 @@ describe('MenuGroup', () => {
 
       menuItems[0].trigger();
 
-      expect(spy).toHaveBeenCalledTimes(0);
-    });
-
-    it('should not emit on menuitem click', () => {
-      const spies: jasmine.Spy[] = [];
-
-      fixture.debugElement.queryAll(By.directive(CdkMenuGroup)).forEach((group, index) => {
-        const spy = jasmine.createSpy(`cdkMenuGroup ${index} change spy`);
-        spies.push(spy);
-        group.injector.get(CdkMenuGroup).change.subscribe(spy);
-      });
-
-      menuItems[2].trigger();
-
-      spies.forEach(spy => expect(spy).toHaveBeenCalledTimes(0));
+      expect(spy).not.toHaveBeenCalled();
     });
   });
 });
@@ -157,12 +131,12 @@ describe('MenuGroup', () => {
       <li role="none">
         <ul cdkMenuGroup>
           <li #first role="none">
-            <button checked="true" role="menuitemcheckbox" cdkMenuItem>
+            <button checked="true" cdkMenuItemCheckbox>
               one
             </button>
           </li>
           <li role="none">
-            <button role="menuitemcheckbox" cdkMenuItem>
+            <button cdkMenuItemCheckbox>
               two
             </button>
           </li>
@@ -179,12 +153,12 @@ class CheckboxMenu {}
       <li role="none">
         <ul cdkMenuGroup>
           <li role="none">
-            <button checked="true" role="menuitemradio" cdkMenuItem>
+            <button checked="true" cdkMenuItemRadio>
               one
             </button>
           </li>
           <li role="none">
-            <button role="menuitemradio" cdkMenuItem>
+            <button cdkMenuItemRadio>
               two
             </button>
           </li>
@@ -193,12 +167,12 @@ class CheckboxMenu {}
       <li role="none">
         <ul cdkMenuGroup>
           <li role="none">
-            <button role="menuitemradio" cdkMenuItem>
+            <button cdkMenuItemRadio>
               three
             </button>
           </li>
           <li role="none">
-            <button role="menuitemradio" cdkMenuItem>
+            <button cdkMenuItemRadio>
               four
             </button>
           </li>
@@ -215,7 +189,7 @@ class MenuWithMultipleRadioGroups {}
       <li role="none">
         <ul cdkMenuGroup>
           <li role="none">
-            <button role="menuitemradio" cdkMenuItem>
+            <button cdkMenuItemRadio>
               one
             </button>
           </li>
@@ -224,7 +198,7 @@ class MenuWithMultipleRadioGroups {}
       <li role="none">
         <ul cdkMenuGroup>
           <li role="none">
-            <button role="menuitemradio" cdkMenuItem>
+            <button cdkMenuItemRadio>
               two
             </button>
           </li>

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

@@ -6,8 +6,19 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Directive, Output, EventEmitter} from '@angular/core';
-import {MenuItem} from './menu-item-interface';
+import {
+  Directive,
+  Output,
+  EventEmitter,
+  ContentChildren,
+  AfterContentInit,
+  QueryList,
+  OnDestroy,
+} from '@angular/core';
+import {UniqueSelectionDispatcher} from '@angular/cdk/collections';
+import {takeUntil} from 'rxjs/operators';
+import {CdkMenuItemSelectable} from './menu-item-selectable';
+import {CdkMenuItem} from './menu-item';
 
 /**
  * Directive which acts as a grouping container for `CdkMenuItem` instances with
@@ -19,18 +30,45 @@ import {MenuItem} from './menu-item-interface';
   host: {
     'role': 'group',
   },
+  providers: [{provide: UniqueSelectionDispatcher, useClass: UniqueSelectionDispatcher}],
 })
-export class CdkMenuGroup {
+export class CdkMenuGroup implements AfterContentInit, OnDestroy {
   /** Emits the element when checkbox or radiobutton state changed  */
-  @Output() change: EventEmitter<MenuItem> = new EventEmitter();
+  @Output() readonly change: EventEmitter<CdkMenuItem> = new EventEmitter();
+
+  /** List of menuitemcheckbox or menuitemradio elements which reside in this group */
+  @ContentChildren(CdkMenuItemSelectable, {descendants: true})
+  private readonly _selectableItems: QueryList<CdkMenuItemSelectable>;
+
+  /** Emits when the _selectableItems QueryList triggers a change */
+  private readonly _selectableChanges: EventEmitter<void> = new EventEmitter();
+
+  ngAfterContentInit() {
+    this._registerMenuSelectionListeners();
+  }
 
   /**
-   * Emits events for the clicked MenuItem
-   * @param menuItem The clicked MenuItem to handle
+   * Register the child selectable elements with the change emitter and ensure any new child
+   * elements do so as well.
    */
-  _registerTriggeredItem(menuItem: MenuItem) {
-    if (menuItem.role !== 'menuitem') {
-      this.change.emit(menuItem);
-    }
+  private _registerMenuSelectionListeners() {
+    this._selectableItems.forEach(selectable => this._registerClickListener(selectable));
+
+    this._selectableItems.changes.subscribe((selectableItems: QueryList<CdkMenuItemSelectable>) => {
+      this._selectableChanges.next();
+      selectableItems.forEach(selectable => this._registerClickListener(selectable));
+    });
+  }
+
+  /** Register each selectable to emit on the change Emitter when clicked */
+  private _registerClickListener(selectable: CdkMenuItemSelectable) {
+    selectable.clicked
+      .pipe(takeUntil(this._selectableChanges))
+      .subscribe(() => this.change.next(selectable));
+  }
+
+  ngOnDestroy() {
+    this._selectableChanges.next();
+    this._selectableChanges.complete();
   }
 }