Skip to content

Commit

Permalink
fix(material/progress-spinner): Progress spinner animation fails for …
Browse files Browse the repository at this point in the history
…floating point diameter values (#20192)

Fixes a bug in the Angular Material `progress-spinner` component where spinner animations do not
work for custom diameters with float point values. This is because the animation-name CSS style
on the spinner circle includes an unsupported period ‘.’ character when the diameter is a
float point number. An underscore can be substituted instead of a period to fix this.

Fixes #20158
  • Loading branch information
AlexLangdon committed Aug 19, 2020
1 parent 7ea505b commit 2c47b06
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/material/progress-spinner/progress-spinner.html
Expand Up @@ -25,7 +25,7 @@
cx="50%"
cy="50%"
[attr.r]="_getCircleRadius()"
[style.animation-name]="'mat-progress-spinner-stroke-rotate-' + diameter"
[style.animation-name]="'mat-progress-spinner-stroke-rotate-' + _spinnerAnimationLabel"
[style.stroke-dashoffset.px]="_getStrokeDashOffset()"
[style.stroke-dasharray.px]="_getStrokeCircumference()"
[style.stroke-width.%]="_getCircleStrokeWidth()"></circle>
Expand Down
67 changes: 67 additions & 0 deletions src/material/progress-spinner/progress-spinner.spec.ts
Expand Up @@ -16,6 +16,7 @@ describe('MatProgressSpinner', () => {
declarations: [
BasicProgressSpinner,
IndeterminateProgressSpinner,
IndeterminateSpinnerCustomDiameter,
ProgressSpinnerWithValueAndBoundMode,
ProgressSpinnerWithColor,
ProgressSpinnerCustomStrokeWidth,
Expand Down Expand Up @@ -185,6 +186,48 @@ describe('MatProgressSpinner', () => {
expect(document.head.querySelectorAll('style[mat-spinner-animation="64"]').length).toBe(1);
}));

it('should allow floating point values for custom diameter', () => {
const fixture = TestBed.createComponent(ProgressSpinnerCustomDiameter);

fixture.componentInstance.diameter = 32.5;
fixture.detectChanges();

const spinner = fixture.debugElement.query(By.css('mat-progress-spinner'))!.nativeElement;
const svgElement = fixture.nativeElement.querySelector('svg');

expect(parseFloat(spinner.style.width))
.toBe(32.5, 'Expected the custom diameter to be applied to the host element width.');
expect(parseFloat(spinner.style.height))
.toBe(32.5, 'Expected the custom diameter to be applied to the host element height.');
expect(parseFloat(svgElement.style.width))
.toBe(32.5, 'Expected the custom diameter to be applied to the svg element width.');
expect(parseFloat(svgElement.style.height))
.toBe(32.5, 'Expected the custom diameter to be applied to the svg element height.');
expect(svgElement.getAttribute('viewBox'))
.toBe('0 0 25.75 25.75', 'Expected the custom diameter to be applied to the svg viewBox.');
});

it('should handle creating animation style tags based on a floating point diameter',
inject([Platform], (platform: Platform) => {
// On Edge and IE we use a fallback animation because the
// browser doesn't support animating SVG correctly.
if (platform.EDGE || platform.TRIDENT) {
return;
}

const fixture = TestBed.createComponent(IndeterminateSpinnerCustomDiameter);

fixture.componentInstance.diameter = 32.5;
fixture.detectChanges();

const circleElement = fixture.nativeElement.querySelector('circle');

expect(circleElement.style.animationName).toBe('mat-progress-spinner-stroke-rotate-32_5',
'Expected the spinner circle element to have an animation name based on the custom diameter');
expect(document.head.querySelectorAll('style[mat-spinner-animation="32_5"]').length).toBe(1,
'Expected a style tag with the indeterminate animation to be attached to the document head');
}));

it('should allow a custom stroke width', () => {
const fixture = TestBed.createComponent(ProgressSpinnerCustomStrokeWidth);

Expand All @@ -200,6 +243,21 @@ describe('MatProgressSpinner', () => {
.toBe('0 0 130 130', 'Expected the viewBox to be adjusted based on the stroke width.');
});

it('should allow floating point values for custom stroke width', () => {
const fixture = TestBed.createComponent(ProgressSpinnerCustomStrokeWidth);

fixture.componentInstance.strokeWidth = 40.5;
fixture.detectChanges();

const circleElement = fixture.nativeElement.querySelector('circle');
const svgElement = fixture.nativeElement.querySelector('svg');

expect(parseFloat(circleElement.style.strokeWidth)).toBe(40.5, 'Expected the custom stroke ' +
'width to be applied to the circle element as a percentage of the element size.');
expect(svgElement.getAttribute('viewBox'))
.toBe('0 0 130.5 130.5', 'Expected the viewBox to be adjusted based on the stroke width.');
});

it('should expand the host element if the stroke width is greater than the default', () => {
const fixture = TestBed.createComponent(ProgressSpinnerCustomStrokeWidth);
const element = fixture.debugElement.nativeElement.querySelector('.mat-progress-spinner');
Expand Down Expand Up @@ -444,6 +502,15 @@ class ProgressSpinnerCustomDiameter {
@Component({template: '<mat-progress-spinner mode="indeterminate"></mat-progress-spinner>'})
class IndeterminateProgressSpinner { }

@Component({
template: `
<mat-progress-spinner mode="indeterminate" [diameter]="diameter"></mat-progress-spinner>
`,
})
class IndeterminateSpinnerCustomDiameter {
diameter: number;
}

@Component({
template: '<mat-progress-spinner [value]="value" [mode]="mode"></mat-progress-spinner>'
})
Expand Down
16 changes: 14 additions & 2 deletions src/material/progress-spinner/progress-spinner.ts
Expand Up @@ -147,11 +147,15 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
/** Whether the _mat-animation-noopable class should be applied, disabling animations. */
_noopAnimations: boolean;

/** A string that is used for setting the spinner animation-name CSS property */
_spinnerAnimationLabel: string;

/** The diameter of the progress spinner (will set width and height of svg). */
@Input()
get diameter(): number { return this._diameter; }
set diameter(size: number) {
this._diameter = coerceNumberProperty(size);
this._spinnerAnimationLabel = this._getSpinnerAnimationLabel();

// If this is set before `ngOnInit`, the style root may not have been resolved yet.
if (!this._fallbackAnimation && this._styleRoot) {
Expand Down Expand Up @@ -190,6 +194,7 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
super(_elementRef);

const trackedDiameters = MatProgressSpinner._diameters;
this._spinnerAnimationLabel = this._getSpinnerAnimationLabel();

// The base size is already inserted via the component's structural styles. We still
// need to track it so we don't end up adding the same styles again.
Expand Down Expand Up @@ -273,7 +278,7 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements

if (!diametersForElement || !diametersForElement.has(currentDiameter)) {
const styleTag: HTMLStyleElement = this._document.createElement('style');
styleTag.setAttribute('mat-spinner-animation', currentDiameter + '');
styleTag.setAttribute('mat-spinner-animation', this._spinnerAnimationLabel);
styleTag.textContent = this._getAnimationText();
styleRoot.appendChild(styleTag);

Expand All @@ -293,7 +298,14 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
// Animation should begin at 5% and end at 80%
.replace(/START_VALUE/g, `${0.95 * strokeCircumference}`)
.replace(/END_VALUE/g, `${0.2 * strokeCircumference}`)
.replace(/DIAMETER/g, `${this.diameter}`);
.replace(/DIAMETER/g, `${this._spinnerAnimationLabel}`);
}

/** Returns the circle diameter formatted for use with the animation-name CSS property. */
private _getSpinnerAnimationLabel(): string {
// The string of a float point number will include a period ‘.’ character,
// which is not valid for a CSS animation-name.
return this.diameter.toString().replace('.', '_');
}

static ngAcceptInputType_diameter: NumberInput;
Expand Down
1 change: 1 addition & 0 deletions tools/public_api_guard/material/progress-spinner.d.ts
Expand Up @@ -5,6 +5,7 @@ export declare function MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS_FACTORY(): MatProgr
export declare class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements OnInit, CanColor {
_elementRef: ElementRef<HTMLElement>;
_noopAnimations: boolean;
_spinnerAnimationLabel: string;
get diameter(): number;
set diameter(size: number);
mode: ProgressSpinnerMode;
Expand Down

0 comments on commit 2c47b06

Please sign in to comment.