Skip to content

Commit

Permalink
fix(progress-spinner): not redrawing when changing modes
Browse files Browse the repository at this point in the history
* Fixes the progress spinner not redrawing when the mode is changed dynamically from `indeterminate` to `determinate`.
* Some minor cleanup in the progress spinner.

Fixes angular#3648.
  • Loading branch information
crisbeto committed Mar 18, 2017
1 parent 3ad6ff0 commit b89aa76
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 22 deletions.
8 changes: 6 additions & 2 deletions src/demo-app/progress-spinner/progress-spinner-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ <h1>Determinate</h1>
<p>Value: {{progressValue}}</p>
<button md-raised-button (click)="step(10)">Increase</button>
<button md-raised-button (click)="step(-10)">Decrease</button>
<md-checkbox [(ngModel)]="modeToggle">Is determinate</md-checkbox>
</div>

<div class="demo-progress-spinner">
<md-progress-spinner mode="determinate" [value]="progressValue" color="primary"></md-progress-spinner>
<md-progress-spinner [value]="progressValue" color="accent"></md-progress-spinner>
<md-progress-spinner [mode]="modeToggle ? 'indeterminate' : 'determinate'"
[value]="progressValue" color="primary"></md-progress-spinner>
<md-progress-spinner [mode]="modeToggle ? 'indeterminate' : 'determinate'"
[value]="progressValue" color="accent"></md-progress-spinner>
</div>

<h1>Indeterminate</h1>
Expand All @@ -24,3 +27,4 @@ <h1>Indeterminate</h1>
<md-progress-spinner mode="indeterminate" [color]="color"></md-progress-spinner>
<md-spinner [color]="color"></md-spinner>
</div>

