Skip to content

feat(material/menu): allow for menu to be conditionally removed from trigger #24437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 25, 2022
Merged

feat(material/menu): allow for menu to be conditionally removed from trigger #24437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions src/material-experimental/mdc-menu/menu.spec.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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