Skip to content

Commit

Permalink
feat(cdk/menu): Allow setting cdkMenuTriggerFor null (#25818)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
klinki authored Nov 9, 2022
1 parent edb2d56 commit e0a74b9
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 10 deletions.
4 changes: 3 additions & 1 deletion src/cdk/menu/menu-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
@Output('cdkMenuItemTriggered') readonly triggered: EventEmitter<void> = 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
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/menu/menu-trigger-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export abstract class CdkMenuTriggerBase implements OnDestroy {
readonly closed: EventEmitter<void> = new EventEmitter();

/** Template reference variable to the menu this trigger opens */
menuTemplateRef: TemplateRef<unknown>;
menuTemplateRef: TemplateRef<unknown> | null;

/** Context data to be passed along to the menu template */
menuData: unknown;
Expand Down
90 changes: 87 additions & 3 deletions src/cdk/menu/menu-trigger.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});

Expand Down Expand Up @@ -469,6 +493,50 @@ describe('MenuTrigger', () => {

expect(document.querySelector('.test-menu')?.textContent).toBe('Hello!');
});

describe('null triggerFor', () => {
let fixture: ComponentFixture<TriggerWithNullValue>;

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({
Expand All @@ -477,7 +545,10 @@ describe('MenuTrigger', () => {
<ng-template #noop><div cdkMenu></div></ng-template>
`,
})
class TriggerForEmptyMenu {}
class TriggerForEmptyMenu {
@ViewChild(CdkMenuTrigger) trigger: CdkMenuTrigger;
@ViewChild(CdkMenuTrigger, {read: ElementRef}) nativeTrigger: ElementRef;
}

@Component({
template: `
Expand Down Expand Up @@ -602,3 +673,16 @@ class StandaloneTriggerWithInlineMenu {
class TriggerWithData {
menuData: unknown;
}

@Component({
template: `
<button [cdkMenuTriggerFor]="null">First</button>
`,
})
class TriggerWithNullValue {
@ViewChild(CdkMenuTrigger, {static: true})
trigger: CdkMenuTrigger;

@ViewChild(CdkMenuTrigger, {static: true, read: ElementRef})
nativeTrigger: ElementRef<HTMLElement>;
}
6 changes: 3 additions & 3 deletions src/cdk/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)',
Expand Down Expand Up @@ -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());
Expand Down
4 changes: 2 additions & 2 deletions tools/public_api_guard/cdk/menu.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -220,7 +220,7 @@ export abstract class CdkMenuTriggerBase implements OnDestroy {
menuData: unknown;
menuPosition: ConnectedPosition[];
protected readonly menuStack: MenuStack;
menuTemplateRef: TemplateRef<unknown>;
menuTemplateRef: TemplateRef<unknown> | null;
// (undocumented)
ngOnDestroy(): void;
readonly opened: EventEmitter<void>;
Expand Down

0 comments on commit e0a74b9

Please sign in to comment.