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 May 13, 2019
1 parent 3712b8f commit 1370735
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 43 deletions.
4 changes: 1 addition & 3 deletions src/material/progress-spinner/progress-spinner-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,17 @@
import {NgModule} from '@angular/core';
import {CommonModule} from '@angular/common';
import {MatCommonModule} from '@angular/material/core';
import {MatProgressSpinner, MatSpinner} from './progress-spinner';
import {MatProgressSpinner} from './progress-spinner';


@NgModule({
imports: [MatCommonModule, CommonModule],
exports: [
MatProgressSpinner,
MatSpinner,
MatCommonModule
],
declarations: [
MatProgressSpinner,
MatSpinner
],
})
class MatProgressSpinnerModule {}
Expand Down
13 changes: 13 additions & 0 deletions src/material/progress-spinner/progress-spinner.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ describe('MatProgressSpinner', () => {
ProgressSpinnerCustomDiameter,
SpinnerWithColor,
ProgressSpinnerWithStringValues,
SpinnerWithMode,
],
}).compileComponents();
}));
Expand Down Expand Up @@ -321,6 +322,14 @@ describe('MatProgressSpinner', () => {
expect(progressElement.nativeElement.hasAttribute('aria-valuenow')).toBe(false);
});

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 @@ -360,3 +369,7 @@ class ProgressSpinnerWithColor { color: string = 'primary'; }
`
})
class ProgressSpinnerWithStringValues { }

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

44 changes: 7 additions & 37 deletions src/material/progress-spinner/progress-spinner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,12 @@ const INDETERMINATE_ANIMATION_TEMPLATE = `
*/
@Component({
moduleId: module.id,
selector: 'mat-progress-spinner',
selector: 'mat-progress-spinner, mat-spinner',
exportAs: 'matProgressSpinner',
host: {
'role': 'progressbar',
'class': 'mat-progress-spinner',
// `mat-spinner` is here for backward compatibility.
'class': 'mat-progress-spinner mat-spinner',
'[class._mat-animation-noopable]': `_noopAnimations`,
'[style.width.px]': 'diameter',
'[style.height.px]': 'diameter',
Expand Down Expand Up @@ -186,6 +187,10 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
this._noopAnimations = animationMode === 'NoopAnimations' &&
(!!defaults && !defaults._forceAnimations);

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

if (defaults) {
if (defaults.diameter) {
this.diameter = defaults.diameter;
Expand Down Expand Up @@ -265,38 +270,3 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
.replace(/DIAMETER/g, `${this.diameter}`);
}
}


/**
* `<mat-spinner>` component.
*
* This is a component definition to be used as a convenience reference to create an
* indeterminate `<mat-progress-spinner>` instance.
*/
@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<HTMLElement>, platform: Platform,
@Optional() @Inject(DOCUMENT) document: any,
@Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode: string,
@Inject(MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS)
defaults?: MatProgressSpinnerDefaultOptions) {
super(elementRef, platform, document, animationMode, defaults);
this.mode = 'indeterminate';
}
}
9 changes: 9 additions & 0 deletions src/material/progress-spinner/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@
* found in the LICENSE file at https://angular.io/license
*/

import {MatProgressSpinner} from './progress-spinner';

export * from './progress-spinner-module';
export * from './progress-spinner';

/**
* @deprecated Import `MatProgressSpinner` instead. Note that the
* `mat-spinner` selector isn't deprecated.
* @breaking-change 8.0.0
*/
// tslint:disable-next-line:variable-name
export const MatSpinner = MatProgressSpinner;
4 changes: 1 addition & 3 deletions tools/public_api_guard/material/progress-spinner.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ export interface MatProgressSpinnerDefaultOptions {
strokeWidth?: number;
}

export declare class MatSpinner extends MatProgressSpinner {
constructor(elementRef: ElementRef<HTMLElement>, platform: Platform, document: any, animationMode: string, defaults?: MatProgressSpinnerDefaultOptions);
}
export declare const MatSpinner: typeof MatProgressSpinner;

export declare type ProgressSpinnerMode = 'determinate' | 'indeterminate';

0 comments on commit 1370735

Please sign in to comment.