From 26e6c1fd2e0af36d11f83c4d95ce3abd991458e3 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Mon, 4 Apr 2022 18:10:57 +0000 Subject: [PATCH] fix(cdk-experimental/menu): simplify radio and checkbox item APIs (#24720) * fix(cdk-experimental/menu): simplify radio and checkbox item APIs * fix(cdk-experimental/menu): Add aria-controls attr to triggers * fixup! fix(cdk-experimental/menu): Add aria-controls attr to triggers * fixup! fix(cdk-experimental/menu): Add aria-controls attr to triggers --- src/cdk-experimental/menu/menu-bar.spec.ts | 35 +--- src/cdk-experimental/menu/menu-base.ts | 12 +- src/cdk-experimental/menu/menu-group.spec.ts | 106 +----------- src/cdk-experimental/menu/menu-group.ts | 54 +----- src/cdk-experimental/menu/menu-interface.ts | 2 + .../menu/menu-item-checkbox.spec.ts | 10 +- .../menu/menu-item-checkbox.ts | 8 +- .../menu/menu-item-radio.spec.ts | 25 +-- src/cdk-experimental/menu/menu-item-radio.ts | 22 +-- .../menu/menu-item-selectable.ts | 33 ++-- .../menu/menu-item-trigger.spec.ts | 4 - src/cdk-experimental/menu/menu-item.spec.ts | 4 - src/cdk-experimental/menu/menu-item.ts | 15 +- src/cdk-experimental/menu/menu-trigger.ts | 6 +- src/cdk-experimental/menu/menu.spec.ts | 156 +----------------- src/cdk-experimental/menu/menu.ts | 33 +--- ...menu-standalone-stateful-menu-example.html | 31 ++-- ...k-menu-standalone-stateful-menu-example.ts | 8 +- .../cdk-experimental/menu/index.ts | 3 +- .../cdk-experimental-menu/cdk-menu-demo.html | 54 +++--- .../cdk-experimental-menu/cdk-menu-demo.ts | 15 +- 21 files changed, 125 insertions(+), 511 deletions(-) diff --git a/src/cdk-experimental/menu/menu-bar.spec.ts b/src/cdk-experimental/menu/menu-bar.spec.ts index e3fd276d8183..56b72dc4e987 100644 --- a/src/cdk-experimental/menu/menu-bar.spec.ts +++ b/src/cdk-experimental/menu/menu-bar.spec.ts @@ -69,39 +69,6 @@ describe('MenuBar', () => { }); }); - describe('radiogroup change events', () => { - let fixture: ComponentFixture; - let menuItems: CdkMenuItemRadio[]; - - beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [CdkMenuModule], - declarations: [MenuBarRadioGroup], - }).compileComponents(); - - fixture = TestBed.createComponent(MenuBarRadioGroup); - - fixture.detectChanges(); - - menuItems = fixture.debugElement - .queryAll(By.directive(CdkMenuItemRadio)) - .map(element => element.injector.get(CdkMenuItemRadio)); - })); - - it('should emit on click', () => { - const spy = jasmine.createSpy('cdkMenu change spy'); - fixture.debugElement - .query(By.directive(CdkMenuBar)) - .injector.get(CdkMenuBar) - .change.subscribe(spy); - - menuItems[0].trigger(); - - expect(spy).toHaveBeenCalledTimes(1); - expect(spy).toHaveBeenCalledWith(menuItems[0]); - }); - }); - describe('Keyboard handling', () => { describe('(with ltr layout)', () => { let fixture: ComponentFixture; @@ -1145,7 +1112,7 @@ describe('MenuBar', () => { template: `
  • -
    • diff --git a/src/cdk-experimental/menu/menu-base.ts b/src/cdk-experimental/menu/menu-base.ts index 64673249a737..0c9350367a43 100644 --- a/src/cdk-experimental/menu/menu-base.ts +++ b/src/cdk-experimental/menu/menu-base.ts @@ -13,6 +13,7 @@ import { Directive, ElementRef, Inject, + Input, OnDestroy, Optional, QueryList, @@ -26,9 +27,12 @@ import {MENU_STACK, MenuStack, MenuStackItem} from './menu-stack'; import {Menu} from './menu-interface'; import {PointerFocusTracker} from './pointer-focus-tracker'; +let nextId = 0; + @Directive({ host: { '[tabindex]': '_isInline ? (_hasFocus ? -1 : 0) : null', + '[id]': 'id', '[attr.aria-orientation]': 'orientation', '(focus)': 'focusFirstItem()', '(focusin)': 'menuStack.setHasFocus(true)', @@ -39,6 +43,8 @@ export abstract class CdkMenuBase extends CdkMenuGroup implements Menu, AfterContentInit, OnDestroy { + @Input() id = `cdk-menu-${nextId++}`; + /** * Sets the aria-orientation attribute and determines where menus will be opened. * Does not affect styling/layout. @@ -73,16 +79,14 @@ export abstract class CdkMenuBase super(); } - override ngAfterContentInit() { - super.ngAfterContentInit(); + ngAfterContentInit() { this._setKeyManager(); this._subscribeToHasFocus(); this._subscribeToMenuOpen(); this._subscribeToMenuStackClosed(); } - override ngOnDestroy() { - super.ngOnDestroy(); + ngOnDestroy() { this.destroyed.next(); this.destroyed.complete(); } diff --git a/src/cdk-experimental/menu/menu-group.spec.ts b/src/cdk-experimental/menu/menu-group.spec.ts index a0bf828cd9e9..71a57476ed1f 100644 --- a/src/cdk-experimental/menu/menu-group.spec.ts +++ b/src/cdk-experimental/menu/menu-group.spec.ts @@ -2,7 +2,6 @@ import {Component, ViewChild} from '@angular/core'; import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {CdkMenuModule} from './menu-module'; -import {CdkMenuGroup} from './menu-group'; import {CdkMenuItemCheckbox} from './menu-item-checkbox'; import {CdkMenuItemRadio} from './menu-item-radio'; import {CdkMenuItem} from './menu-item'; @@ -58,14 +57,14 @@ describe('MenuGroup', () => { })); it('should change state of sibling menuitemradio in same group', () => { - menuItems[1].trigger(); + menuItems[1].trigger({keepOpen: true}); expect(menuItems[1].checked).toBeTrue(); expect(menuItems[0].checked).toBeFalse(); }); it('should not change state of menuitemradio in sibling group', () => { - menuItems[3].trigger(); + menuItems[3].trigger({keepOpen: true}); expect(menuItems[3].checked).toBeTrue(); expect(menuItems[0].checked).toBeTrue(); @@ -74,65 +73,12 @@ describe('MenuGroup', () => { it('should not change radiogroup state with disabled button', () => { menuItems[1].disabled = true; - menuItems[1].trigger(); + menuItems[1].trigger({keepOpen: true}); expect(menuItems[0].checked).toBeTrue(); expect(menuItems[1].checked).toBeFalse(); }); }); - - describe('change events', () => { - let fixture: ComponentFixture; - let menuItems: CdkMenuItemRadio[]; - - beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [CdkMenuModule], - declarations: [MenuWithMenuItemsAndRadioGroups], - }).compileComponents(); - - fixture = TestBed.createComponent(MenuWithMenuItemsAndRadioGroups); - fixture.detectChanges(); - - fixture.componentInstance.trigger.getMenuTrigger()?.toggle(); - fixture.detectChanges(); - - menuItems = fixture.debugElement - .queryAll(By.directive(CdkMenuItemRadio)) - .map(element => element.injector.get(CdkMenuItemRadio)); - })); - - it('should emit from enclosing radio group only', () => { - 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[0].trigger(); - - expect(spies[2]).toHaveBeenCalledTimes(1); - expect(spies[2]).toHaveBeenCalledWith(menuItems[0]); - expect(spies[3]).not.toHaveBeenCalled(); - expect(spies[4]).not.toHaveBeenCalled(); - }); - - it('should not emit with click on disabled button', () => { - const spy = jasmine.createSpy('cdkMenuGroup change spy'); - - fixture.debugElement - .queryAll(By.directive(CdkMenuGroup))[1] - .injector.get(CdkMenuGroup) - .change.subscribe(spy); - menuItems[0].disabled = true; - - menuItems[0].trigger(); - - expect(spy).not.toHaveBeenCalled(); - }); - }); }); @Component({ @@ -145,7 +91,7 @@ describe('MenuGroup', () => {
    • -
      • @@ -174,7 +120,7 @@ class CheckboxMenu {
      • -
        • @@ -206,45 +152,3 @@ class CheckboxMenu { class MenuWithMultipleRadioGroups { @ViewChild(CdkMenuItem) readonly trigger: CdkMenuItem; } - -@Component({ - template: ` -
        - -
        - -
          -
        • -
            -
          • - -
            • -
          -
          • -
        • -
            -
          • - -
            • -
          -
          • -
        • -
            -
          • - -
            • -
          -
          • -
        -         - `, -}) -class MenuWithMenuItemsAndRadioGroups { - @ViewChild(CdkMenuItem) readonly trigger: CdkMenuItem; -} diff --git a/src/cdk-experimental/menu/menu-group.ts b/src/cdk-experimental/menu/menu-group.ts index 7f6e2670b1b7..dc5be809e003 100644 --- a/src/cdk-experimental/menu/menu-group.ts +++ b/src/cdk-experimental/menu/menu-group.ts @@ -6,19 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import { - AfterContentInit, - ContentChildren, - Directive, - EventEmitter, - OnDestroy, - Output, - QueryList, -} from '@angular/core'; +import {Directive} 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 @@ -33,43 +22,4 @@ import {CdkMenuItem} from './menu-item'; }, providers: [{provide: UniqueSelectionDispatcher, useClass: UniqueSelectionDispatcher}], }) -export class CdkMenuGroup implements AfterContentInit, OnDestroy { - /** Emits the element when checkbox or radiobutton state changed */ - @Output() readonly change: EventEmitter = new EventEmitter(); - - /** List of menuitemcheckbox or menuitemradio elements which reside in this group */ - @ContentChildren(CdkMenuItemSelectable, {descendants: true}) - private readonly _selectableItems: QueryList; - - /** Emits when the _selectableItems QueryList triggers a change */ - private readonly _selectableChanges: EventEmitter = new EventEmitter(); - - ngAfterContentInit() { - this._registerMenuSelectionListeners(); - } - - ngOnDestroy() { - this._selectableChanges.next(); - this._selectableChanges.complete(); - } - - /** - * Register the child selectable elements with the change emitter and ensure any new child - * elements do so as well. - */ - private _registerMenuSelectionListeners() { - this._selectableItems.forEach(selectable => this._registerClickListener(selectable)); - - this._selectableItems.changes.subscribe((selectableItems: QueryList) => { - 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.toggled - .pipe(takeUntil(this._selectableChanges)) - .subscribe(() => this.change.next(selectable)); - } -} +export class CdkMenuGroup {} diff --git a/src/cdk-experimental/menu/menu-interface.ts b/src/cdk-experimental/menu/menu-interface.ts index ca8846d4c459..e1bf115635fb 100644 --- a/src/cdk-experimental/menu/menu-interface.ts +++ b/src/cdk-experimental/menu/menu-interface.ts @@ -15,6 +15,8 @@ export const CDK_MENU = new InjectionToken('cdk-menu'); /** Interface which specifies Menu operations and used to break circular dependency issues */ export interface Menu extends MenuStackItem { + id: string; + /** The element the Menu directive is placed on. */ _elementRef: ElementRef; diff --git a/src/cdk-experimental/menu/menu-item-checkbox.spec.ts b/src/cdk-experimental/menu/menu-item-checkbox.spec.ts index 96bba9873d4d..eb3714bec710 100644 --- a/src/cdk-experimental/menu/menu-item-checkbox.spec.ts +++ b/src/cdk-experimental/menu/menu-item-checkbox.spec.ts @@ -48,16 +48,12 @@ describe('MenuItemCheckbox', () => { expect(checkboxElement.getAttribute('aria-disabled')).toBe('true'); }); - it('should be a button type', () => { - expect(checkboxElement.getAttribute('type')).toBe('button'); - }); - it('should not have a menu', () => { expect(checkbox.hasMenu()).toBeFalse(); }); it('should toggle the aria checked attribute', () => { - expect(checkboxElement.getAttribute('aria-checked')).toBeNull(); + expect(checkboxElement.getAttribute('aria-checked')).toBe('false'); checkboxElement.click(); fixture.detectChanges(); @@ -84,7 +80,7 @@ describe('MenuItemCheckbox', () => { it('should emit on clicked emitter when triggered', () => { const spy = jasmine.createSpy('cdkMenuItemCheckbox clicked spy'); - checkbox.toggled.subscribe(spy); + checkbox.triggered.subscribe(spy); checkbox.trigger(); @@ -93,7 +89,7 @@ describe('MenuItemCheckbox', () => { it('should not emit on clicked emitter when disabled', () => { const spy = jasmine.createSpy('cdkMenuItemCheckbox clicked spy'); - checkbox.toggled.subscribe(spy); + checkbox.triggered.subscribe(spy); checkbox.disabled = true; checkbox.trigger(); diff --git a/src/cdk-experimental/menu/menu-item-checkbox.ts b/src/cdk-experimental/menu/menu-item-checkbox.ts index e0cfd605ece0..9b177edf6939 100644 --- a/src/cdk-experimental/menu/menu-item-checkbox.ts +++ b/src/cdk-experimental/menu/menu-item-checkbox.ts @@ -18,11 +18,7 @@ import {CdkMenuItem} from './menu-item'; selector: '[cdkMenuItemCheckbox]', exportAs: 'cdkMenuItemCheckbox', host: { - '[tabindex]': '_tabindex', - 'type': 'button', 'role': 'menuitemcheckbox', - '[attr.aria-checked]': 'checked || null', - '[attr.aria-disabled]': 'disabled || null', }, providers: [ {provide: CdkMenuItemSelectable, useExisting: CdkMenuItemCheckbox}, @@ -31,8 +27,8 @@ import {CdkMenuItem} from './menu-item'; }) export class CdkMenuItemCheckbox extends CdkMenuItemSelectable { /** Toggle the checked state of the checkbox. */ - override trigger() { - super.trigger(); + override trigger(options?: {keepOpen: boolean}) { + super.trigger(options); if (!this.disabled) { this.checked = !this.checked; diff --git a/src/cdk-experimental/menu/menu-item-radio.spec.ts b/src/cdk-experimental/menu/menu-item-radio.spec.ts index 5315a6293e48..3583b257fe14 100644 --- a/src/cdk-experimental/menu/menu-item-radio.spec.ts +++ b/src/cdk-experimental/menu/menu-item-radio.spec.ts @@ -53,7 +53,7 @@ describe('MenuItemRadio', () => { }); it('should toggle the aria checked attribute', () => { - expect(radioElement.getAttribute('aria-checked')).toBeNull(); + expect(radioElement.getAttribute('aria-checked')).toBe('false'); radioElement.click(); fixture.detectChanges(); @@ -61,10 +61,6 @@ describe('MenuItemRadio', () => { expect(radioElement.getAttribute('aria-checked')).toBe('true'); }); - it('should be a button type', () => { - expect(radioElement.getAttribute('type')).toBe('button'); - }); - it('should not have a menu', () => { expect(radioButton.hasMenu()).toBeFalse(); }); @@ -78,7 +74,7 @@ describe('MenuItemRadio', () => { it('should emit on clicked emitter when triggered', () => { const spy = jasmine.createSpy('cdkMenuItemRadio clicked spy'); - radioButton.toggled.subscribe(spy); + radioButton.triggered.subscribe(spy); radioButton.trigger(); @@ -87,28 +83,13 @@ describe('MenuItemRadio', () => { it('should not emit on clicked emitter when disabled', () => { const spy = jasmine.createSpy('cdkMenuItemRadio clicked spy'); - radioButton.toggled.subscribe(spy); + radioButton.triggered.subscribe(spy); radioButton.disabled = true; radioButton.trigger(); expect(spy).not.toHaveBeenCalled(); }); - - it('should toggle state when called with own id and not toggle when called with other id', () => { - const id = 'id-1'; - const name = 'name-1'; - radioButton.id = id; - radioButton.name = name; - - expect(radioButton.checked).withContext('Initial state').toBeFalse(); - - selectionDispatcher.notify(id, name); - expect(radioButton.checked).withContext('Called with own id and name').toBeTrue(); - - selectionDispatcher.notify('another-id', 'another-name'); - expect(radioButton.checked).withContext('Called with differing id and name').toBeFalse(); - }); }); @Component({ diff --git a/src/cdk-experimental/menu/menu-item-radio.ts b/src/cdk-experimental/menu/menu-item-radio.ts index aa3206fe98db..5a6531f56870 100644 --- a/src/cdk-experimental/menu/menu-item-radio.ts +++ b/src/cdk-experimental/menu/menu-item-radio.ts @@ -16,6 +16,9 @@ import {CDK_MENU, Menu} from './menu-interface'; import {MENU_AIM, MenuAim} from './menu-aim'; import {MENU_STACK, MenuStack} from './menu-stack'; +/** Counter used to set a unique id and name for a selectable item */ +let nextId = 0; + /** * A directive providing behavior for the "menuitemradio" ARIA role, which behaves similarly to * a conventional radio-button. Any sibling `CdkMenuItemRadio` instances within the same `CdkMenu` @@ -25,11 +28,7 @@ import {MENU_STACK, MenuStack} from './menu-stack'; selector: '[cdkMenuItemRadio]', exportAs: 'cdkMenuItemRadio', host: { - '[tabindex]': '_tabindex', - 'type': 'button', 'role': 'menuitemradio', - '[attr.aria-checked]': 'checked || null', - '[attr.aria-disabled]': 'disabled || null', }, providers: [ {provide: CdkMenuItemSelectable, useExisting: CdkMenuItemRadio}, @@ -37,6 +36,8 @@ import {MENU_STACK, MenuStack} from './menu-stack'; ], }) export class CdkMenuItemRadio extends CdkMenuItemSelectable implements OnDestroy { + private _id = `${nextId++}`; + /** Function to unregister the selection dispatcher */ private _removeDispatcherListener: () => void; @@ -60,22 +61,23 @@ export class CdkMenuItemRadio extends CdkMenuItemSelectable implements OnDestroy /** Configure the unique selection dispatcher listener in order to toggle the checked state */ private _registerDispatcherListener() { - this._removeDispatcherListener = this._selectionDispatcher.listen( - (id: string, name: string) => (this.checked = this.id === id && this.name === name), - ); + this._removeDispatcherListener = this._selectionDispatcher.listen((id: string) => { + this.checked = this._id === id; + }); } /** Toggles the checked state of the radio-button. */ - override trigger() { - super.trigger(); + override trigger(options?: {keepOpen: boolean}) { + super.trigger(options); if (!this.disabled) { - this._selectionDispatcher.notify(this.id, this.name); + this._selectionDispatcher.notify(this._id, ''); } } override ngOnDestroy() { super.ngOnDestroy(); + this._removeDispatcherListener(); } } diff --git a/src/cdk-experimental/menu/menu-item-selectable.ts b/src/cdk-experimental/menu/menu-item-selectable.ts index d6073454c75f..002434819950 100644 --- a/src/cdk-experimental/menu/menu-item-selectable.ts +++ b/src/cdk-experimental/menu/menu-item-selectable.ts @@ -6,25 +6,23 @@ * found in the LICENSE file at https://angular.io/license */ -import {coerceBooleanProperty, BooleanInput} from '@angular/cdk/coercion'; -import {Input, Directive, Output, EventEmitter} from '@angular/core'; +import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion'; +import {Directive, Input} from '@angular/core'; import {CdkMenuItem} from './menu-item'; -/** Counter used to set a unique id and name for a selectable item */ -let nextId = 0; - /** * Base class providing checked state for MenuItems along with outputting a clicked event when the * element is triggered. It provides functionality for selectable elements. */ -@Directive() +@Directive({ + host: { + '[attr.aria-checked]': '!!checked', + '[attr.aria-disabled]': 'disabled || null', + }, +}) export abstract class CdkMenuItemSelectable extends CdkMenuItem { - /** Event emitted when the selectable item is clicked */ - @Output('cdkMenuItemToggled') readonly toggled: EventEmitter = - new EventEmitter(); - /** Whether the element is checked */ - @Input() + @Input('cdkMenuItemChecked') get checked(): boolean { return this._checked; } @@ -33,16 +31,5 @@ export abstract class CdkMenuItemSelectable extends CdkMenuItem { } private _checked = false; - /** The name of the selectable element with a default value */ - @Input() name: string = `cdk-selectable-item-${nextId++}`; - - /** The id of the selectable element with a default value */ - @Input() id: string = `cdk-selectable-item-${nextId++}`; - - /** If the element is not disabled emit the click event */ - override trigger() { - if (!this.disabled) { - this.toggled.next(this); - } - } + protected override closeOnSpacebarTrigger = false; } diff --git a/src/cdk-experimental/menu/menu-item-trigger.spec.ts b/src/cdk-experimental/menu/menu-item-trigger.spec.ts index 58074e45d2c2..d6bfb8ad4151 100644 --- a/src/cdk-experimental/menu/menu-item-trigger.spec.ts +++ b/src/cdk-experimental/menu/menu-item-trigger.spec.ts @@ -47,10 +47,6 @@ describe('MenuItemTrigger', () => { expect(menuItemElement.getAttribute('aria-haspopup')).toEqual('menu'); }); - it('should be a button type', () => { - expect(menuItemElement.getAttribute('type')).toBe('button'); - }); - it('should have a menu', () => { expect(menuItem.hasMenu()).toBeTrue(); }); diff --git a/src/cdk-experimental/menu/menu-item.spec.ts b/src/cdk-experimental/menu/menu-item.spec.ts index bb325121b0c3..eae87c553170 100644 --- a/src/cdk-experimental/menu/menu-item.spec.ts +++ b/src/cdk-experimental/menu/menu-item.spec.ts @@ -57,10 +57,6 @@ describe('MenuItem', () => { expect(nativeButton.hasAttribute('aria-disabled')).toBeFalse(); }); - it('should be a button type', () => { - expect(nativeButton.getAttribute('type')).toBe('button'); - }); - it('should not have a menu', () => { expect(menuItem.hasMenu()).toBeFalse(); }); diff --git a/src/cdk-experimental/menu/menu-item.ts b/src/cdk-experimental/menu/menu-item.ts index 67d314f3f1ea..e70efb47e2d3 100644 --- a/src/cdk-experimental/menu/menu-item.ts +++ b/src/cdk-experimental/menu/menu-item.ts @@ -39,10 +39,9 @@ import {MENU_AIM, MenuAim, Toggler} from './menu-aim'; selector: '[cdkMenuItem]', exportAs: 'cdkMenuItem', host: { - '[tabindex]': '_tabindex', - 'type': 'button', 'role': 'menuitem', 'class': 'cdk-menu-item', + '[tabindex]': '_tabindex', '[attr.aria-disabled]': 'disabled || null', '(blur)': '_resetTabIndex()', '(focus)': '_setTabIndex()', @@ -79,6 +78,9 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler, */ _tabindex: 0 | -1 = -1; + /** Whether the item should close the menu if triggered by the spacebar. */ + protected closeOnSpacebarTrigger = true; + /** Emits when the menu item is destroyed. */ private readonly _destroyed = new Subject(); @@ -137,10 +139,13 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler, * If the menu item is not disabled and the element does not have a menu trigger attached, emit * on the cdkMenuItemTriggered emitter and close all open menus. */ - trigger() { + trigger(options?: {keepOpen: boolean}) { + const {keepOpen} = {...options}; if (!this.disabled && !this.hasMenu()) { this.triggered.next(); - this._menuStack.closeAll({focusParentTrigger: true}); + if (!keepOpen) { + this._menuStack.closeAll({focusParentTrigger: true}); + } } } @@ -183,7 +188,7 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler, case SPACE: case ENTER: event.preventDefault(); - this.trigger(); + this.trigger({keepOpen: event.keyCode === SPACE && !this.closeOnSpacebarTrigger}); break; case RIGHT_ARROW: diff --git a/src/cdk-experimental/menu/menu-trigger.ts b/src/cdk-experimental/menu/menu-trigger.ts index 896f1bc8cc6a..a98b25846e66 100644 --- a/src/cdk-experimental/menu/menu-trigger.ts +++ b/src/cdk-experimental/menu/menu-trigger.ts @@ -24,7 +24,11 @@ import {merge, Subject} from 'rxjs'; /** Injection token used for an implementation of MenuStack. */ export const MENU_TRIGGER = new InjectionToken('cdk-menu-trigger'); -@Directive() +@Directive({ + host: { + '[attr.aria-controls]': 'childMenu?.id', + }, +}) export abstract class MenuTrigger implements OnDestroy { /** A list of preferred menu positions to be used when constructing the `FlexibleConnectedPositionStrategy` for this trigger's menu. */ menuPosition: ConnectedPosition[]; diff --git a/src/cdk-experimental/menu/menu.spec.ts b/src/cdk-experimental/menu/menu.spec.ts index 79d8cb0535be..8ad7a3c67414 100644 --- a/src/cdk-experimental/menu/menu.spec.ts +++ b/src/cdk-experimental/menu/menu.spec.ts @@ -56,117 +56,6 @@ describe('Menu', () => { }); }); - describe('checkbox change events', () => { - let fixture: ComponentFixture; - let menuItems: CdkMenuItemCheckbox[]; - - beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [CdkMenuModule], - declarations: [MenuCheckboxGroup], - }).compileComponents(); - - fixture = TestBed.createComponent(MenuCheckboxGroup); - fixture.detectChanges(); - - fixture.componentInstance.trigger.getMenuTrigger()?.toggle(); - fixture.detectChanges(); - - menuItems = fixture.debugElement - .queryAll(By.directive(CdkMenuItemCheckbox)) - .map(element => element.injector.get(CdkMenuItemCheckbox)); - })); - - it('should emit on click', () => { - const spy = jasmine.createSpy('cdkMenu change spy'); - fixture.debugElement.query(By.directive(CdkMenu)).injector.get(CdkMenu).change.subscribe(spy); - - menuItems[0].trigger(); - - expect(spy).toHaveBeenCalledTimes(1); - expect(spy).toHaveBeenCalledWith(menuItems[0]); - }); - }); - - describe('with inner group', () => { - let fixture: ComponentFixture; - let menuItems: CdkMenuItemCheckbox[]; - let menu: CdkMenu; - - beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [CdkMenuModule], - declarations: [MenuWithNestedGroup], - }).compileComponents(); - - fixture = TestBed.createComponent(MenuWithNestedGroup); - fixture.detectChanges(); - - fixture.componentInstance.trigger.getMenuTrigger()?.toggle(); - fixture.detectChanges(); - - menu = fixture.debugElement.query(By.directive(CdkMenu)).injector.get(CdkMenu); - - menuItems = fixture.debugElement - .queryAll(By.directive(CdkMenuItemCheckbox)) - .map(element => element.injector.get(CdkMenuItemCheckbox)); - })); - - it('should not emit change from root menu ', () => { - const spy = jasmine.createSpy('changeSpy for root menu'); - menu.change.subscribe(spy); - - for (let menuItem of menuItems) { - menuItem.trigger(); - } - - expect(spy).not.toHaveBeenCalled(); - }); - }); - - describe('with inner group render delayed', () => { - let fixture: ComponentFixture; - let menuItems: CdkMenuItemCheckbox[]; - let menu: CdkMenu; - - const getMenuItems = () => { - return fixture.debugElement - .queryAll(By.directive(CdkMenuItemCheckbox)) - .map(element => element.injector.get(CdkMenuItemCheckbox)); - }; - - beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [CdkMenuModule], - declarations: [MenuWithConditionalGroup], - }).compileComponents(); - - fixture = TestBed.createComponent(MenuWithConditionalGroup); - fixture.detectChanges(); - - fixture.componentInstance.trigger.getMenuTrigger()?.toggle(); - fixture.detectChanges(); - - menu = fixture.debugElement.query(By.directive(CdkMenu)).injector.get(CdkMenu); - menuItems = getMenuItems(); - })); - - it('should not emit after the menu group element renders', () => { - const spy = jasmine.createSpy('cdkMenu change'); - menu.change.subscribe(spy); - - menuItems[0].trigger(); - fixture.componentInstance.renderInnerGroup = true; - fixture.detectChanges(); - - menuItems = getMenuItems(); - menuItems[1].trigger(); - fixture.detectChanges(); - - expect(spy).withContext('Expected initial trigger only').toHaveBeenCalledTimes(1); - }); - }); - describe('when configured inline', () => { let fixture: ComponentFixture; let nativeMenu: HTMLElement; @@ -632,7 +521,7 @@ describe('Menu', () => {
        • - +
        • @@ -645,48 +534,6 @@ class MenuCheckboxGroup { @ViewChild(CdkMenuItem) readonly trigger: CdkMenuItem; } -@Component({ - template: ` -
          - -
          - -
            -
          • -
              -
              • -
            -
            • -
          -           - `, -}) -class MenuWithNestedGroup { - @ViewChild(CdkMenuItem) readonly trigger: CdkMenuItem; -} - -@Component({ - template: ` -
          - -
          - -
            -
            • -
            -
              -
              • -
            -
            -
          -           - `, -}) -class MenuWithConditionalGroup { - renderInnerGroup = false; - @ViewChild(CdkMenuItem) readonly trigger: CdkMenuItem; -} - @Component({ template: `
          @@ -754,6 +601,7 @@ class WithComplexNestedMenus { @ViewChildren(CdkMenu) menus: QueryList; } + @Component({ template: `
          diff --git a/src/cdk-experimental/menu/menu.ts b/src/cdk-experimental/menu/menu.ts index 789442db569b..19c6d59d619d 100644 --- a/src/cdk-experimental/menu/menu.ts +++ b/src/cdk-experimental/menu/menu.ts @@ -8,7 +8,6 @@ import { AfterContentInit, - ContentChildren, Directive, ElementRef, EventEmitter, @@ -17,7 +16,6 @@ import { OnDestroy, Optional, Output, - QueryList, Self, } from '@angular/core'; import { @@ -30,7 +28,7 @@ import { UP_ARROW, } from '@angular/cdk/keycodes'; import {Directionality} from '@angular/cdk/bidi'; -import {take, takeUntil} from 'rxjs/operators'; +import {takeUntil} from 'rxjs/operators'; import {CdkMenuGroup} from './menu-group'; import {CDK_MENU} from './menu-interface'; import { @@ -70,10 +68,6 @@ export class CdkMenu extends CdkMenuBase implements AfterContentInit, OnDestroy /** Event emitted when the menu is closed. */ @Output() readonly closed: EventEmitter = new EventEmitter(); - /** List of nested CdkMenuGroup elements */ - @ContentChildren(CdkMenuGroup, {descendants: true}) - private readonly _nestedGroups: QueryList; - override _isInline = !this._parentTrigger; constructor( @@ -94,7 +88,6 @@ export class CdkMenu extends CdkMenuBase implements AfterContentInit, OnDestroy override ngAfterContentInit() { super.ngAfterContentInit(); - this._completeChangeEmitter(); this._subscribeToMenuStackEmptied(); this._subscribeToMouseManager(); this._menuAim?.initialize(this, this.pointerTracker!); @@ -147,30 +140,6 @@ export class CdkMenu extends CdkMenuBase implements AfterContentInit, OnDestroy } } - /** - * Complete the change emitter if there are any nested MenuGroups or register to complete the - * change emitter if a MenuGroup is rendered at some point - */ - // TODO(mmalerba): This doesnt' quite work. It causes change events to stop - // firing for radio items directly in the menu if a second group of options - // is added in a menu-group. - private _completeChangeEmitter() { - if (this._hasNestedGroups()) { - this.change.complete(); - } else { - this._nestedGroups.changes.pipe(take(1)).subscribe(() => this.change.complete()); - } - } - - /** Return true if there are nested CdkMenuGroup elements within the Menu */ - private _hasNestedGroups() { - // view engine has a bug where @ContentChildren will return the current element - // along with children if the selectors match - not just the children. - // Here, if there is at least one element, we check to see if the first element is a CdkMenu in - // order to ensure that we return true iff there are child CdkMenuGroup elements. - return this._nestedGroups.length > 0 && !(this._nestedGroups.first instanceof CdkMenu); - } - /** * Set the PointerFocusTracker and ensure that when mouse focus changes the key manager is updated * with the latest menu item under mouse focus. diff --git a/src/components-examples/cdk-experimental/menu/cdk-menu-standalone-stateful-menu/cdk-menu-standalone-stateful-menu-example.html b/src/components-examples/cdk-experimental/menu/cdk-menu-standalone-stateful-menu/cdk-menu-standalone-stateful-menu-example.html index 5fb0e1fa3bdf..470ac5b5585c 100644 --- a/src/components-examples/cdk-experimental/menu/cdk-menu-standalone-stateful-menu/cdk-menu-standalone-stateful-menu-example.html +++ b/src/components-examples/cdk-experimental/menu/cdk-menu-standalone-stateful-menu/cdk-menu-standalone-stateful-menu-example.html @@ -3,28 +3,29 @@
          -
          - - -
          diff --git a/src/components-examples/cdk-experimental/menu/cdk-menu-standalone-stateful-menu/cdk-menu-standalone-stateful-menu-example.ts b/src/components-examples/cdk-experimental/menu/cdk-menu-standalone-stateful-menu/cdk-menu-standalone-stateful-menu-example.ts index 06e41c0dafc3..e2e3d93e77f8 100644 --- a/src/components-examples/cdk-experimental/menu/cdk-menu-standalone-stateful-menu/cdk-menu-standalone-stateful-menu-example.ts +++ b/src/components-examples/cdk-experimental/menu/cdk-menu-standalone-stateful-menu/cdk-menu-standalone-stateful-menu-example.ts @@ -1,5 +1,4 @@ import {Component} from '@angular/core'; -import {CdkMenuItem} from '@angular/cdk-experimental/menu'; /** @title Stateful Menu with Standalone Trigger. */ @Component({ @@ -11,9 +10,6 @@ export class CdkMenuStandaloneStatefulMenuExample { bold = true; italic = false; - size: string | undefined = 'Normal'; - - onSizeChange(item: CdkMenuItem) { - this.size = item._elementRef.nativeElement.textContent?.trim(); - } + sizes = ['Small', 'Normal', 'Large']; + selectedSize: string | undefined = 'Normal'; } diff --git a/src/components-examples/cdk-experimental/menu/index.ts b/src/components-examples/cdk-experimental/menu/index.ts index e99877a15891..f37fc6fb93df 100644 --- a/src/components-examples/cdk-experimental/menu/index.ts +++ b/src/components-examples/cdk-experimental/menu/index.ts @@ -5,6 +5,7 @@ import {CdkMenuStandaloneStatefulMenuExample} from './cdk-menu-standalone-statef import {CdkMenuMenubarExample} from './cdk-menu-menubar/cdk-menu-menubar-example'; import {CdkMenuInlineExample} from './cdk-menu-inline/cdk-menu-inline-example'; import {CdkMenuContextExample} from './cdk-menu-context/cdk-menu-context-example'; +import {CommonModule} from '@angular/common'; export { CdkMenuStandaloneMenuExample, @@ -23,7 +24,7 @@ const EXAMPLES = [ ]; @NgModule({ - imports: [CdkMenuModule], + imports: [CdkMenuModule, CommonModule], declarations: EXAMPLES, exports: EXAMPLES, }) diff --git a/src/dev-app/cdk-experimental-menu/cdk-menu-demo.html b/src/dev-app/cdk-experimental-menu/cdk-menu-demo.html index e3abeb517c92..9a5199cec0a4 100644 --- a/src/dev-app/cdk-experimental-menu/cdk-menu-demo.html +++ b/src/dev-app/cdk-experimental-menu/cdk-menu-demo.html @@ -55,9 +55,9 @@

          MenuBar Example

          - - - + + +
          @@ -165,33 +165,49 @@

          Radio items

          -
          - - - +
          +
          -
          - - - +
          +
          -
          - - - +
          +
          -
          - - - +
          +
          diff --git a/src/dev-app/cdk-experimental-menu/cdk-menu-demo.ts b/src/dev-app/cdk-experimental-menu/cdk-menu-demo.ts index 9db26ca91d0f..8d3b2bba1da7 100644 --- a/src/dev-app/cdk-experimental-menu/cdk-menu-demo.ts +++ b/src/dev-app/cdk-experimental-menu/cdk-menu-demo.ts @@ -8,7 +8,6 @@ import {Component} from '@angular/core'; import {ConnectedPosition} from '@angular/cdk/overlay'; -import {CdkMenuItem} from '@angular/cdk-experimental/menu'; @Component({ templateUrl: 'cdk-menu-demo.html', @@ -19,14 +18,8 @@ export class CdkMenuDemo { {originX: 'center', originY: 'center', overlayX: 'center', overlayY: 'center'}, ] as ConnectedPosition[]; - size: string | undefined = 'Normal'; - color: string | undefined = 'Red'; - - onSizeChange(item: CdkMenuItem) { - this.size = item._elementRef.nativeElement.textContent?.trim(); - } - - onColorChange(item: CdkMenuItem) { - this.color = item._elementRef.nativeElement.textContent?.trim(); - } + sizes = ['Small', 'Normal', 'Large']; + colors = ['Red', 'Green', 'Blue']; + selectedSize: string | undefined = 'Normal'; + selectedColor: string | undefined = 'Red'; }