From 004eeb82185b170fcea78773ee29baf36fdab6ed Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 20 Feb 2020 19:55:36 +0100 Subject: [PATCH] fix(progress-bar): changed after checked error on animation end event with noop animations (#18441) If the `animationEnd` of a progress bar is used to update something in the view of an app that has disabled animations, a "changed after checked" error will be thrown because of the timing at which we dispatch the event. These changes work around the issue by not having separate logic for when the animations are enabled or disabled. --- src/material/progress-bar/progress-bar.scss | 5 ++- .../progress-bar/progress-bar.spec.ts | 30 +-------------- src/material/progress-bar/progress-bar.ts | 38 +++++++------------ 3 files changed, 19 insertions(+), 54 deletions(-) diff --git a/src/material/progress-bar/progress-bar.scss b/src/material/progress-bar/progress-bar.scss index 0e3670aedaeb..323b455a740c 100644 --- a/src/material/progress-bar/progress-bar.scss +++ b/src/material/progress-bar/progress-bar.scss @@ -147,7 +147,10 @@ $mat-progress-bar-piece-animation-duration: 250ms !default; .mat-progress-bar-secondary.mat-progress-bar-fill::after, .mat-progress-bar-background { animation: none; - transition: none; + + // Use a 1ms transition, because we have an event that + // is dispatched based on a `transitionend` being fired. + transition-duration: 1ms; } } } diff --git a/src/material/progress-bar/progress-bar.spec.ts b/src/material/progress-bar/progress-bar.spec.ts index 482c2060d240..b465201c3e0f 100644 --- a/src/material/progress-bar/progress-bar.spec.ts +++ b/src/material/progress-bar/progress-bar.spec.ts @@ -1,8 +1,7 @@ -import {TestBed, async, ComponentFixture} from '@angular/core/testing'; +import {TestBed, ComponentFixture} from '@angular/core/testing'; import {Component, DebugElement, Type} from '@angular/core'; import {By} from '@angular/platform-browser'; import {dispatchFakeEvent} from '@angular/cdk/testing/private'; -import {NoopAnimationsModule} from '@angular/platform-browser/animations'; import {MatProgressBarModule, MAT_PROGRESS_BAR_LOCATION} from './index'; import {MatProgressBar} from './progress-bar'; @@ -261,33 +260,6 @@ describe('MatProgressBar', () => { }); }); - describe('With NoopAnimations', () => { - let progressComponent: MatProgressBar; - let primaryValueBar: DebugElement; - let fixture: ComponentFixture; - - beforeEach(async(() => { - fixture = createComponent(BasicProgressBar, [MatProgressBarModule, NoopAnimationsModule]); - const progressElement = fixture.debugElement.query(By.css('mat-progress-bar'))!; - progressComponent = progressElement.componentInstance; - primaryValueBar = progressElement.query(By.css('.mat-progress-bar-primary'))!; - })); - - it('should not bind transition end listener', () => { - spyOn(primaryValueBar.nativeElement, 'addEventListener'); - fixture.detectChanges(); - - expect(primaryValueBar.nativeElement.addEventListener).not.toHaveBeenCalled(); - }); - - it('should trigger the animationEnd output on value set', () => { - fixture.detectChanges(); - spyOn(progressComponent.animationEnd, 'next'); - - progressComponent.value = 40; - expect(progressComponent.animationEnd.next).toHaveBeenCalledWith({ value: 40 }); - }); - }); }); @Component({template: ''}) diff --git a/src/material/progress-bar/progress-bar.ts b/src/material/progress-bar/progress-bar.ts index ecb3780510e8..1e60694f50b4 100644 --- a/src/material/progress-bar/progress-bar.ts +++ b/src/material/progress-bar/progress-bar.ts @@ -134,11 +134,6 @@ export class MatProgressBar extends _MatProgressBarMixinBase implements CanColor get value(): number { return this._value; } set value(v: number) { this._value = clamp(coerceNumberProperty(v) || 0); - - // When noop animation is set to true, trigger animationEnd directly. - if (this._isNoopAnimation) { - this._emitAnimationEnd(); - } } private _value: number = 0; @@ -194,31 +189,26 @@ export class MatProgressBar extends _MatProgressBarMixinBase implements CanColor } ngAfterViewInit() { - if (!this._isNoopAnimation) { - // Run outside angular so change detection didn't get triggered on every transition end - // instead only on the animation that we care about (primary value bar's transitionend) - this._ngZone.runOutsideAngular((() => { - const element = this._primaryValueBar.nativeElement; - - this._animationEndSubscription = - (fromEvent(element, 'transitionend') as Observable) - .pipe(filter(((e: TransitionEvent) => e.target === element))) - .subscribe(() => this._ngZone.run(() => this._emitAnimationEnd())); - })); - } + // Run outside angular so change detection didn't get triggered on every transition end + // instead only on the animation that we care about (primary value bar's transitionend) + this._ngZone.runOutsideAngular((() => { + const element = this._primaryValueBar.nativeElement; + + this._animationEndSubscription = + (fromEvent(element, 'transitionend') as Observable) + .pipe(filter(((e: TransitionEvent) => e.target === element))) + .subscribe(() => { + if (this.mode === 'determinate' || this.mode === 'buffer') { + this._ngZone.run(() => this.animationEnd.next({value: this.value})); + } + }); + })); } ngOnDestroy() { this._animationEndSubscription.unsubscribe(); } - /** Emit an animationEnd event if in determinate or buffer mode. */ - private _emitAnimationEnd(): void { - if (this.mode === 'determinate' || this.mode === 'buffer') { - this.animationEnd.next({value: this.value}); - } - } - static ngAcceptInputType_value: NumberInput; }