Skip to content

Commit

Permalink
feat(material/menu): allow for menu to be conditionally removed from …
Browse files Browse the repository at this point in the history 
…trigger (angular#24437)

Adds support for conditionally removing the menu from a menu trigger by passing in `null`.

Fixes angular#24030.
  • Loading branch information
@crisbeto
crisbeto authored Feb 25, 2022
1 parent e86be88 commit 856c016
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 107 deletions.
27 changes: 13 additions & 14 deletions src/material-experimental/mdc-menu/menu.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,19 @@ describe('MDC-based MatMenu', () => {
);
}));

it('should set aria-haspopup based on whether a menu is assigned', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();
const triggerElement = fixture.componentInstance.triggerEl.nativeElement;

expect(triggerElement.getAttribute('aria-haspopup')).toBe('true');

fixture.componentInstance.trigger.menu = null;
fixture.detectChanges();

expect(triggerElement.hasAttribute('aria-haspopup')).toBe(false);
}));

it('should open the menu as an idempotent operation', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();
Expand Down Expand Up @@ -828,20 +841,6 @@ describe('MDC-based MatMenu', () => {
expect(triggerEl.hasAttribute('aria-expanded')).toBe(false);
}));

it('should throw the correct error if the menu is not defined after init', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();

fixture.componentInstance.trigger.menu = null!;
fixture.detectChanges();

expect(() => {
fixture.componentInstance.trigger.openMenu();
fixture.detectChanges();
tick(500);
}).toThrowError(/must pass in an mat-menu instance/);
}));

