From 9113846a067a82cf3b084adc6e99b573a8f04e8a Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 29 Dec 2024 09:06:59 +0200 Subject: [PATCH] fix(material/sidenav): switch away from animations module Reworks the sidenav to animate using CSS, rather than the animations module. This requires less JavaScript, is simpler to maintain and avoids some memory leaks caused by the animations module. --- src/dev-app/sidenav/sidenav-demo.ts | 4 +- src/material/sidenav/drawer-animations.ts | 2 + src/material/sidenav/drawer.scss | 30 ++-- src/material/sidenav/drawer.ts | 158 +++++++++++++-------- src/material/sidenav/sidenav.ts | 3 - tools/public_api_guard/material/sidenav.md | 15 +- 6 files changed, 129 insertions(+), 83 deletions(-) diff --git a/src/dev-app/sidenav/sidenav-demo.ts b/src/dev-app/sidenav/sidenav-demo.ts index d62f3fcc70be..975e7d85516d 100644 --- a/src/dev-app/sidenav/sidenav-demo.ts +++ b/src/dev-app/sidenav/sidenav-demo.ts @@ -22,7 +22,9 @@ import {MatToolbarModule} from '@angular/material/toolbar'; }) export class SidenavDemo { isLaunched = false; - fillerContent = Array(30); + fillerContent = Array(30) + .fill(null) + .map((_, index) => index); fixed = false; coverHeader = false; showHeader = false; diff --git a/src/material/sidenav/drawer-animations.ts b/src/material/sidenav/drawer-animations.ts index 10586a7840bb..c4c3b7570b25 100644 --- a/src/material/sidenav/drawer-animations.ts +++ b/src/material/sidenav/drawer-animations.ts @@ -17,6 +17,8 @@ import { /** * Animations used by the Material drawers. * @docs-private + * @deprecated No longer used, will be removed. + * @breaking-change 21.0.0 */ export const matDrawerAnimations: { readonly transformDrawer: AnimationTriggerMetadata; diff --git a/src/material/sidenav/drawer.scss b/src/material/sidenav/drawer.scss index 0d2d6be652b2..2bd5e859f08b 100644 --- a/src/material/sidenav/drawer.scss +++ b/src/material/sidenav/drawer.scss @@ -211,15 +211,27 @@ $drawer-over-drawer-z-index: 4; } } - // Usually the `visibility: hidden` added by the animation is enough to prevent focus from - // entering the hidden drawer content, but children with their own `visibility` can override it. - // This is a fallback that completely hides the content when the element becomes hidden. - // Note that we can't do this in the animation definition, because the style gets recomputed too - // late, breaking the animation because Angular didn't have time to figure out the target - // transform. This can also be achieved with JS, but it has issues when starting an - // animation before the previous one has finished. - &[style*='visibility: hidden'] { - display: none; + .mat-drawer-transition & { + transition: transform 400ms cubic-bezier(0.25, 0.8, 0.25, 1); + } + + &:not(.mat-drawer-opened):not(.mat-drawer-animating) { + // Stops the sidenav from poking out (e.g. with the box shadow) while it's off-screen. + // We can't use `display` because it interrupts the transition and `transition-behaviof` + // isn't available in all browsers. + visibility: hidden; + box-shadow: none; + + // The `visibility` above should prevent focus from entering the sidenav, but if a child + // element has `visibility`, it'll override the inherited value. This guarantees that the + // content won't be focusable. + .mat-drawer-inner-container { + display: none; + } + } + + &.mat-drawer-opened { + transform: none; } } diff --git a/src/material/sidenav/drawer.ts b/src/material/sidenav/drawer.ts index 0a4b58e137cd..bc3285664e34 100644 --- a/src/material/sidenav/drawer.ts +++ b/src/material/sidenav/drawer.ts @@ -5,7 +5,6 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.dev/license */ -import {AnimationEvent} from '@angular/animations'; import { FocusMonitor, FocusOrigin, @@ -20,7 +19,6 @@ import {Platform} from '@angular/cdk/platform'; import {CdkScrollable, ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling'; import {DOCUMENT} from '@angular/common'; import { - AfterContentChecked, AfterContentInit, afterNextRender, AfterRenderPhase, @@ -48,7 +46,6 @@ import { } from '@angular/core'; import {fromEvent, merge, Observable, Subject} from 'rxjs'; import {debounceTime, filter, map, mapTo, startWith, take, takeUntil} from 'rxjs/operators'; -import {matDrawerAnimations} from './drawer-animations'; /** * Throws an exception when two MatDrawer are matching the same position. @@ -152,7 +149,6 @@ export class MatDrawerContent extends CdkScrollable implements AfterContentInit selector: 'mat-drawer', exportAs: 'matDrawer', templateUrl: 'drawer.html', - animations: [matDrawerAnimations.transformDrawer], host: { 'class': 'mat-drawer', // must prevent the browser from aligning text based on value @@ -161,17 +157,17 @@ export class MatDrawerContent extends CdkScrollable implements AfterContentInit '[class.mat-drawer-over]': 'mode === "over"', '[class.mat-drawer-push]': 'mode === "push"', '[class.mat-drawer-side]': 'mode === "side"', - '[class.mat-drawer-opened]': 'opened', + // The styles that render the sidenav off-screen come from the drawer container. Prior to #30235 + // this was also done by the animations module which some internal tests seem to depend on. + // Simulate it by toggling the `hidden` attribute instead. + '[style.visibility]': '(!_container && !opened) ? "hidden" : null', 'tabIndex': '-1', - '[@transform]': '_animationState', - '(@transform.start)': '_animationStarted.next($event)', - '(@transform.done)': '_animationEnd.next($event)', }, changeDetection: ChangeDetectionStrategy.OnPush, encapsulation: ViewEncapsulation.None, imports: [CdkScrollable], }) -export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy { +export class MatDrawer implements AfterViewInit, OnDestroy { private _elementRef = inject>(ElementRef); private _focusTrapFactory = inject(FocusTrapFactory); private _focusMonitor = inject(FocusMonitor); @@ -184,9 +180,8 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy private _focusTrap: FocusTrap | null = null; private _elementFocusedBeforeDrawerWasOpened: HTMLElement | null = null; - - /** Whether the drawer is initialized. Used for disabling the initial animation. */ - private _enableAnimations = false; + private _eventCleanups: (() => void)[]; + private _fallbackTimer: ReturnType | undefined; /** Whether the view of the component has been attached. */ private _isAttached: boolean; @@ -284,13 +279,10 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy private _openedVia: FocusOrigin | null; /** Emits whenever the drawer has started animating. */ - readonly _animationStarted = new Subject(); + readonly _animationStarted = new Subject(); /** Emits whenever the drawer is done animating. */ - readonly _animationEnd = new Subject(); - - /** Current state of the sidenav animation. */ - _animationState: 'open-instant' | 'open' | 'void' = 'void'; + readonly _animationEnd = new Subject(); /** Event emitted when the drawer open state is changed. */ @Output() readonly openedChange: EventEmitter = @@ -307,7 +299,7 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy /** Event emitted when the drawer has started opening. */ @Output() readonly openedStart: Observable = this._animationStarted.pipe( - filter(e => e.fromState !== e.toState && e.toState.indexOf('open') === 0), + filter(() => this.opened), mapTo(undefined), ); @@ -321,7 +313,7 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy /** Event emitted when the drawer has started closing. */ @Output() readonly closedStart: Observable = this._animationStarted.pipe( - filter(e => e.fromState !== e.toState && e.toState === 'void'), + filter(() => !this.opened), mapTo(undefined), ); @@ -364,7 +356,8 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy * and we don't have close disabled. */ this._ngZone.runOutsideAngular(() => { - (fromEvent(this._elementRef.nativeElement, 'keydown') as Observable) + const element = this._elementRef.nativeElement; + (fromEvent(element, 'keydown') as Observable) .pipe( filter(event => { return event.keyCode === ESCAPE && !this.disableClose && !hasModifierKey(event); @@ -378,17 +371,16 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy event.preventDefault(); }), ); - }); - this._animationEnd.subscribe((event: AnimationEvent) => { - const {fromState, toState} = event; + this._eventCleanups = [ + this._renderer.listen(element, 'transitionrun', this._handleTransitionEvent), + this._renderer.listen(element, 'transitionend', this._handleTransitionEvent), + this._renderer.listen(element, 'transitioncancel', this._handleTransitionEvent), + ]; + }); - if ( - (toState.indexOf('open') === 0 && fromState === 'void') || - (toState === 'void' && fromState.indexOf('open') === 0) - ) { - this.openedChange.emit(this._opened); - } + this._animationEnd.subscribe(() => { + this.openedChange.emit(this._opened); }); } @@ -508,17 +500,9 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy } } - ngAfterContentChecked() { - // Enable the animations after the lifecycle hooks have run, in order to avoid animating - // drawers that are open by default. When we're on the server, we shouldn't enable the - // animations, because we don't want the drawer to animate the first time the user sees - // the page. - if (this._platform.isBrowser) { - this._enableAnimations = true; - } - } - ngOnDestroy() { + clearTimeout(this._fallbackTimer); + this._eventCleanups.forEach(cleanup => cleanup()); this._focusTrap?.destroy(); this._anchor?.remove(); this._anchor = null; @@ -588,15 +572,28 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy restoreFocus: boolean, focusOrigin: Exclude, ): Promise { + if (isOpen === this._opened) { + return Promise.resolve(isOpen ? 'open' : 'close'); + } + this._opened = isOpen; - if (isOpen) { - this._animationState = this._enableAnimations ? 'open' : 'open-instant'; + if (this._container?._transitionsEnabled) { + // Note: it's importatnt to set this as early as possible, + // otherwise the animation can look glitchy in some cases. + this._setIsAnimating(true); } else { - this._animationState = 'void'; - if (restoreFocus) { - this._restoreFocus(focusOrigin); - } + // Simulate the animation events if animations are disabled. + setTimeout(() => { + this._animationStarted.next(); + this._animationEnd.next(); + }); + } + + this._elementRef.nativeElement.classList.toggle('mat-drawer-opened', isOpen); + + if (!isOpen && restoreFocus) { + this._restoreFocus(focusOrigin); } // Needed to ensure that the closing sequence fires off correctly. @@ -608,8 +605,23 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy }); } + /** Toggles whether the drawer is currently animating. */ + private _setIsAnimating(isAnimating: boolean) { + clearTimeout(this._fallbackTimer); + this._elementRef.nativeElement.classList.toggle('mat-drawer-animating', isAnimating); + + // Some internal integration tests disable animations by setting `* {transition: none}`. This + // will stop transition events from firing and prevent this class from being removed. Since + // it's somewhat load-bearing we need a fallback for such cases. + if (isAnimating) { + this._fallbackTimer = this._ngZone.runOutsideAngular(() => + setTimeout(() => this._setIsAnimating(false), 500), + ); + } + } + _getWidth(): number { - return this._elementRef.nativeElement ? this._elementRef.nativeElement.offsetWidth || 0 : 0; + return this._elementRef.nativeElement.offsetWidth || 0; } /** Updates the enabled state of the focus trap. */ @@ -647,6 +659,28 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy this._anchor.parentNode!.insertBefore(element, this._anchor); } } + + /** Event handler for animation events. */ + private _handleTransitionEvent = (event: TransitionEvent) => { + const element = this._elementRef.nativeElement; + console.log(event); + + if (event.target === element) { + this._ngZone.run(() => { + if (event.type === 'transitionrun') { + this._animationStarted.next(event); + } else { + // Don't toggle the animating state on `transitioncancel` since another animation should + // start afterwards. This prevents the drawer from blinking if an animation is interrupted. + if (event.type === 'transitionend') { + this._setIsAnimating(false); + } + + this._animationEnd.next(event); + } + }); + } + }; } /** @@ -680,6 +714,7 @@ export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy private _ngZone = inject(NgZone); private _changeDetectorRef = inject(ChangeDetectorRef); private _animationMode = inject(ANIMATION_MODULE_TYPE, {optional: true}); + _transitionsEnabled = false; /** All drawers in the container. Includes drawers from inside nested containers. */ @ContentChildren(MatDrawer, { @@ -777,6 +812,7 @@ export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy constructor(...args: unknown[]); constructor() { + const platform = inject(Platform); const viewportRuler = inject(ViewportRuler); // If a `Dir` directive exists up the tree, listen direction changes @@ -792,6 +828,17 @@ export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy .change() .pipe(takeUntil(this._destroyed)) .subscribe(() => this.updateContentMargins()); + + if (this._animationMode !== 'NoopAnimations' && platform.isBrowser) { + this._ngZone.runOutsideAngular(() => { + // Enable the animations after a delay in order to skip + // the initial transition if a drawer is open by default. + setTimeout(() => { + this._element.nativeElement.classList.add('mat-drawer-transition'); + this._transitionsEnabled = true; + }, 200); + }); + } } ngAfterContentInit() { @@ -915,21 +962,10 @@ export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy * is properly hidden. */ private _watchDrawerToggle(drawer: MatDrawer): void { - drawer._animationStarted - .pipe( - filter((event: AnimationEvent) => event.fromState !== event.toState), - takeUntil(this._drawers.changes), - ) - .subscribe((event: AnimationEvent) => { - // Set the transition class on the container so that the animations occur. This should not - // be set initially because animations should only be triggered via a change in state. - if (event.toState !== 'open-instant' && this._animationMode !== 'NoopAnimations') { - this._element.nativeElement.classList.add('mat-drawer-transition'); - } - - this.updateContentMargins(); - this._changeDetectorRef.markForCheck(); - }); + drawer._animationStarted.pipe(takeUntil(this._drawers.changes)).subscribe(() => { + this.updateContentMargins(); + this._changeDetectorRef.markForCheck(); + }); if (drawer.mode !== 'side') { drawer.openedChange diff --git a/src/material/sidenav/sidenav.ts b/src/material/sidenav/sidenav.ts index 3a877964d9c2..0dbfe6b6ada4 100644 --- a/src/material/sidenav/sidenav.ts +++ b/src/material/sidenav/sidenav.ts @@ -16,7 +16,6 @@ import { QueryList, } from '@angular/core'; import {MatDrawer, MatDrawerContainer, MatDrawerContent, MAT_DRAWER_CONTAINER} from './drawer'; -import {matDrawerAnimations} from './drawer-animations'; import { BooleanInput, coerceBooleanProperty, @@ -46,7 +45,6 @@ export class MatSidenavContent extends MatDrawerContent {} selector: 'mat-sidenav', exportAs: 'matSidenav', templateUrl: 'drawer.html', - animations: [matDrawerAnimations.transformDrawer], host: { 'class': 'mat-drawer mat-sidenav', 'tabIndex': '-1', @@ -56,7 +54,6 @@ export class MatSidenavContent extends MatDrawerContent {} '[class.mat-drawer-over]': 'mode === "over"', '[class.mat-drawer-push]': 'mode === "push"', '[class.mat-drawer-side]': 'mode === "side"', - '[class.mat-drawer-opened]': 'opened', '[class.mat-sidenav-fixed]': 'fixedInViewport', '[style.top.px]': 'fixedInViewport ? fixedTopGap : null', '[style.bottom.px]': 'fixedInViewport ? fixedBottomGap : null', diff --git a/tools/public_api_guard/material/sidenav.md b/tools/public_api_guard/material/sidenav.md index 8878fd1f9684..65cae7f04938 100644 --- a/tools/public_api_guard/material/sidenav.md +++ b/tools/public_api_guard/material/sidenav.md @@ -4,10 +4,8 @@ ```ts -import { AfterContentChecked } from '@angular/core'; import { AfterContentInit } from '@angular/core'; import { AfterViewInit } from '@angular/core'; -import { AnimationEvent as AnimationEvent_2 } from '@angular/animations'; import { AnimationTriggerMetadata } from '@angular/animations'; import { BooleanInput } from '@angular/cdk/coercion'; import { CdkScrollable } from '@angular/cdk/scrolling'; @@ -32,11 +30,10 @@ export const MAT_DRAWER_DEFAULT_AUTOSIZE: InjectionToken; export function MAT_DRAWER_DEFAULT_AUTOSIZE_FACTORY(): boolean; // @public -export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy { +export class MatDrawer implements AfterViewInit, OnDestroy { constructor(...args: unknown[]); - readonly _animationEnd: Subject; - readonly _animationStarted: Subject; - _animationState: 'open-instant' | 'open' | 'void'; + readonly _animationEnd: Subject; + readonly _animationStarted: Subject; get autoFocus(): AutoFocusTarget | string | boolean; set autoFocus(value: AutoFocusTarget | string | BooleanInput); close(): Promise; @@ -54,8 +51,6 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy set mode(value: MatDrawerMode); readonly _modeChanged: Subject; // (undocumented) - ngAfterContentChecked(): void; - // (undocumented) ngAfterViewInit(): void; // (undocumented) ngOnDestroy(): void; @@ -75,7 +70,7 @@ export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy static ɵfac: i0.ɵɵFactoryDeclaration; } -// @public +// @public @deprecated export const matDrawerAnimations: { readonly transformDrawer: AnimationTriggerMetadata; }; @@ -120,6 +115,8 @@ export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy open(): void; get scrollable(): CdkScrollable; get start(): MatDrawer | null; + // (undocumented) + _transitionsEnabled: boolean; updateContentMargins(): void; // (undocumented) _userContent: MatDrawerContent;