Skip to content

Commit

Permalink
fix(material/menu): remove dependency on animations module
Browse files Browse the repository at this point in the history
Second attempt at reworking the menu so it no longer depends on the animations module (after angular#30099). This should allow us to avoid some of the pitfalls of the animations module like occasional memory leaks (e.g. #47748) and reduce the bundle size.
  • Loading branch information
crisbeto committed Dec 11, 2024
1 parent 7739315 commit 5329cf8
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 83 deletions.
2 changes: 2 additions & 0 deletions src/material/menu/menu-animations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
* Animation duration and timing values are based on:
* https://material.io/guidelines/components/menus.html#menus-usage
* @docs-private
* @deprecated No longer used, will be removed.
* @breaking-change 21.0.0
*/
export const matMenuAnimations: {
readonly transformMenu: AnimationTriggerMetadata;
Expand Down
73 changes: 41 additions & 32 deletions src/material/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import {
} from '@angular/core';
import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
import {merge, Observable, of as observableOf, Subscription} from 'rxjs';
import {filter, takeUntil} from 'rxjs/operators';
import {filter, take, takeUntil} from 'rxjs/operators';
import {MatMenu, MenuCloseReason} from './menu';
import {throwMatMenuRecursiveError} from './menu-errors';
import {MatMenuItem} from './menu-item';
Expand Down Expand Up @@ -115,6 +115,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
private _closingActionsSubscription = Subscription.EMPTY;
private _hoverSubscription = Subscription.EMPTY;
private _menuCloseSubscription = Subscription.EMPTY;
private _pendingRemoval: Subscription | undefined;

/**
* We're specifically looking for a `MatMenu` here since the generic `MatMenuPanel`
Expand Down Expand Up @@ -247,6 +248,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
passiveEventListenerOptions,
);

this._pendingRemoval?.unsubscribe();
this._menuCloseSubscription.unsubscribe();
this._closingActionsSubscription.unsubscribe();
this._hoverSubscription.unsubscribe();
Expand Down Expand Up @@ -285,24 +287,39 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
return;
}

this._pendingRemoval?.unsubscribe();
const previousTrigger = PANELS_TO_TRIGGERS.get(menu);
PANELS_TO_TRIGGERS.set(menu, this);

// If the same menu is currently attached to another trigger,
// we need to close it so it doesn't end up in a broken state.
if (previousTrigger && previousTrigger !== this) {
previousTrigger.closeMenu();
}

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

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

if (menu.lazyContent) {
menu.lazyContent.attach(this.menuData);
// We need the `hasAttached` check for the case where the user kicked off a removal animation,
// but re-entered the menu. Re-attaching the same portal will trigger an error otherwise.
if (!overlayRef.hasAttached()) {
overlayRef.attach(this._getPortal(menu));
menu.lazyContent?.attach(this.menuData);
}

this._closingActionsSubscription = this._menuClosingActions().subscribe(() => this.closeMenu());
this._initMenu(menu);
menu.parentMenu = this.triggersSubmenu() ? this._parentMaterialMenu : undefined;
menu.direction = this.dir;
menu.focusFirstItem(this._openedBy || 'program');
this._setIsMenuOpen(true);

if (menu instanceof MatMenu) {
menu._startAnimation();
menu._setIsOpen(true);
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.
Expand Down Expand Up @@ -338,12 +355,28 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {

/** Closes the menu and does the necessary cleanup. */
private _destroyMenu(reason: MenuCloseReason) {
if (!this._overlayRef || !this.menuOpen) {
const overlayRef = this._overlayRef;
const menu = this._menu;

if (!overlayRef || !this.menuOpen) {
return;
}

this._closingActionsSubscription.unsubscribe();
this._overlayRef.detach();
this._pendingRemoval?.unsubscribe();

// Note that we don't wait for the animation to finish if another trigger took
// over the menu, because the panel will end up empty which looks glitchy.
if (menu instanceof MatMenu && this._ownsMenu(menu)) {
this._pendingRemoval = menu._animationDone.pipe(take(1)).subscribe(() => overlayRef.detach());
menu._setIsOpen(false);
} else {
overlayRef.detach();
}

if (menu && this._ownsMenu(menu)) {
PANELS_TO_TRIGGERS.delete(menu);
}

// Always restore focus if the user is navigating using the keyboard or the menu was opened
// programmatically. We don't restore for non-root triggers, because it can prevent focus
Expand All @@ -355,30 +388,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {

this._openedBy = undefined;
this._setIsMenuOpen(false);

if (this.menu && this._ownsMenu(this.menu)) {
PANELS_TO_TRIGGERS.delete(this.menu);
}
}

/**
* This method sets the menu state to open and focuses the first item if
* the menu was opened via the keyboard.
*/
private _initMenu(menu: MatMenuPanel): void {
const previousTrigger = PANELS_TO_TRIGGERS.get(menu);

// If the same menu is currently attached to another trigger,
// we need to close it so it doesn't end up in a broken state.
if (previousTrigger && previousTrigger !== this) {
previousTrigger.closeMenu();
}

PANELS_TO_TRIGGERS.set(menu, this);
menu.parentMenu = this.triggersSubmenu() ? this._parentMaterialMenu : undefined;
menu.direction = this.dir;
menu.focusFirstItem(this._openedBy || 'program');
this._setIsMenuOpen(true);
}

// set state rather than toggle to support triggers sharing a menu
Expand Down
9 changes: 6 additions & 3 deletions src/material/menu/menu.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@
class="mat-mdc-menu-panel"
[id]="panelId"
[class]="_classList"
[class.mat-menu-panel-animations-disabled]="_animationsDisabled"
[class.mat-menu-panel-exit-animation]="_panelAnimationState === 'void'"
[class.mat-menu-panel-animating]="_isAnimating"
(click)="closed.emit('click')"
[@transformMenu]="_panelAnimationState"
(@transformMenu.start)="_onAnimationStart($event)"
(@transformMenu.done)="_onAnimationDone($event)"
tabindex="-1"
role="menu"
(animationstart)="_onAnimationStart($event.animationName)"
(animationend)="_onAnimationDone($event.animationName)"
(animationcancel)="_onAnimationDone($event.animationName)"
[attr.aria-label]="ariaLabel || null"
[attr.aria-labelledby]="ariaLabelledby || null"
[attr.aria-describedby]="ariaDescribedby || null">
Expand Down
33 changes: 32 additions & 1 deletion src/material/menu/menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,33 @@ mat-menu {
}
}

@keyframes _mat-menu-enter {
from {
opacity: 0;
transform: scale(0.8);
}

to {
opacity: 1;
transform: none;
}
}

@keyframes _mat-menu-exit {
from {
opacity: 1;
}

to {
opacity: 0;
}
}

.mat-mdc-menu-panel {
@include menu-common.base;
box-sizing: border-box;
outline: 0;
animation: _mat-menu-enter 120ms cubic-bezier(0, 0, 0.2, 1);

@include token-utils.use-tokens(tokens-mat-menu.$prefix, tokens-mat-menu.get-token-slots()) {
@include token-utils.create-token-slot(border-radius, container-shape);
Expand All @@ -48,14 +71,22 @@ mat-menu {
// We should clean it up eventually and re-approve all the screenshots.
will-change: transform, opacity;

&.mat-menu-panel-exit-animation {
animation: _mat-menu-exit 100ms 25ms linear forwards;
}

&.mat-menu-panel-animations-disabled {
animation: none;
}

// Prevent users from interacting with the panel while it's animating. Note that
// people won't be able to click through it, because the overlay pane will catch the click.
// This fixes the following issues:
// * Users accidentally opening sub-menus when the `overlapTrigger` option is enabled.
// * Users accidentally tapping on content inside the sub-menu on touch devices, if the
// sub-menu overlaps the trigger. The issue is due to touch devices emulating the
// `mouseenter` event by dispatching it on tap.
&.ng-animating {
&.mat-menu-panel-animating {
pointer-events: none;

// If the same menu is assigned to multiple triggers and the user moves quickly between them
Expand Down
Loading

0 comments on commit 5329cf8

Please sign in to comment.