3 changes: 2 additions & 1 deletion src/demo-app/progress-spinner/progress-spinner-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import {Component} from '@angular/core';
styleUrls: ['progress-spinner-demo.css'],
})
export class ProgressSpinnerDemo {
progressValue: number = 40;
progressValue: number = 60;
color: string = 'primary';
modeToggle: boolean = false;

step(val: number) {
this.progressValue = Math.max(0, Math.min(100, val + this.progressValue));
Expand Down
35 changes: 26 additions & 9 deletions src/lib/progress-spinner/progress-spinner.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ describe('MdProgressSpinner', () => {
it('should set the value to undefined when the mode is set to indeterminate', () => {
let fixture = TestBed.createComponent(ProgressSpinnerWithValueAndBoundMode);
let progressElement = fixture.debugElement.query(By.css('md-progress-spinner'));
fixture.debugElement.componentInstance.mode = 'determinate';
fixture.componentInstance.mode = 'determinate';
fixture.detectChanges();

expect(progressElement.componentInstance.value).toBe(50);
fixture.debugElement.componentInstance.mode = 'indeterminate';
fixture.componentInstance.mode = 'indeterminate';
fixture.detectChanges();
expect(progressElement.componentInstance.value).toBe(undefined);
});
Expand Down Expand Up @@ -89,7 +89,7 @@ describe('MdProgressSpinner', () => {
let progressElement = fixture.debugElement.query(By.css('md-progress-spinner'));
expect(progressElement.componentInstance.interdeterminateInterval).toBeTruthy();

fixture.debugElement.componentInstance.isHidden = true;
fixture.componentInstance.isHidden = true;
fixture.detectChanges();
expect(progressElement.componentInstance.interdeterminateInterval).toBeFalsy();
});
Expand All @@ -102,7 +102,7 @@ describe('MdProgressSpinner', () => {

expect(progressElement.componentInstance.interdeterminateInterval).toBeTruthy();

fixture.debugElement.componentInstance.isHidden = true;
fixture.componentInstance.isHidden = true;
fixture.detectChanges();

expect(progressElement.componentInstance.interdeterminateInterval).toBeFalsy();
Expand All @@ -116,7 +116,7 @@ describe('MdProgressSpinner', () => {

expect(progressElement.nativeElement.classList).toContain('mat-primary');

fixture.debugElement.componentInstance.color = 'accent';
fixture.componentInstance.color = 'accent';
fixture.detectChanges();

expect(progressElement.nativeElement.classList).toContain('mat-accent');
Expand All @@ -131,13 +131,30 @@ describe('MdProgressSpinner', () => {

expect(progressElement.nativeElement.classList).toContain('mat-primary');

fixture.debugElement.componentInstance.color = 'accent';
fixture.componentInstance.color = 'accent';
fixture.detectChanges();

expect(progressElement.nativeElement.classList).toContain('mat-accent');
expect(progressElement.nativeElement.classList).not.toContain('mat-primary');
});

it('should re-render the circle when switching from indeterminate to determinate mode', () => {
let fixture = TestBed.createComponent(ProgressSpinnerWithValueAndBoundMode);
let progressElement = fixture.debugElement.query(By.css('md-progress-spinner')).nativeElement;

fixture.componentInstance.mode = 'indeterminate';
fixture.detectChanges();

let path = progressElement.querySelector('path');
let oldDimesions = path.getAttribute('d');

fixture.componentInstance.mode = 'determinate';
fixture.detectChanges();

expect(path.getAttribute('d')).not
.toBe(oldDimesions, 'Expected circle dimensions to have changed.');
});

});


Expand All @@ -148,14 +165,14 @@ class BasicProgressSpinner { }
class IndeterminateProgressSpinner { }

@Component({template: '<md-progress-spinner value="50" [mode]="mode"></md-progress-spinner>'})
class ProgressSpinnerWithValueAndBoundMode { }
class ProgressSpinnerWithValueAndBoundMode { mode = 'indeterminate'; }

@Component({template: `
<md-progress-spinner mode="indeterminate" *ngIf="!isHidden"></md-progress-spinner>`})
class IndeterminateProgressSpinnerWithNgIf { }
class IndeterminateProgressSpinnerWithNgIf { isHidden = false; }

@Component({template: `<md-spinner *ngIf="!isHidden"></md-spinner>`})
class SpinnerWithNgIf { }
class SpinnerWithNgIf { isHidden = false; }

@Component({template: `<md-spinner [color]="color"></md-spinner>`})
class SpinnerWithColor { color: string = 'primary'; }
Expand Down
27 changes: 17 additions & 10 deletions src/lib/progress-spinner/progress-spinner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export class MdProgressSpinner implements OnDestroy {
set value(v: number) {
if (v != null && this.mode == 'determinate') {
let newValue = clamp(v);
this._animateCircle((this.value || 0), newValue, linearEase, DURATION_DETERMINATE, 0);
this._animateCircle(this.value || 0, newValue);
this._value = newValue;
}
}
Expand All @@ -150,13 +150,13 @@ export class MdProgressSpinner implements OnDestroy {
get mode() {
return this._mode;
}
set mode(m: ProgressSpinnerMode) {
if (m == 'indeterminate') {
set mode(mode: ProgressSpinnerMode) {
if (mode === 'indeterminate') {
this._startIndeterminateAnimation();
} else {
this._cleanupIndeterminateAnimation();
this._cleanupIndeterminateAnimation(true);
}
this._mode = m;
this._mode = mode;
}

constructor(
Expand All @@ -176,8 +176,8 @@ export class MdProgressSpinner implements OnDestroy {
* @param rotation The starting angle of the circle fill, with 0° represented at the top center
* of the circle.
*/
private _animateCircle(animateFrom: number, animateTo: number, ease: EasingFn,
duration: number, rotation: number) {
private _animateCircle(animateFrom: number, animateTo: number, ease: EasingFn = linearEase,
duration = DURATION_DETERMINATE, rotation = 0) {

let id = ++this._lastAnimationId;
let startTime = Date.now();
Expand Down Expand Up @@ -237,16 +237,23 @@ export class MdProgressSpinner implements OnDestroy {

/**
* Removes interval, ending the animation.
* @param redraw Whether to redraw the circle after the animation is cleared.
*/
private _cleanupIndeterminateAnimation() {
this.interdeterminateInterval = null;
private _cleanupIndeterminateAnimation(redraw = false) {
if (this.interdeterminateInterval) {
this.interdeterminateInterval = null;

if (redraw) {
this._animateCircle(0, this._value);
}
}
}

/**
* Renders the arc onto the SVG element. Proxies `getArc` while setting the proper
* DOM attribute on the `<path>`.
*/
private _renderArc(currentValue: number, rotation: number) {
private _renderArc(currentValue: number, rotation = 0) {
// Caches the path reference so it doesn't have to be looked up every time.
let path = this._path = this._path || this._elementRef.nativeElement.querySelector('path');

Expand Down

0 comments on commit b89aa76

Please sign in to comment.