From 289eecd253ecb77e0c1f6954a2debc302a7724b7 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 20 Feb 2020 20:08:55 +0100 Subject: [PATCH] fix(material-experimental/mdc-progress-bar): changed after checked error on animation end event with noop animations (#18508) Along the same lines as #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. --- .../mdc-progress-bar/progress-bar.scss | 8 +++++ .../mdc-progress-bar/progress-bar.spec.ts | 30 +--------------- .../mdc-progress-bar/progress-bar.ts | 34 +++++++------------ 3 files changed, 21 insertions(+), 51 deletions(-) diff --git a/src/material-experimental/mdc-progress-bar/progress-bar.scss b/src/material-experimental/mdc-progress-bar/progress-bar.scss index cc147d464f91..ba79e7866de4 100644 --- a/src/material-experimental/mdc-progress-bar/progress-bar.scss +++ b/src/material-experimental/mdc-progress-bar/progress-bar.scss @@ -7,6 +7,14 @@ // Explicitly set to `block` since the browser defaults custom elements to `inline`. display: block; + &._mat-animation-noopable { + .mdc-linear-progress__primary-bar { + // There's a `transitionend` event that depends on this element. Add a very short + // transition when animations are disabled so that the event can still fire. + transition: transform 1ms; + } + } + &:not(._mat-animation-noopable) { @include mdc-linear-progress-core-styles($query: animation); } diff --git a/src/material-experimental/mdc-progress-bar/progress-bar.spec.ts b/src/material-experimental/mdc-progress-bar/progress-bar.spec.ts index 5c934d8ccf3e..f406da0bc8c1 100644 --- a/src/material-experimental/mdc-progress-bar/progress-bar.spec.ts +++ b/src/material-experimental/mdc-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} from './index'; import {MatProgressBar} from './progress-bar'; @@ -213,33 +212,6 @@ describe('MDC-based 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('.mdc-linear-progress__primary-bar'))!; - })); - - 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-experimental/mdc-progress-bar/progress-bar.ts b/src/material-experimental/mdc-progress-bar/progress-bar.ts index 6bc576e39579..2de11a0eaff5 100644 --- a/src/material-experimental/mdc-progress-bar/progress-bar.ts +++ b/src/material-experimental/mdc-progress-bar/progress-bar.ts @@ -100,11 +100,6 @@ export class MatProgressBar extends _MatProgressBarMixinBase implements AfterVie set value(v: number) { this._value = clamp(v || 0); this._syncFoundation(); - - // When noop animation is set to true, trigger animationEnd directly. - if (this._isNoopAnimation) { - this._emitAnimationEnd(); - } } private _value = 0; @@ -162,16 +157,18 @@ export class MatProgressBar extends _MatProgressBarMixinBase implements AfterVie this._foundation.init(); this._syncFoundation(); - 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((() => { - this._animationEndSubscription = - (fromEvent(this._primaryBar, 'transitionend') as Observable) - .pipe(filter(((e: TransitionEvent) => e.target === this._primaryBar))) - .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((() => { + this._animationEndSubscription = + (fromEvent(this._primaryBar, 'transitionend') as Observable) + .pipe(filter(((e: TransitionEvent) => e.target === this._primaryBar))) + .subscribe(() => { + if (this.mode === 'determinate' || this.mode === 'buffer') { + this._ngZone.run(() => this.animationEnd.next({value: this.value})); + } + }); + })); } ngOnDestroy() { @@ -182,13 +179,6 @@ export class MatProgressBar extends _MatProgressBarMixinBase implements AfterVie this._dirChangeSubscription.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}); - } - } - /** Syncs the state of the progress bar with the MDC foundation. */ private _syncFoundation() { const foundation = this._foundation;