From e0a74b9c2bf0aabaedbfdc13ebc2cb3e7f24115b Mon Sep 17 00:00:00 2001 From: David Klingenberg Date: Wed, 9 Nov 2022 10:17:51 +0100 Subject: [PATCH] feat(cdk/menu): Allow setting cdkMenuTriggerFor null (#25818) * feat(cdk/menu): Add tests for setting cdkMenuTriggerFor null * feat(cdk/menu): Allow setting cdkMenuTrigger null Add ability to set [cdkMenuTrigger]="null" as is now possible with material menu. Fixes #25782 * feat(cdk/menu): Update tests for setting cdkMenuTriggerFor null * feat(cdk/menu): Allow setting cdkMenuTrigger null - Update hasMenu - Add yarn approve-api cdk/menu - Uncomment xdescribe and simplify it only for cdkMenuTrigger * feat(cdk/menu): Allow setting cdkMenuTrigger null Remove unused import --- src/cdk/menu/menu-item.ts | 4 +- src/cdk/menu/menu-trigger-base.ts | 2 +- src/cdk/menu/menu-trigger.spec.ts | 90 +++++++++++++++++++++++++++++- src/cdk/menu/menu-trigger.ts | 6 +- tools/public_api_guard/cdk/menu.md | 4 +- 5 files changed, 96 insertions(+), 10 deletions(-) diff --git a/src/cdk/menu/menu-item.ts b/src/cdk/menu/menu-item.ts index 1bd0c91b6143..84a5e7a4213c 100644 --- a/src/cdk/menu/menu-item.ts +++ b/src/cdk/menu/menu-item.ts @@ -93,7 +93,9 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler, @Output('cdkMenuItemTriggered') readonly triggered: EventEmitter = new EventEmitter(); /** Whether the menu item opens a menu. */ - readonly hasMenu = !!this._menuTrigger; + get hasMenu() { + return this._menuTrigger?.menuTemplateRef != null; + } /** * The tabindex for this menu item managed internally and used for implementing roving a diff --git a/src/cdk/menu/menu-trigger-base.ts b/src/cdk/menu/menu-trigger-base.ts index 2f4f3180acea..bf0ab7509a61 100644 --- a/src/cdk/menu/menu-trigger-base.ts +++ b/src/cdk/menu/menu-trigger-base.ts @@ -58,7 +58,7 @@ export abstract class CdkMenuTriggerBase implements OnDestroy { readonly closed: EventEmitter = new EventEmitter(); /** Template reference variable to the menu this trigger opens */ - menuTemplateRef: TemplateRef; + menuTemplateRef: TemplateRef | null; /** Context data to be passed along to the menu template */ menuData: unknown; diff --git a/src/cdk/menu/menu-trigger.spec.ts b/src/cdk/menu/menu-trigger.spec.ts index 67bfa66f3793..9fb43112b819 100644 --- a/src/cdk/menu/menu-trigger.spec.ts +++ b/src/cdk/menu/menu-trigger.spec.ts @@ -47,12 +47,36 @@ describe('MenuTrigger', () => { expect(menuItemElement.getAttribute('aria-disabled')).toBe('true'); }); - it('should set aria-haspopup to menu', () => { + it('should set aria-haspopup based on whether a menu is assigned', () => { expect(menuItemElement.getAttribute('aria-haspopup')).toEqual('menu'); + + fixture.componentInstance.trigger.menuTemplateRef = null; + fixture.detectChanges(); + + expect(menuItemElement.hasAttribute('aria-haspopup')).toBe(false); }); - it('should have a menu', () => { + it('should have a menu based on whether a menu is assigned', () => { expect(menuItem.hasMenu).toBeTrue(); + + fixture.componentInstance.trigger.menuTemplateRef = null; + fixture.detectChanges(); + + expect(menuItem.hasMenu).toBeFalse(); + }); + + it('should set aria-controls based on whether a menu is assigned', () => { + expect(menuItemElement.hasAttribute('aria-controls')).toBeFalse(); + }); + + it('should set aria-expanded based on whether a menu is assigned', () => { + expect(menuItemElement.hasAttribute('aria-expanded')).toBeTrue(); + expect(menuItemElement.getAttribute('aria-expanded')).toBe('false'); + + fixture.componentInstance.trigger.menuTemplateRef = null; + fixture.detectChanges(); + + expect(menuItemElement.hasAttribute('aria-expanded')).toBeFalse(); }); }); @@ -469,6 +493,50 @@ describe('MenuTrigger', () => { expect(document.querySelector('.test-menu')?.textContent).toBe('Hello!'); }); + + describe('null triggerFor', () => { + let fixture: ComponentFixture; + + let nativeTrigger: HTMLElement; + + beforeEach(waitForAsync(() => { + TestBed.configureTestingModule({ + imports: [CdkMenuModule], + declarations: [TriggerWithNullValue], + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(TriggerWithNullValue); + nativeTrigger = fixture.componentInstance.nativeTrigger.nativeElement; + }); + + it('should not set aria-haspopup', () => { + expect(nativeTrigger.hasAttribute('aria-haspopup')).toBeFalse(); + }); + + it('should not set aria-controls', () => { + expect(nativeTrigger.hasAttribute('aria-controls')).toBeFalse(); + }); + + it('should not toggle the menu on trigger', () => { + expect(fixture.componentInstance.trigger.isOpen()).toBeFalse(); + + nativeTrigger.click(); + fixture.detectChanges(); + + expect(fixture.componentInstance.trigger.isOpen()).toBeFalse(); + }); + + it('should not toggle the menu on keyboard events', () => { + expect(fixture.componentInstance.trigger.isOpen()).toBeFalse(); + + dispatchKeyboardEvent(nativeTrigger, 'keydown', SPACE); + fixture.detectChanges(); + + expect(fixture.componentInstance.trigger.isOpen()).toBeFalse(); + }); + }); }); @Component({ @@ -477,7 +545,10 @@ describe('MenuTrigger', () => {
`, }) -class TriggerForEmptyMenu {} +class TriggerForEmptyMenu { + @ViewChild(CdkMenuTrigger) trigger: CdkMenuTrigger; + @ViewChild(CdkMenuTrigger, {read: ElementRef}) nativeTrigger: ElementRef; +} @Component({ template: ` @@ -602,3 +673,16 @@ class StandaloneTriggerWithInlineMenu { class TriggerWithData { menuData: unknown; } + +@Component({ + template: ` + + `, +}) +class TriggerWithNullValue { + @ViewChild(CdkMenuTrigger, {static: true}) + trigger: CdkMenuTrigger; + + @ViewChild(CdkMenuTrigger, {static: true, read: ElementRef}) + nativeTrigger: ElementRef; +} diff --git a/src/cdk/menu/menu-trigger.ts b/src/cdk/menu/menu-trigger.ts index 9f5968351420..d1fe9f8c825f 100644 --- a/src/cdk/menu/menu-trigger.ts +++ b/src/cdk/menu/menu-trigger.ts @@ -45,8 +45,8 @@ import {CdkMenuTriggerBase, MENU_TRIGGER} from './menu-trigger-base'; standalone: true, host: { 'class': 'cdk-menu-trigger', - 'aria-haspopup': 'menu', - '[attr.aria-expanded]': 'isOpen()', + '[attr.aria-haspopup]': 'menuTemplateRef ? "menu" : null', + '[attr.aria-expanded]': 'menuTemplateRef == null ? null : isOpen()', '(focusin)': '_setHasFocus(true)', '(focusout)': '_setHasFocus(false)', '(keydown)': '_toggleOnKeydown($event)', @@ -99,7 +99,7 @@ export class CdkMenuTrigger extends CdkMenuTriggerBase implements OnDestroy { /** Open the attached menu. */ open() { - if (!this.isOpen()) { + if (!this.isOpen() && this.menuTemplateRef != null) { this.opened.next(); this.overlayRef = this.overlayRef || this._overlay.create(this._getOverlayConfig()); diff --git a/tools/public_api_guard/cdk/menu.md b/tools/public_api_guard/cdk/menu.md index a7d882f8d9a7..6338441cb57a 100644 --- a/tools/public_api_guard/cdk/menu.md +++ b/tools/public_api_guard/cdk/menu.md @@ -127,7 +127,7 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler, getLabel(): string; getMenu(): Menu | undefined; getMenuTrigger(): CdkMenuTrigger | null; - readonly hasMenu: boolean; + get hasMenu(): boolean; isMenuOpen(): boolean; // (undocumented) ngOnDestroy(): void; @@ -220,7 +220,7 @@ export abstract class CdkMenuTriggerBase implements OnDestroy { menuData: unknown; menuPosition: ConnectedPosition[]; protected readonly menuStack: MenuStack; - menuTemplateRef: TemplateRef; + menuTemplateRef: TemplateRef | null; // (undocumented) ngOnDestroy(): void; readonly opened: EventEmitter;