it('should throw if assigning a menu that contains the trigger', fakeAsync(() => {
expect(() => {
const fixture = createComponent(InvalidRecursiveMenu, [], [FakeIcon]);
Expand Down
12 changes: 0 additions & 12 deletions src/material/menu/menu-errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/

/**
* Throws an exception for the case when menu trigger doesn't have a valid mat-menu instance
* @docs-private
*/
export function throwMatMenuMissingError() {
throw Error(`matMenuTriggerFor: must pass in an mat-menu instance.
Example:
<mat-menu #menu="matMenu"></mat-menu>
<button [matMenuTriggerFor]="menu"></button>`);
}

/**
* Throws an exception for the case when menu's x-position value isn't valid.
* In other words, it doesn't match 'before' or 'after'.
Expand Down
115 changes: 52 additions & 63 deletions src/material/menu/menu-trigger.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
import {asapScheduler, merge, Observable, of as observableOf, Subscription} from 'rxjs';
import {delay, filter, take, takeUntil} from 'rxjs/operators';
import {_MatMenuBase, MenuCloseReason} from './menu';
import {throwMatMenuMissingError, throwMatMenuRecursiveError} from './menu-errors';
import {throwMatMenuRecursiveError} from './menu-errors';
import {MatMenuItem} from './menu-item';
import {MAT_MENU_PANEL, MatMenuPanel} from './menu-panel';
import {MenuPositionX, MenuPositionY} from './menu-positions';
Expand Down Expand Up @@ -75,7 +75,7 @@ const passiveEventListenerOptions = normalizePassiveListenerOptions({passive: tr

@Directive({
host: {
'aria-haspopup': 'true',
'[attr.aria-haspopup]': 'menu ? true : null',
'[attr.aria-expanded]': 'menuOpen || null',
'[attr.aria-controls]': 'menuOpen ? menu.panelId : null',
'(click)': '_handleClick($event)',
Expand Down Expand Up @@ -117,19 +117,19 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
* @breaking-change 8.0.0
*/
@Input('mat-menu-trigger-for')
get _deprecatedMatMenuTriggerFor(): MatMenuPanel {
get _deprecatedMatMenuTriggerFor(): MatMenuPanel | null {
return this.menu;
}
set _deprecatedMatMenuTriggerFor(v: MatMenuPanel) {
set _deprecatedMatMenuTriggerFor(v: MatMenuPanel | null) {
this.menu = v;
}

/** References the menu instance that the trigger is associated with. */
@Input('matMenuTriggerFor')
get menu() {
get menu(): MatMenuPanel | null {
return this._menu;
}
set menu(menu: MatMenuPanel) {
set menu(menu: MatMenuPanel | null) {
if (menu === this._menu) {
return;
}
Expand All @@ -152,7 +152,7 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
});
}
}
private _menu: MatMenuPanel;
private _menu: MatMenuPanel | null;

/** Data to be passed along to any lazily-rendered content. */
@Input('matMenuTriggerData') menuData: any;
Expand Down Expand Up @@ -244,7 +244,6 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
}

ngAfterContentInit() {
this._checkMenu();
this._handleHover();
}

Expand Down Expand Up @@ -287,31 +286,31 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy

/** Opens the menu. */
openMenu(): void {
if (this._menuOpen) {
const menu = this.menu;

if (this._menuOpen || !menu) {
return;
}

this._checkMenu();

const overlayRef = this._createOverlay();
const overlayRef = this._createOverlay(menu);
const overlayConfig = overlayRef.getConfig();
const positionStrategy = overlayConfig.positionStrategy as FlexibleConnectedPositionStrategy;

this._setPosition(positionStrategy);
this._setPosition(menu, positionStrategy);
overlayConfig.hasBackdrop =
this.menu.hasBackdrop == null ? !this.triggersSubmenu() : this.menu.hasBackdrop;
overlayRef.attach(this._getPortal());
menu.hasBackdrop == null ? !this.triggersSubmenu() : menu.hasBackdrop;
overlayRef.attach(this._getPortal(menu));

if (this.menu.lazyContent) {
this.menu.lazyContent.attach(this.menuData);
if (menu.lazyContent) {
menu.lazyContent.attach(this.menuData);
}

this._closingActionsSubscription = this._menuClosingActions().subscribe(() => this.closeMenu());
this._initMenu();
this._initMenu(menu);

if (this.menu instanceof _MatMenuBase) {
this.menu._startAnimation();
this.menu._directDescendantItems.changes.pipe(takeUntil(this.menu.close)).subscribe(() => {
if (menu instanceof _MatMenuBase) {
menu._startAnimation();
menu._directDescendantItems.changes.pipe(takeUntil(menu.close)).subscribe(() => {
// Re-adjust the position without locking when the amount of items
// changes so that the overlay is allowed to pick a new optimal position.
positionStrategy.withLockedPosition(false).reapplyLastPosition();
Expand All @@ -322,7 +321,7 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy

/** Closes the menu. */
closeMenu(): void {
this.menu.close.emit();
this.menu?.close.emit();
}

/**
Expand Down Expand Up @@ -386,37 +385,34 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
}
} else {
this._setIsMenuOpen(false);

if (menu.lazyContent) {
menu.lazyContent.detach();
}
menu?.lazyContent?.detach();
}
}

/**
* This method sets the menu state to open and focuses the first item if
* the menu was opened via the keyboard.
*/
private _initMenu(): void {
this.menu.parentMenu = this.triggersSubmenu() ? this._parentMaterialMenu : undefined;
this.menu.direction = this.dir;
this._setMenuElevation();
this.menu.focusFirstItem(this._openedBy || 'program');
private _initMenu(menu: MatMenuPanel): void {
menu.parentMenu = this.triggersSubmenu() ? this._parentMaterialMenu : undefined;
menu.direction = this.dir;
this._setMenuElevation(menu);
menu.focusFirstItem(this._openedBy || 'program');
this._setIsMenuOpen(true);
}

/** Updates the menu elevation based on the amount of parent menus that it has. */
private _setMenuElevation(): void {
if (this.menu.setElevation) {
private _setMenuElevation(menu: MatMenuPanel): void {
if (menu.setElevation) {
let depth = 0;
let parentMenu = this.menu.parentMenu;
let parentMenu = menu.parentMenu;

while (parentMenu) {
depth++;
parentMenu = parentMenu.parentMenu;
}

this.menu.setElevation(depth);
menu.setElevation(depth);
}
}

Expand All @@ -430,24 +426,17 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
}
}

/**
* This method checks that a valid instance of MatMenu has been passed into
* matMenuTriggerFor. If not, an exception is thrown.
*/
private _checkMenu() {
if (!this.menu && (typeof ngDevMode === 'undefined' || ngDevMode)) {
throwMatMenuMissingError();
}
}

/**
* This method creates the overlay from the provided menu's template and saves its
* OverlayRef so that it can be attached to the DOM when openMenu is called.
*/
private _createOverlay(): OverlayRef {
private _createOverlay(menu: MatMenuPanel): OverlayRef {
if (!this._overlayRef) {
const config = this._getOverlayConfig();
this._subscribeToPositions(config.positionStrategy as FlexibleConnectedPositionStrategy);
const config = this._getOverlayConfig(menu);
this._subscribeToPositions(
menu,
config.positionStrategy as FlexibleConnectedPositionStrategy,
);
this._overlayRef = this._overlay.create(config);

// Consume the `keydownEvents` in order to prevent them from going to another overlay.
Expand All @@ -463,16 +452,16 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
* This method builds the configuration object needed to create the overlay, the OverlayState.
* @returns OverlayConfig
*/
private _getOverlayConfig(): OverlayConfig {
private _getOverlayConfig(menu: MatMenuPanel): OverlayConfig {
return new OverlayConfig({
positionStrategy: this._overlay
.position()
.flexibleConnectedTo(this._element)
.withLockedPosition()
.withGrowAfterOpen()
.withTransformOriginOn('.mat-menu-panel, .mat-mdc-menu-panel'),
backdropClass: this.menu.backdropClass || 'cdk-overlay-transparent-backdrop',
panelClass: this.menu.overlayPanelClass,
backdropClass: menu.backdropClass || 'cdk-overlay-transparent-backdrop',
panelClass: menu.overlayPanelClass,
scrollStrategy: this._scrollStrategy(),
direction: this._dir,
});
Expand All @@ -483,8 +472,8 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
* on the menu based on the new position. This ensures the animation origin is always
* correct, even if a fallback position is used for the overlay.
*/
private _subscribeToPositions(position: FlexibleConnectedPositionStrategy): void {
if (this.menu.setPositionClasses) {
private _subscribeToPositions(menu: MatMenuPanel, position: FlexibleConnectedPositionStrategy) {
if (menu.setPositionClasses) {
position.positionChanges.subscribe(change => {
const posX: MenuPositionX = change.connectionPair.overlayX === 'start' ? 'after' : 'before';
const posY: MenuPositionY = change.connectionPair.overlayY === 'top' ? 'below' : 'above';
Expand All @@ -493,9 +482,9 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
// `positionChanges` fires outside of the `ngZone` and `setPositionClasses` might be
// updating something in the view so we need to bring it back in.
if (this._ngZone) {
this._ngZone.run(() => this.menu.setPositionClasses!(posX, posY));
this._ngZone.run(() => menu.setPositionClasses!(posX, posY));
} else {
this.menu.setPositionClasses!(posX, posY);
menu.setPositionClasses!(posX, posY);
}
});
}
Expand All @@ -506,12 +495,12 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
* so the overlay connects with the trigger correctly.
* @param positionStrategy Strategy whose position to update.
*/
private _setPosition(positionStrategy: FlexibleConnectedPositionStrategy) {
private _setPosition(menu: MatMenuPanel, positionStrategy: FlexibleConnectedPositionStrategy) {
let [originX, originFallbackX]: HorizontalConnectionPos[] =
this.menu.xPosition === 'before' ? ['end', 'start'] : ['start', 'end'];
menu.xPosition === 'before' ? ['end', 'start'] : ['start', 'end'];

let [overlayY, overlayFallbackY]: VerticalConnectionPos[] =
this.menu.yPosition === 'above' ? ['bottom', 'top'] : ['top', 'bottom'];
menu.yPosition === 'above' ? ['bottom', 'top'] : ['top', 'bottom'];

let [originY, originFallbackY] = [overlayY, overlayFallbackY];
let [overlayX, overlayFallbackX] = [originX, originFallbackX];
Expand All @@ -520,10 +509,10 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
if (this.triggersSubmenu()) {
// When the menu is a sub-menu, it should always align itself
// to the edges of the trigger, instead of overlapping it.
overlayFallbackX = originX = this.menu.xPosition === 'before' ? 'start' : 'end';
overlayFallbackX = originX = menu.xPosition === 'before' ? 'start' : 'end';
originFallbackX = overlayX = originX === 'end' ? 'start' : 'end';
offsetY = overlayY === 'bottom' ? MENU_PANEL_TOP_PADDING : -MENU_PANEL_TOP_PADDING;
} else if (!this.menu.overlapTrigger) {
} else if (!menu.overlapTrigger) {
originY = overlayY === 'top' ? 'bottom' : 'top';
originFallbackY = overlayFallbackY === 'top' ? 'bottom' : 'top';
}
Expand Down Expand Up @@ -644,12 +633,12 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
}

/** Gets the portal that should be attached to the overlay. */
private _getPortal(): TemplatePortal {
private _getPortal(menu: MatMenuPanel): TemplatePortal {
// Note that we can avoid this check by keeping the portal on the menu panel.
// While it would be cleaner, we'd have to introduce another required method on
// `MatMenuPanel`, making it harder to consume.
if (!this._portal || this._portal.templateRef !== this.menu.templateRef) {
this._portal = new TemplatePortal(this.menu.templateRef, this._viewContainerRef);
if (!this._portal || this._portal.templateRef !== menu.templateRef) {
this._portal = new TemplatePortal(menu.templateRef, this._viewContainerRef);
}

return this._portal;
Expand Down
27 changes: 13 additions & 14 deletions src/material/menu/menu.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,19 @@ describe('MatMenu', () => {
);
}));

it('should set aria-haspopup based on whether a menu is assigned', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();
const triggerElement = fixture.componentInstance.triggerEl.nativeElement;

expect(triggerElement.getAttribute('aria-haspopup')).toBe('true');

fixture.componentInstance.trigger.menu = null;
fixture.detectChanges();

expect(triggerElement.hasAttribute('aria-haspopup')).toBe(false);
}));

it('should open the menu as an idempotent operation', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();
Expand Down Expand Up @@ -827,20 +840,6 @@ describe('MatMenu', () => {
expect(triggerEl.hasAttribute('aria-expanded')).toBe(false);
}));

it('should throw the correct error if the menu is not defined after init', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();

fixture.componentInstance.trigger.menu = null!;
fixture.detectChanges();

expect(() => {
fixture.componentInstance.trigger.openMenu();
fixture.detectChanges();
tick(500);
}).toThrowError(/must pass in an mat-menu instance/);
}));

it('should throw if assigning a menu that contains the trigger', fakeAsync(() => {
expect(() => {
const fixture = createComponent(InvalidRecursiveMenu, [], [FakeIcon]);
Expand Down
Loading

0 comments on commit 856c016

Please sign in to comment.