Skip to content

Commit

Permalink
fix(material/progress-spinner): animation not working on some zoom le…
Browse files Browse the repository at this point in the history
…vels in Safari

Fixes that the progress spinner animation was broken on Safari on non-default zoom levels. The problem seems to be that the `transform-origin` was being offset by the amount of zoom. These changes work around it by setting the origin based on the zoom level.

Fixes #23668.
  • Loading branch information
crisbeto committed Oct 1, 2021
1 parent 7c16258 commit 19bab1d
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 13 deletions.
9 changes: 6 additions & 3 deletions src/material/progress-spinner/progress-spinner.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
preserveAspectRatio="xMidYMid meet"
focusable="false"
[ngSwitch]="mode === 'indeterminate'"
aria-hidden="true">
aria-hidden="true"
#svg>

<!--
Technically we can reuse the same `circle` element, however Safari has an issue that breaks
Expand All @@ -31,7 +32,8 @@
[style.animation-name]="'mat-progress-spinner-stroke-rotate-' + _spinnerAnimationLabel"
[style.stroke-dashoffset.px]="_getStrokeDashOffset()"
[style.stroke-dasharray.px]="_getStrokeCircumference()"
[style.stroke-width.%]="_getCircleStrokeWidth()"></circle>
[style.stroke-width.%]="_getCircleStrokeWidth()"
[style.transform-origin]="_getCircleTransformOrigin(svg)"></circle>

<circle
*ngSwitchCase="false"
Expand All @@ -40,5 +42,6 @@
[attr.r]="_getCircleRadius()"
[style.stroke-dashoffset.px]="_getStrokeDashOffset()"
[style.stroke-dasharray.px]="_getStrokeCircumference()"
[style.stroke-width.%]="_getCircleStrokeWidth()"></circle>
[style.stroke-width.%]="_getCircleStrokeWidth()"
[style.transform-origin]="_getCircleTransformOrigin(svg)"></circle>
</svg>
1 change: 0 additions & 1 deletion src/material/progress-spinner/progress-spinner.scss
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ $_default-circumference: variables.$pi * $_default-radius * 2;
circle {
@include private.private-animation-noop();
fill: transparent;
transform-origin: center;
transition: stroke-dashoffset 225ms linear;

@include a11y.high-contrast(active, off) {
Expand Down
57 changes: 48 additions & 9 deletions src/material/progress-spinner/progress-spinner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
Optional,
ViewEncapsulation,
OnInit,
ChangeDetectorRef,
OnDestroy,
} from '@angular/core';
import {CanColor, mixinColor} from '@angular/material/core';
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
Expand Down Expand Up @@ -124,7 +126,8 @@ const INDETERMINATE_ANIMATION_TEMPLATE = `
changeDetection: ChangeDetectionStrategy.OnPush,
encapsulation: ViewEncapsulation.None,
})
export class MatProgressSpinner extends _MatProgressSpinnerBase implements OnInit, CanColor {
export class MatProgressSpinner extends _MatProgressSpinnerBase implements OnInit, OnDestroy,
CanColor {
private _diameter = BASE_SIZE;
private _value = 0;
private _strokeWidth: number;
Expand Down Expand Up @@ -185,15 +188,16 @@ export class MatProgressSpinner extends _MatProgressSpinnerBase implements OnIni
}

constructor(elementRef: ElementRef<HTMLElement>,
/**
* @deprecated `_platform` parameter no longer being used.
* @breaking-change 14.0.0
*/
_platform: Platform,
private _platform: Platform,
@Optional() @Inject(DOCUMENT) private _document: any,
@Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode: string,
@Inject(MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS)
defaults?: MatProgressSpinnerDefaultOptions) {
defaults?: MatProgressSpinnerDefaultOptions,
/**
* @deprecated `changeDetectorRef` parameter to become required.
* @breaking-change 14.0.0
*/
private _changeDetectorRef?: ChangeDetectorRef) {

super(elementRef);

Expand All @@ -218,6 +222,14 @@ export class MatProgressSpinner extends _MatProgressSpinnerBase implements OnIni
this.strokeWidth = defaults.strokeWidth;
}
}

// Safari has an issue where the circle isn't positioned correctly when the page has a
// different zoom level from the default. This handler triggers a recalculation of the
// `transform-origin` when the page zoom level changes.
// See `_getCircleTransformOrigin` for more info.
if (_platform.isBrowser && _platform.SAFARI) {
window.addEventListener('resize', this._resizeHandler);
}
}

ngOnInit() {
Expand All @@ -231,6 +243,12 @@ export class MatProgressSpinner extends _MatProgressSpinnerBase implements OnIni
element.classList.add('mat-progress-spinner-indeterminate-animation');
}

ngOnDestroy() {
if (this._platform.isBrowser) {
window.removeEventListener('resize', this._resizeHandler);
}
}

/** The radius of the spinner, adjusted for stroke width. */
_getCircleRadius() {
return (this.diameter - BASE_STROKE_WIDTH) / 2;
Expand Down Expand Up @@ -261,6 +279,16 @@ export class MatProgressSpinner extends _MatProgressSpinnerBase implements OnIni
return this.strokeWidth / this.diameter * 100;
}

/** Gets the `transform-origin` for the inner circle element. */
_getCircleTransformOrigin(svg: HTMLElement): string {
// Safari has an issue where the `transform-origin` doesn't work as expected when the page
// has a different zoom level from the default. The problem appears to be that a zoom
// is applied on the `svg` node itself. We can work around it by calculating the origin
// based on the zoom level. On all other browsers the `currentScale` appears to always be 1.
const scale = ((svg as unknown as SVGSVGElement).currentScale ?? 1) * 50;
return `${scale}% ${scale}%`;
}

/** Dynamically generates a style tag containing the correct animation for this diameter. */
private _attachStyleNode(): void {
const styleRoot = this._styleRoot;
Expand Down Expand Up @@ -300,6 +328,16 @@ export class MatProgressSpinner extends _MatProgressSpinnerBase implements OnIni
return this.diameter.toString().replace('.', '_');
}

/** Handles window `resize` events. */
private _resizeHandler = () => {
// When the window is resize while the spinner is in `indeterminate` mode, we
// have to mark for check so the transform origin of the circle can be recomputed.
if (this.mode === 'indeterminate') {
// @breaking-change 14.0.0 Remove null check for `_changeDetectorRef`.
this._changeDetectorRef?.markForCheck();
}
}

static ngAcceptInputType_diameter: NumberInput;
static ngAcceptInputType_strokeWidth: NumberInput;
static ngAcceptInputType_value: NumberInput;
Expand Down Expand Up @@ -333,8 +371,9 @@ export class MatSpinner extends MatProgressSpinner {
@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);
defaults?: MatProgressSpinnerDefaultOptions,
changeDetectorRef?: ChangeDetectorRef) {
super(elementRef, platform, document, animationMode, defaults, changeDetectorRef);
this.mode = 'indeterminate';
}
}

0 comments on commit 19bab1d

Please sign in to comment.