Skip to content

Commit

Permalink
fix(progress-spinner): unable to change mode on spinner directive
Browse files Browse the repository at this point in the history
Currently we have the `mat-spinner` directive which is a shortcut to a `mat-progress-spinner` with `mode="indeterminate"`. Since the spinner inherits all of the inputs from the progress spinner, there's nothing stoping people from changing the mode back to `determinate`, however the element will look half-broken because the host bindings assume that the mode won't change. These changes update the host bindings to allow switching between modes.

Fixes #14511.
  • Loading branch information
crisbeto committed Dec 19, 2018
1 parent ecaec18 commit 908df0e
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 35 deletions.
15 changes: 14 additions & 1 deletion src/lib/progress-spinner/progress-spinner.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from './index';


describe('MatProgressSpinner', () => {
fdescribe('MatProgressSpinner', () => {

beforeEach(async(() => {
TestBed.configureTestingModule({
Expand All @@ -22,6 +22,7 @@ describe('MatProgressSpinner', () => {
ProgressSpinnerCustomDiameter,
SpinnerWithColor,
ProgressSpinnerWithStringValues,
SpinnerWithMode,
],
}).compileComponents();
}));
Expand Down Expand Up @@ -296,6 +297,14 @@ describe('MatProgressSpinner', () => {
expect(progressElement.componentInstance.strokeWidth).toBe(7);
});

it('should be able to change the mode on a mat-spinner', () => {
const fixture = TestBed.createComponent(SpinnerWithMode);
fixture.detectChanges();

const progressElement = fixture.debugElement.query(By.css('mat-spinner')).nativeElement;
expect(progressElement.getAttribute('mode')).toBe('determinate');
});

});


Expand Down Expand Up @@ -335,3 +344,7 @@ class ProgressSpinnerWithColor { color: string = 'primary'; }
`
})
class ProgressSpinnerWithStringValues { }

@Component({template: '<mat-spinner mode="determinate"></mat-spinner>'})
class SpinnerWithMode { }

44 changes: 10 additions & 34 deletions src/lib/progress-spinner/progress-spinner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ const INDETERMINATE_ANIMATION_TEMPLATE = `
*/
@Component({
moduleId: module.id,
selector: 'mat-progress-spinner',
selector: 'mat-progress-spinner, mat-spinner',
exportAs: 'matProgressSpinner',
host: {
'role': 'progressbar',
Expand Down Expand Up @@ -186,6 +186,10 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
super(_elementRef);
this._fallbackAnimation = platform.EDGE || platform.TRIDENT;

if (_elementRef.nativeElement.nodeName.toLowerCase() === 'mat-spinner') {
this.mode = 'indeterminate';
}

if (defaults) {
if (defaults.diameter) {
this.diameter = defaults.diameter;
Expand Down Expand Up @@ -266,38 +270,10 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
}
}


/**
* `<mat-spinner>` component.
*
* This is a component definition to be used as a convenience reference to create an
* indeterminate `<mat-progress-spinner>` instance.
* @deprecated Import `MatProgressSpinner` instead. Note that the
* `mat-spinner` selector isn't deprecated.
* @breaking-change 8.0.0
*/
@Component({
moduleId: module.id,
selector: 'mat-spinner',
host: {
'role': 'progressbar',
'mode': 'indeterminate',
'class': 'mat-spinner mat-progress-spinner',
'[class._mat-animation-noopable]': `_noopAnimations`,
'[style.width.px]': 'diameter',
'[style.height.px]': 'diameter',
},
inputs: ['color'],
templateUrl: 'progress-spinner.html',
styleUrls: ['progress-spinner.css'],
changeDetection: ChangeDetectionStrategy.OnPush,
encapsulation: ViewEncapsulation.None,
})
export class MatSpinner extends MatProgressSpinner {
constructor(elementRef: ElementRef, platform: Platform,
@Optional() @Inject(DOCUMENT) document: any,
// @breaking-change 8.0.0 animationMode and defaults parameters to be made required.
@Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: string,
@Inject(MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS)
defaults?: MatProgressSpinnerDefaultOptions) {
super(elementRef, platform, document, animationMode, defaults);
this.mode = 'indeterminate';
}
}
// tslint:disable-next-line:variable-name
export const MatSpinner = MatProgressSpinner;

0 comments on commit 908df0e

Please sign in to comment.