Skip to content

Commit

Permalink
fix(material/menu): remove dependency on animations module (#30163)
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 #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 authored Dec 13, 2024
1 parent a141c22 commit 4620df1
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 4620df1

Please sign in to comment.