From b3dc7590a38f96c17a53fb182ccd51bb9b6f4a47 Mon Sep 17 00:00:00 2001 From: David Klingenberg Date: Mon, 10 Oct 2022 16:25:29 +0200 Subject: [PATCH 1/5] feat(cdk/menu): Add tests for setting cdkMenuTriggerFor null --- src/cdk/menu/menu-trigger.spec.ts | 71 +++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/cdk/menu/menu-trigger.spec.ts b/src/cdk/menu/menu-trigger.spec.ts index 67bfa66f3793..0a16138afad6 100644 --- a/src/cdk/menu/menu-trigger.spec.ts +++ b/src/cdk/menu/menu-trigger.spec.ts @@ -1,3 +1,4 @@ +import {CdkContextMenuTrigger} from './context-menu-trigger'; import {Component, ViewChildren, QueryList, ElementRef, ViewChild, Type} from '@angular/core'; import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; @@ -469,6 +470,59 @@ describe('MenuTrigger', () => { expect(document.querySelector('.test-menu')?.textContent).toBe('Hello!'); }); + + describe('null triggerFor', () => { + let fixture: ComponentFixture; + + let nativeTrigger: HTMLElement; + let contextNativeTrigger: HTMLElement; + + const grabElementsForTesting = () => { + nativeTrigger = fixture.componentInstance.nativeTrigger.nativeElement; + }; + + /** run change detection and, extract and set the rendered elements */ + const detectChanges = () => { + fixture.detectChanges(); + grabElementsForTesting(); + }; + + beforeEach(waitForAsync(() => { + TestBed.configureTestingModule({ + imports: [CdkMenuModule], + declarations: [TriggerWithNullValue], + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(TriggerWithNullValue); + detectChanges(); + }); + + 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(); + 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); + detectChanges(); + expect(fixture.componentInstance.trigger.isOpen()).toBeFalse(); + }); + }); }); @Component({ @@ -602,3 +656,20 @@ class StandaloneTriggerWithInlineMenu { class TriggerWithData { menuData: unknown; } + +@Component({ + template: ` + + + `, +}) +class TriggerWithNullValue { + @ViewChild(CdkMenuTrigger) + trigger: CdkMenuTrigger; + + @ViewChild(CdkMenuTrigger, {read: ElementRef}) + nativeTrigger: ElementRef; + + @ViewChild(CdkContextMenuTrigger, {read: ElementRef}) + contextMenuNativeTrigger: ElementRef; +} From 450095e6241b04904ee6177fff2c0f03ed000af4 Mon Sep 17 00:00:00 2001 From: David Klingenberg Date: Mon, 10 Oct 2022 16:11:19 +0200 Subject: [PATCH 2/5] feat(cdk/menu): Allow setting cdkMenuTrigger null Add ability to set [cdkMenuTrigger]="null" as is now possible with material menu. Fixes #25782 --- src/cdk/menu/menu-trigger-base.ts | 2 +- src/cdk/menu/menu-trigger.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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.ts b/src/cdk/menu/menu-trigger.ts index 9f5968351420..bb95b203ef6c 100644 --- a/src/cdk/menu/menu-trigger.ts +++ b/src/cdk/menu/menu-trigger.ts @@ -45,7 +45,7 @@ import {CdkMenuTriggerBase, MENU_TRIGGER} from './menu-trigger-base'; standalone: true, host: { 'class': 'cdk-menu-trigger', - 'aria-haspopup': 'menu', + '[attr.aria-haspopup]': 'menuTemplateRef ? "menu" : null', '[attr.aria-expanded]': 'isOpen()', '(focusin)': '_setHasFocus(true)', '(focusout)': '_setHasFocus(false)', @@ -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()); From 4a77921e37d2e06dfaefa1d0ce48681040838a34 Mon Sep 17 00:00:00 2001 From: David Klingenberg Date: Fri, 28 Oct 2022 19:58:05 +0200 Subject: [PATCH 3/5] feat(cdk/menu): Update tests for setting cdkMenuTriggerFor null --- src/cdk/menu/menu-item.ts | 8 ++++- src/cdk/menu/menu-trigger.spec.ts | 54 ++++++++++++++++++++----------- src/cdk/menu/menu-trigger.ts | 2 +- 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/src/cdk/menu/menu-item.ts b/src/cdk/menu/menu-item.ts index 1bd0c91b6143..e38fc9080211 100644 --- a/src/cdk/menu/menu-item.ts +++ b/src/cdk/menu/menu-item.ts @@ -93,7 +93,13 @@ 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() { + if (this._menuTrigger?.menuTemplateRef == null) { + return false; + } + + return true; + } /** * The tabindex for this menu item managed internally and used for implementing roving a diff --git a/src/cdk/menu/menu-trigger.spec.ts b/src/cdk/menu/menu-trigger.spec.ts index 0a16138afad6..ab536c61e6b9 100644 --- a/src/cdk/menu/menu-trigger.spec.ts +++ b/src/cdk/menu/menu-trigger.spec.ts @@ -48,12 +48,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(); }); }); @@ -471,21 +495,10 @@ describe('MenuTrigger', () => { expect(document.querySelector('.test-menu')?.textContent).toBe('Hello!'); }); - describe('null triggerFor', () => { + xdescribe('null triggerFor', () => { let fixture: ComponentFixture; let nativeTrigger: HTMLElement; - let contextNativeTrigger: HTMLElement; - - const grabElementsForTesting = () => { - nativeTrigger = fixture.componentInstance.nativeTrigger.nativeElement; - }; - - /** run change detection and, extract and set the rendered elements */ - const detectChanges = () => { - fixture.detectChanges(); - grabElementsForTesting(); - }; beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ @@ -496,7 +509,7 @@ describe('MenuTrigger', () => { beforeEach(() => { fixture = TestBed.createComponent(TriggerWithNullValue); - detectChanges(); + nativeTrigger = fixture.componentInstance.nativeTrigger.nativeElement }); it('should not set aria-haspopup', () => { @@ -511,7 +524,8 @@ describe('MenuTrigger', () => { expect(fixture.componentInstance.trigger.isOpen()).toBeFalse(); nativeTrigger.click(); - detectChanges(); + fixture.detectChanges(); + expect(fixture.componentInstance.trigger.isOpen()).toBeFalse(); }); @@ -519,7 +533,8 @@ describe('MenuTrigger', () => { expect(fixture.componentInstance.trigger.isOpen()).toBeFalse(); dispatchKeyboardEvent(nativeTrigger, 'keydown', SPACE); - detectChanges(); + fixture.detectChanges(); + expect(fixture.componentInstance.trigger.isOpen()).toBeFalse(); }); }); @@ -531,7 +546,10 @@ describe('MenuTrigger', () => {
`, }) -class TriggerForEmptyMenu {} +class TriggerForEmptyMenu { + @ViewChild(CdkMenuTrigger) trigger: CdkMenuTrigger; + @ViewChild(CdkMenuTrigger, { read: ElementRef }) nativeTrigger: ElementRef; +} @Component({ template: ` diff --git a/src/cdk/menu/menu-trigger.ts b/src/cdk/menu/menu-trigger.ts index bb95b203ef6c..d1fe9f8c825f 100644 --- a/src/cdk/menu/menu-trigger.ts +++ b/src/cdk/menu/menu-trigger.ts @@ -46,7 +46,7 @@ import {CdkMenuTriggerBase, MENU_TRIGGER} from './menu-trigger-base'; host: { 'class': 'cdk-menu-trigger', '[attr.aria-haspopup]': 'menuTemplateRef ? "menu" : null', - '[attr.aria-expanded]': 'isOpen()', + '[attr.aria-expanded]': 'menuTemplateRef == null ? null : isOpen()', '(focusin)': '_setHasFocus(true)', '(focusout)': '_setHasFocus(false)', '(keydown)': '_toggleOnKeydown($event)', From 0ccdc02e4e732077a35436e3076bb32a4d390d80 Mon Sep 17 00:00:00 2001 From: David Klingenberg Date: Tue, 8 Nov 2022 19:01:34 +0100 Subject: [PATCH 4/5] feat(cdk/menu): Allow setting cdkMenuTrigger null - Update hasMenu - Add yarn approve-api cdk/menu - Uncomment xdescribe and simplify it only for cdkMenuTrigger --- src/cdk/menu/menu-item.ts | 6 +----- src/cdk/menu/menu-trigger.spec.ts | 26 +++++++++++--------------- tools/public_api_guard/cdk/menu.md | 4 ++-- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/src/cdk/menu/menu-item.ts b/src/cdk/menu/menu-item.ts index e38fc9080211..84a5e7a4213c 100644 --- a/src/cdk/menu/menu-item.ts +++ b/src/cdk/menu/menu-item.ts @@ -94,11 +94,7 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler, /** Whether the menu item opens a menu. */ get hasMenu() { - if (this._menuTrigger?.menuTemplateRef == null) { - return false; - } - - return true; + return this._menuTrigger?.menuTemplateRef != null; } /** diff --git a/src/cdk/menu/menu-trigger.spec.ts b/src/cdk/menu/menu-trigger.spec.ts index ab536c61e6b9..adc8d2bc5e31 100644 --- a/src/cdk/menu/menu-trigger.spec.ts +++ b/src/cdk/menu/menu-trigger.spec.ts @@ -71,13 +71,13 @@ describe('MenuTrigger', () => { }); 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'); + expect(menuItemElement.hasAttribute('aria-expanded')).toBeTrue(); + expect(menuItemElement.getAttribute('aria-expanded')).toBe('false'); - fixture.componentInstance.trigger.menuTemplateRef = null; - fixture.detectChanges(); + fixture.componentInstance.trigger.menuTemplateRef = null; + fixture.detectChanges(); - expect(menuItemElement.hasAttribute('aria-expanded')).toBeFalse(); + expect(menuItemElement.hasAttribute('aria-expanded')).toBeFalse(); }); }); @@ -495,7 +495,7 @@ describe('MenuTrigger', () => { expect(document.querySelector('.test-menu')?.textContent).toBe('Hello!'); }); - xdescribe('null triggerFor', () => { + describe('null triggerFor', () => { let fixture: ComponentFixture; let nativeTrigger: HTMLElement; @@ -509,7 +509,7 @@ describe('MenuTrigger', () => { beforeEach(() => { fixture = TestBed.createComponent(TriggerWithNullValue); - nativeTrigger = fixture.componentInstance.nativeTrigger.nativeElement + nativeTrigger = fixture.componentInstance.nativeTrigger.nativeElement; }); it('should not set aria-haspopup', () => { @@ -547,8 +547,8 @@ describe('MenuTrigger', () => { `, }) class TriggerForEmptyMenu { - @ViewChild(CdkMenuTrigger) trigger: CdkMenuTrigger; - @ViewChild(CdkMenuTrigger, { read: ElementRef }) nativeTrigger: ElementRef; + @ViewChild(CdkMenuTrigger) trigger: CdkMenuTrigger; + @ViewChild(CdkMenuTrigger, {read: ElementRef}) nativeTrigger: ElementRef; } @Component({ @@ -678,16 +678,12 @@ class TriggerWithData { @Component({ template: ` - `, }) class TriggerWithNullValue { - @ViewChild(CdkMenuTrigger) + @ViewChild(CdkMenuTrigger, {static: true}) trigger: CdkMenuTrigger; - @ViewChild(CdkMenuTrigger, {read: ElementRef}) + @ViewChild(CdkMenuTrigger, {static: true, read: ElementRef}) nativeTrigger: ElementRef; - - @ViewChild(CdkContextMenuTrigger, {read: ElementRef}) - contextMenuNativeTrigger: ElementRef; } 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; From 3930736a80f379b6f21cbf581edcba95c09d3939 Mon Sep 17 00:00:00 2001 From: David Klingenberg Date: Wed, 9 Nov 2022 09:13:07 +0100 Subject: [PATCH 5/5] feat(cdk/menu): Allow setting cdkMenuTrigger null Remove unused import --- src/cdk/menu/menu-trigger.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cdk/menu/menu-trigger.spec.ts b/src/cdk/menu/menu-trigger.spec.ts index adc8d2bc5e31..9fb43112b819 100644 --- a/src/cdk/menu/menu-trigger.spec.ts +++ b/src/cdk/menu/menu-trigger.spec.ts @@ -1,4 +1,3 @@ -import {CdkContextMenuTrigger} from './context-menu-trigger'; import {Component, ViewChildren, QueryList, ElementRef, ViewChild, Type} from '@angular/core'; import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; import {By} from '@angular/platform-browser';