Skip to content

Commit

Permalink
fix(material/progress-spinner): unable to change mode on spinner dire…
Browse files Browse the repository at this point in the history
…ctive

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 Feb 24, 2022
1 parent 469b790 commit 8eb98ca
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('MDC-based MatProgressSpinner', () => {
ProgressSpinnerWithStringValues,
IndeterminateSpinnerInShadowDom,
IndeterminateSpinnerInShadowDomWithNgIf,
SpinnerWithMode,
],
}).compileComponents();
}),
Expand Down Expand Up @@ -397,6 +398,14 @@ describe('MDC-based MatProgressSpinner', () => {
expect(children.length).toBeGreaterThan(0);
expect(children.every(child => child.getAttribute('aria-hidden') === 'true')).toBe(true);
});

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');
});
});

@Component({template: '<mat-progress-spinner></mat-progress-spinner>'})
Expand Down Expand Up @@ -470,3 +479,6 @@ class IndeterminateSpinnerInShadowDomWithNgIf {

diameter: number;
}

@Component({template: '<mat-spinner mode="determinate"></mat-spinner>'})
class SpinnerWithMode {}
6 changes: 3 additions & 3 deletions src/material/progress-spinner/progress-spinner-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
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],
exports: [MatProgressSpinner, MatCommonModule],
declarations: [MatProgressSpinner],
})
export class MatProgressSpinnerModule {}
12 changes: 12 additions & 0 deletions src/material/progress-spinner/progress-spinner.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('MatProgressSpinner', () => {
ProgressSpinnerWithStringValues,
IndeterminateSpinnerInShadowDom,
IndeterminateSpinnerInShadowDomWithNgIf,
SpinnerWithMode,
],
}).compileComponents();
}),
Expand Down Expand Up @@ -540,6 +541,14 @@ describe('MatProgressSpinner', () => {
expect(children.length).toBeGreaterThan(0);
expect(children.every(child => child.getAttribute('aria-hidden') === 'true')).toBe(true);
});

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');
});
});

@Component({template: '<mat-progress-spinner></mat-progress-spinner>'})
Expand Down Expand Up @@ -616,3 +625,6 @@ class IndeterminateSpinnerInShadowDomWithNgIf {

diameter: number;
}

@Component({template: '<mat-spinner mode="determinate"></mat-spinner>'})
class SpinnerWithMode {}
62 changes: 7 additions & 55 deletions src/material/progress-spinner/progress-spinner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,12 @@ const INDETERMINATE_ANIMATION_TEMPLATE = `
* `<mat-progress-spinner>` component.
*/
@Component({
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',
// set tab index to -1 so screen readers will read the aria-label
// Note: there is a known issue with JAWS that does not read progressbar aria labels on FireFox
'tabindex': '-1',
Expand Down Expand Up @@ -229,6 +230,10 @@ export class MatProgressSpinner
this._noopAnimations =
animationMode === 'NoopAnimations' && !!defaults && !defaults._forceAnimations;

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

if (defaults) {
if (defaults.color) {
this.color = this.defaultColor = defaults.color;
Expand Down Expand Up @@ -356,56 +361,3 @@ export class MatProgressSpinner
return this.diameter.toString().replace('.', '_');
}
}

/**
* `<mat-spinner>` component.
*
* This is a component definition to be used as a convenience reference to create an
* indeterminate `<mat-progress-spinner>` instance.
*/
@Component({
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,
/**
* @deprecated `changeDetectorRef`, `viewportRuler` and `ngZone`
* parameters to become required.
* @breaking-change 14.0.0
*/
changeDetectorRef?: ChangeDetectorRef,
viewportRuler?: ViewportRuler,
ngZone?: NgZone,
) {
super(
elementRef,
platform,
document,
animationMode,
defaults,
changeDetectorRef,
viewportRuler,
ngZone,
);
this.mode = 'indeterminate';
}
}
12 changes: 11 additions & 1 deletion src/material/progress-spinner/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,22 @@
* found in the LICENSE file at https://angular.io/license
*/

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

export * from './progress-spinner-module';
export {
MatProgressSpinner,
MatSpinner,
MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS,
ProgressSpinnerMode,
MatProgressSpinnerDefaultOptions,
MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS_FACTORY,
} 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;
15 changes: 4 additions & 11 deletions tools/public_api_guard/material/progress-spinner.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export class MatProgressSpinner extends _MatProgressSpinnerBase implements OnIni
get value(): number;
set value(newValue: NumberInput);
// (undocumented)
static ɵcmp: i0.ɵɵComponentDeclaration<MatProgressSpinner, "mat-progress-spinner", ["matProgressSpinner"], { "color": "color"; "diameter": "diameter"; "strokeWidth": "strokeWidth"; "mode": "mode"; "value": "value"; }, {}, never, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MatProgressSpinner, "mat-progress-spinner, mat-spinner", ["matProgressSpinner"], { "color": "color"; "diameter": "diameter"; "strokeWidth": "strokeWidth"; "mode": "mode"; "value": "value"; }, {}, never, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<MatProgressSpinner, [null, null, { optional: true; }, { optional: true; }, null, null, null, null]>;
}
Expand All @@ -71,18 +71,11 @@ export class MatProgressSpinnerModule {
// (undocumented)
static ɵinj: i0.ɵɵInjectorDeclaration<MatProgressSpinnerModule>;
// (undocumented)
static ɵmod: i0.ɵɵNgModuleDeclaration<MatProgressSpinnerModule, [typeof i1.MatProgressSpinner, typeof i1.MatSpinner], [typeof i2.MatCommonModule, typeof i3.CommonModule], [typeof i1.MatProgressSpinner, typeof i1.MatSpinner, typeof i2.MatCommonModule]>;
static ɵmod: i0.ɵɵNgModuleDeclaration<MatProgressSpinnerModule, [typeof i1.MatProgressSpinner], [typeof i2.MatCommonModule, typeof i3.CommonModule], [typeof i1.MatProgressSpinner, typeof i2.MatCommonModule]>;
}

// @public
export class MatSpinner extends MatProgressSpinner {
constructor(elementRef: ElementRef<HTMLElement>, platform: Platform, document: any, animationMode: string, defaults?: MatProgressSpinnerDefaultOptions,
changeDetectorRef?: ChangeDetectorRef, viewportRuler?: ViewportRuler, ngZone?: NgZone);
// (undocumented)
static ɵcmp: i0.ɵɵComponentDeclaration<MatSpinner, "mat-spinner", never, { "color": "color"; }, {}, never, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<MatSpinner, [null, null, { optional: true; }, { optional: true; }, null, null, null, null]>;
}
// @public @deprecated (undocumented)
export const MatSpinner: typeof MatProgressSpinner;

// @public
export type ProgressSpinnerMode = 'determinate' | 'indeterminate';
Expand Down

0 comments on commit 8eb98ca

Please sign in to comment.