Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(material/progress-spinner): Progress spinner animation fails for floating point diameter values #20192

Merged
merged 1 commit into from
Aug 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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