Skip to content

Commit

Permalink
fix(material-experimental/mdc-progress-bar): changed after checked er…
Browse files Browse the repository at this point in the history
…ror 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.
  • Loading branch information
crisbeto authored Feb 20, 2020
1 parent 5615979 commit 289eecd
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 51 deletions.
8 changes: 8 additions & 0 deletions src/material-experimental/mdc-progress-bar/progress-bar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
30 changes: 1 addition & 29 deletions src/material-experimental/mdc-progress-bar/progress-bar.spec.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -213,33 +212,6 @@ describe('MDC-based MatProgressBar', () => {
});
});

describe('With NoopAnimations', () => {
let progressComponent: MatProgressBar;
let primaryValueBar: DebugElement;
let fixture: ComponentFixture<BasicProgressBar>;

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: '<mat-progress-bar></mat-progress-bar>'})
Expand Down
34 changes: 12 additions & 22 deletions src/material-experimental/mdc-progress-bar/progress-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<TransitionEvent>)
.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<TransitionEvent>)
.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() {
Expand All @@ -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;
Expand Down

0 comments on commit 289eecd

Please sign in to comment.