From a2d02188e841f7495fd8580f88f3cbaf3f54cebc Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 15 Jan 2025 13:30:59 +0100 Subject: [PATCH] fix(material/stepper): switch away from animations module Reworks the stepper so it uses CSS directly to animate, instead of going through the animations module. This both simplifies the setup and allows us to avoid the issues that come with the animations module. --- src/cdk/stepper/stepper.ts | 2 +- src/material/stepper/stepper-animations.ts | 9 +- src/material/stepper/stepper.html | 55 ++++---- src/material/stepper/stepper.scss | 83 +++++++---- src/material/stepper/stepper.spec.ts | 11 +- src/material/stepper/stepper.ts | 153 ++++++++++++++++----- tools/public_api_guard/cdk/stepper.md | 2 + tools/public_api_guard/material/stepper.md | 15 +- 8 files changed, 229 insertions(+), 101 deletions(-) diff --git a/src/cdk/stepper/stepper.ts b/src/cdk/stepper/stepper.ts index 95f08f9942a3..7d588386fa93 100644 --- a/src/cdk/stepper/stepper.ts +++ b/src/cdk/stepper/stepper.ts @@ -255,7 +255,7 @@ export class CdkStep implements OnChanges { export class CdkStepper implements AfterContentInit, AfterViewInit, OnDestroy { private _dir = inject(Directionality, {optional: true}); private _changeDetectorRef = inject(ChangeDetectorRef); - private _elementRef = inject>(ElementRef); + protected _elementRef = inject>(ElementRef); /** Emits when the component is destroyed. */ protected readonly _destroyed = new Subject(); diff --git a/src/material/stepper/stepper-animations.ts b/src/material/stepper/stepper-animations.ts index e48166584716..39ac7c723ef5 100644 --- a/src/material/stepper/stepper-animations.ts +++ b/src/material/stepper/stepper-animations.ts @@ -17,12 +17,11 @@ import { animateChild, } from '@angular/animations'; -export const DEFAULT_HORIZONTAL_ANIMATION_DURATION = '500ms'; -export const DEFAULT_VERTICAL_ANIMATION_DURATION = '225ms'; - /** * Animations used by the Material steppers. * @docs-private + * @deprecated No longer used, will be removed. + * @breaking-change 21.0.0 */ export const matStepperAnimations: { readonly horizontalStepTransition: AnimationTriggerMetadata; @@ -43,7 +42,7 @@ export const matStepperAnimations: { query('@*', animateChild(), {optional: true}), ]), { - params: {'animationDuration': DEFAULT_HORIZONTAL_ANIMATION_DURATION}, + params: {'animationDuration': '500ms'}, }, ), ]), @@ -63,7 +62,7 @@ export const matStepperAnimations: { query('@*', animateChild(), {optional: true}), ]), { - params: {'animationDuration': DEFAULT_VERTICAL_ANIMATION_DURATION}, + params: {'animationDuration': '225ms'}, }, ), ]), diff --git a/src/material/stepper/stepper.html b/src/material/stepper/stepper.html index 083d7f1314ab..a75e48522600 100644 --- a/src/material/stepper/stepper.html +++ b/src/material/stepper/stepper.html @@ -12,28 +12,27 @@ @case ('horizontal') {
- @for (step of steps; track step; let i = $index, isLast = $last) { + @for (step of steps; track step) { - @if (!isLast) { + [ngTemplateOutletContext]="{step, i: $index}"/> + @if (!$last) {
} }
- @for (step of steps; track step; let i = $index) { -
- + @for (step of steps; track step) { +
+
}
@@ -41,23 +40,23 @@ } @case ('vertical') { - @for (step of steps; track step; let i = $index, isLast = $last) { + @for (step of steps; track step) {
-
-
+ [ngTemplateOutletContext]="{step, i: $index}"/> +
+
- +
@@ -91,5 +90,5 @@ [errorMessage]="step.errorMessage" [iconOverrides]="_iconOverrides" [disableRipple]="disableRipple || !_stepIsNavigable(i, step)" - [color]="step.color || color"> + [color]="step.color || color"/> diff --git a/src/material/stepper/stepper.scss b/src/material/stepper/stepper.scss index 20537d4d55c6..19ab2d88ea56 100644 --- a/src/material/stepper/stepper.scss +++ b/src/material/stepper/stepper.scss @@ -178,20 +178,34 @@ } .mat-horizontal-stepper-content { + visibility: hidden; + overflow: hidden; outline: 0; + height: 0; - &.mat-horizontal-stepper-content-inactive { - height: 0; - overflow: hidden; + .mat-stepper-animations-enabled & { + transition: transform var(--mat-stepper-animation-duration, 0) cubic-bezier(0.35, 0, 0.25, 1); + } + + &.mat-horizontal-stepper-content-previous { + transform: translate3d(-100%, 0, 0); + } + + &.mat-horizontal-stepper-content-next { + transform: translate3d(100%, 0, 0); } - // Used to avoid an issue where when the stepper is nested inside a component that - // changes the `visibility` as a part of an Angular animation, the stepper's content - // stays hidden (see #25925). The value has to be `!important` to override the incorrect - // `visibility` from the animations package. This can also be solved using `visibility: visible` - // on `.mat-horizontal-stepper-content`, but it can allow tabbing into hidden content. - &:not(.mat-horizontal-stepper-content-inactive) { - visibility: inherit !important; + &.mat-horizontal-stepper-content-current { + // TODO(crisbeto): the height and visibility switches are a bit jarring, but that's how the + // animation was set up when we still used the Animations module. We should be able to make + // it a bit smoother. + visibility: visible; + transform: none; + height: auto; + } + + .mat-stepper-horizontal:not(.mat-stepper-animating) &.mat-horizontal-stepper-content-current { + overflow: visible; } } @@ -209,10 +223,26 @@ } .mat-vertical-content-container { + display: grid; + grid-template-rows: 0fr; + grid-template-columns: 100%; margin-left: stepper-variables.$vertical-stepper-content-margin; border: 0; position: relative; + .mat-stepper-animations-enabled & { + transition: grid-template-rows var(--mat-stepper-animation-duration, 0) + cubic-bezier(0.4, 0, 0.2, 1); + } + + &.mat-vertical-content-container-active { + grid-template-rows: 1fr; + } + + .mat-step:last-child & { + border: none; + } + @include cdk.high-contrast { outline: solid 1px; } @@ -221,6 +251,19 @@ margin-left: 0; margin-right: stepper-variables.$vertical-stepper-content-margin; } + + + // All the browsers we support have support for `grid` as well, but given that these styles are + // load-bearing for the stepper, we have a fallback to height which doesn't animate, just in case. + // stylelint-disable material/no-prefixes + @supports not (grid-template-rows: 0fr) { + height: 0; + + &.mat-vertical-content-container-active { + height: auto; + } + } + // stylelint-enable material/no-prefixes } .mat-stepper-vertical-line::before { @@ -252,23 +295,17 @@ .mat-vertical-stepper-content { overflow: hidden; outline: 0; + visibility: hidden; + + .mat-stepper-animations-enabled & { + transition: visibility var(--mat-stepper-animation-duration, 0) linear; + } - // Used to avoid an issue where when the stepper is nested inside a component that - // changes the `visibility` as a part of an Angular animation, the stepper's content - // stays hidden (see #25925). The value has to be `!important` to override the incorrect - // `visibility` from the animations package. This can also be solved using `visibility: visible` - // on `.mat-vertical-stepper-content`, but it can allow tabbing into hidden content. - &:not(.mat-vertical-stepper-content-inactive) { - visibility: inherit !important; + .mat-vertical-content-container-active > & { + visibility: visible; } } .mat-vertical-content { padding: 0 stepper-variables.$side-gap stepper-variables.$side-gap stepper-variables.$side-gap; } - -.mat-step:last-child { - .mat-vertical-content-container { - border: none; - } -} diff --git a/src/material/stepper/stepper.spec.ts b/src/material/stepper/stepper.spec.ts index ea1d4a38050e..feb4c497012c 100644 --- a/src/material/stepper/stepper.spec.ts +++ b/src/material/stepper/stepper.spec.ts @@ -34,7 +34,7 @@ import { inject, signal, } from '@angular/core'; -import {ComponentFixture, TestBed, fakeAsync, flush} from '@angular/core/testing'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; import { AbstractControl, AsyncValidatorFn, @@ -364,7 +364,7 @@ describe('MatStepper', () => { expect(stepperComponent._getIndicatorType(0)).toBe('done'); }); - it('should emit an event when the enter animation is done', fakeAsync(() => { + it('should emit an event when the enter animation is done', () => { const stepper = fixture.debugElement.query(By.directive(MatStepper))!.componentInstance; const selectionChangeSpy = jasmine.createSpy('selectionChange spy'); const animationDoneSpy = jasmine.createSpy('animationDone spy'); @@ -374,17 +374,12 @@ describe('MatStepper', () => { stepper.selectedIndex = 1; fixture.detectChanges(); - expect(selectionChangeSpy).toHaveBeenCalledTimes(1); - expect(animationDoneSpy).not.toHaveBeenCalled(); - - flush(); - expect(selectionChangeSpy).toHaveBeenCalledTimes(1); expect(animationDoneSpy).toHaveBeenCalledTimes(1); selectionChangeSubscription.unsubscribe(); animationDoneSubscription.unsubscribe(); - })); + }); it('should set the correct aria-posinset and aria-setsize', () => { const headers = Array.from( diff --git a/src/material/stepper/stepper.ts b/src/material/stepper/stepper.ts index 57f01ecbbe40..a320d3a2ad86 100644 --- a/src/material/stepper/stepper.ts +++ b/src/material/stepper/stepper.ts @@ -6,10 +6,11 @@ * found in the LICENSE file at https://angular.dev/license */ -import {CdkStep, CdkStepper, StepContentPositionState} from '@angular/cdk/stepper'; -import {AnimationEvent} from '@angular/animations'; +import {CdkStep, CdkStepper} from '@angular/cdk/stepper'; import { AfterContentInit, + AfterViewInit, + ANIMATION_MODULE_TYPE, ChangeDetectionStrategy, Component, ContentChild, @@ -18,31 +19,29 @@ import { EventEmitter, inject, Input, + NgZone, OnDestroy, Output, QueryList, + Renderer2, + signal, TemplateRef, ViewChildren, ViewContainerRef, ViewEncapsulation, } from '@angular/core'; +import {NgTemplateOutlet} from '@angular/common'; import {AbstractControl, FormGroupDirective, NgForm} from '@angular/forms'; import {ErrorStateMatcher, ThemePalette} from '@angular/material/core'; +import {Platform} from '@angular/cdk/platform'; import {CdkPortalOutlet, TemplatePortal} from '@angular/cdk/portal'; -import {Subject, Subscription} from 'rxjs'; +import {Subscription} from 'rxjs'; import {takeUntil, map, startWith, switchMap} from 'rxjs/operators'; import {MatStepHeader} from './step-header'; import {MatStepLabel} from './step-label'; -import { - DEFAULT_HORIZONTAL_ANIMATION_DURATION, - DEFAULT_VERTICAL_ANIMATION_DURATION, - matStepperAnimations, -} from './stepper-animations'; import {MatStepperIcon, MatStepperIconContext} from './stepper-icon'; import {MatStepContent} from './step-content'; -import {NgTemplateOutlet} from '@angular/common'; -import {Platform} from '@angular/cdk/platform'; @Component({ selector: 'mat-step', @@ -130,28 +129,31 @@ export class MatStep extends CdkStep implements ErrorStateMatcher, AfterContentI '[class.mat-stepper-label-position-bottom]': 'orientation === "horizontal" && labelPosition == "bottom"', '[class.mat-stepper-header-position-bottom]': 'headerPosition === "bottom"', + '[class.mat-stepper-animating]': '_isAnimating()', + '[style.--mat-stepper-animation-duration]': '_getAnimationDuration()', '[attr.aria-orientation]': 'orientation', 'role': 'tablist', }, - animations: [ - matStepperAnimations.horizontalStepTransition, - matStepperAnimations.verticalStepTransition, - ], providers: [{provide: CdkStepper, useExisting: MatStepper}], encapsulation: ViewEncapsulation.None, changeDetection: ChangeDetectionStrategy.OnPush, imports: [NgTemplateOutlet, MatStepHeader], }) -export class MatStepper extends CdkStepper implements AfterContentInit { +export class MatStepper extends CdkStepper implements AfterViewInit, AfterContentInit, OnDestroy { + private _ngZone = inject(NgZone); + private _renderer = inject(Renderer2); + private _animationsModule = inject(ANIMATION_MODULE_TYPE, {optional: true}); + private _cleanupTransition: (() => void) | undefined; + protected _isAnimating = signal(false); + /** The list of step headers of the steps in the stepper. */ - // We need an initializer here to avoid a TS error. - @ViewChildren(MatStepHeader) override _stepHeader: QueryList = - undefined as unknown as QueryList; + @ViewChildren(MatStepHeader) override _stepHeader: QueryList = undefined!; + + /** Elements hosting the step animations. */ + @ViewChildren('animatedContainer') _animatedContainers: QueryList; /** Full list of steps inside the stepper, including inside nested steppers. */ - // We need an initializer here to avoid a TS error. - @ContentChildren(MatStep, {descendants: true}) override _steps: QueryList = - undefined as unknown as QueryList; + @ContentChildren(MatStep, {descendants: true}) override _steps: QueryList = undefined!; /** Steps that belong to the current stepper, excluding ones from nested steppers. */ override readonly steps: QueryList = new QueryList(); @@ -191,9 +193,6 @@ export class MatStepper extends CdkStepper implements AfterContentInit { /** Consumer-specified template-refs to be used to override the header icons. */ _iconOverrides: Record> = {}; - /** Stream of animation `done` events when the body expands/collapses. */ - readonly _animationDone = new Subject(); - /** Duration for the animation. Will be normalized to milliseconds if no units are set. */ @Input() get animationDuration(): string { @@ -222,28 +221,118 @@ export class MatStepper extends CdkStepper implements AfterContentInit { this._icons.forEach(({name, templateRef}) => (this._iconOverrides[name] = templateRef)); // Mark the component for change detection whenever the content children query changes - this.steps.changes.pipe(takeUntil(this._destroyed)).subscribe(() => { - this._stateChanged(); + this.steps.changes.pipe(takeUntil(this._destroyed)).subscribe(() => this._stateChanged()); + + // Transition events won't fire if animations are disabled so we simulate them. + this.selectedIndexChange.pipe(takeUntil(this._destroyed)).subscribe(() => { + const duration = this._getAnimationDuration(); + if (duration === '0ms' || duration === '0s') { + this._onAnimationDone(); + } else { + this._isAnimating.set(true); + } }); - this._animationDone.pipe(takeUntil(this._destroyed)).subscribe(event => { - if ((event.toState as StepContentPositionState) === 'current') { - this.animationDone.emit(); + this._ngZone.runOutsideAngular(() => { + if (this._animationsModule !== 'NoopAnimations') { + setTimeout(() => { + // Delay enabling the animations so we don't animate the initial state. + this._elementRef.nativeElement.classList.add('mat-stepper-animations-enabled'); + + // Bind this outside the zone since it fires for all transitions inside the stepper. + this._cleanupTransition = this._renderer.listen( + this._elementRef.nativeElement, + 'transitionend', + this._handleTransitionend, + ); + }, 200); } }); } + override ngAfterViewInit(): void { + super.ngAfterViewInit(); + + // Prior to #30314 the stepper had animation `done` events bound to each animated container. + // The animations module was firing them on initialization and for each subsequent animation. + // Since the events were bound in the template, it had the unintended side-effect of triggering + // change detection as well. It appears that this side-effect ended up being load-bearing, + // because it was ensuring that the content elements (e.g. `matStepLabel`) that are defined + // in sub-components actually get picked up in a timely fashion. This subscription simulates + // the same change detection by using `queueMicrotask` similarly to the animations module. + if (typeof queueMicrotask === 'function') { + let hasEmittedInitial = false; + this._animatedContainers.changes + .pipe(startWith(null), takeUntil(this._destroyed)) + .subscribe(() => + queueMicrotask(() => { + // Simulate the initial `animationDone` event + // that gets emitted by the animations module. + if (!hasEmittedInitial) { + hasEmittedInitial = true; + this.animationDone.emit(); + } + + this._stateChanged(); + }), + ); + } + } + + override ngOnDestroy(): void { + super.ngOnDestroy(); + this._cleanupTransition?.(); + } + _stepIsNavigable(index: number, step: MatStep): boolean { return step.completed || this.selectedIndex === index || !this.linear; } _getAnimationDuration() { + if (this._animationsModule === 'NoopAnimations') { + return '0ms'; + } + if (this.animationDuration) { return this.animationDuration; } - return this.orientation === 'horizontal' - ? DEFAULT_HORIZONTAL_ANIMATION_DURATION - : DEFAULT_VERTICAL_ANIMATION_DURATION; + return this.orientation === 'horizontal' ? '500ms' : '225ms'; + } + + private _handleTransitionend = (event: TransitionEvent) => { + const target = event.target as HTMLElement | null; + + if (!target) { + return; + } + + // Because we bind a single `transitionend` handler on the host node and because transition + // events bubble, we have to filter down to only the active step so don't emit events too + // often. We check the orientation and `property` name first to reduce the amount of times + // we need to check the DOM. + const isHorizontalActiveElement = + this.orientation === 'horizontal' && + event.propertyName === 'transform' && + target.classList.contains('mat-horizontal-stepper-content-current'); + const isVerticalActiveElement = + this.orientation === 'vertical' && + event.propertyName === 'grid-template-rows' && + target.classList.contains('mat-vertical-content-container-active'); + + // Finally we need to ensure that the animated element is a direct descendant, + // rather than one coming from a nested stepper. + const shouldEmit = + (isHorizontalActiveElement || isVerticalActiveElement) && + this._animatedContainers.find(ref => ref.nativeElement === target); + + if (shouldEmit) { + this._onAnimationDone(); + } + }; + + private _onAnimationDone() { + this._isAnimating.set(false); + this.animationDone.emit(); } } diff --git a/tools/public_api_guard/cdk/stepper.md b/tools/public_api_guard/cdk/stepper.md index 5279141041e4..80a9ffe0cf7a 100644 --- a/tools/public_api_guard/cdk/stepper.md +++ b/tools/public_api_guard/cdk/stepper.md @@ -95,6 +95,8 @@ export class CdkStepLabel { export class CdkStepper implements AfterContentInit, AfterViewInit, OnDestroy { constructor(...args: unknown[]); protected readonly _destroyed: Subject; + // (undocumented) + protected _elementRef: ElementRef; _getAnimationDirection(index: number): StepContentPositionState; _getFocusIndex(): number | null; _getIndicatorType(index: number, state?: StepState): StepState; diff --git a/tools/public_api_guard/material/stepper.md b/tools/public_api_guard/material/stepper.md index 00c63518d5ed..0dbb7e0481e8 100644 --- a/tools/public_api_guard/material/stepper.md +++ b/tools/public_api_guard/material/stepper.md @@ -7,7 +7,6 @@ import { AbstractControl } from '@angular/forms'; import { AfterContentInit } from '@angular/core'; import { AfterViewInit } from '@angular/core'; -import { AnimationEvent as AnimationEvent_2 } from '@angular/animations'; import { AnimationTriggerMetadata } from '@angular/animations'; import { CdkStep } from '@angular/cdk/stepper'; import { CdkStepHeader } from '@angular/cdk/stepper'; @@ -15,6 +14,7 @@ import { CdkStepLabel } from '@angular/cdk/stepper'; import { CdkStepper } from '@angular/cdk/stepper'; import { CdkStepperNext } from '@angular/cdk/stepper'; import { CdkStepperPrevious } from '@angular/cdk/stepper'; +import { ElementRef } from '@angular/core'; import { ErrorStateMatcher } from '@angular/material/core'; import { EventEmitter } from '@angular/core'; import { FocusOrigin } from '@angular/cdk/a11y'; @@ -34,6 +34,7 @@ import { Subject } from 'rxjs'; import { TemplatePortal } from '@angular/cdk/portal'; import { TemplateRef } from '@angular/core'; import { ThemePalette } from '@angular/material/core'; +import { WritableSignal } from '@angular/core'; // @public export const MAT_STEPPER_INTL_PROVIDER: { @@ -116,10 +117,10 @@ export class MatStepLabel extends CdkStepLabel { } // @public (undocumented) -export class MatStepper extends CdkStepper implements AfterContentInit { +export class MatStepper extends CdkStepper implements AfterViewInit, AfterContentInit, OnDestroy { constructor(...args: unknown[]); + _animatedContainers: QueryList; readonly animationDone: EventEmitter; - readonly _animationDone: Subject; get animationDuration(): string; set animationDuration(value: string); color: ThemePalette; @@ -129,10 +130,16 @@ export class MatStepper extends CdkStepper implements AfterContentInit { headerPosition: 'top' | 'bottom'; _iconOverrides: Record>; _icons: QueryList; + // (undocumented) + protected _isAnimating: WritableSignal; protected _isServer: boolean; labelPosition: 'bottom' | 'end'; // (undocumented) ngAfterContentInit(): void; + // (undocumented) + ngAfterViewInit(): void; + // (undocumented) + ngOnDestroy(): void; _stepHeader: QueryList; // (undocumented) _stepIsNavigable(index: number, step: MatStep): boolean; @@ -144,7 +151,7 @@ export class MatStepper extends CdkStepper implements AfterContentInit { static ɵfac: i0.ɵɵFactoryDeclaration; } -// @public +// @public @deprecated export const matStepperAnimations: { readonly horizontalStepTransition: AnimationTriggerMetadata; readonly verticalStepTransition: AnimationTriggerMetadata;