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 (#23674)

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.

(cherry picked from commit 653e46b)
  • Loading branch information
crisbeto authored and andrewseguin committed Jan 15, 2022
1 parent 966b2c5 commit 462cb6d
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 16 deletions.
1 change: 1 addition & 0 deletions src/material/progress-spinner/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ ng_module(
deps = [
"//src/cdk/coercion",
"//src/cdk/platform",
"//src/cdk/scrolling",
"//src/material/core",
"@npm//@angular/animations",
"@npm//@angular/common",
Expand Down
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
72 changes: 66 additions & 6 deletions src/material/progress-spinner/progress-spinner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import {coerceNumberProperty, NumberInput} from '@angular/cdk/coercion';
import {Platform, _getShadowRoot} from '@angular/cdk/platform';
import {ViewportRuler} from '@angular/cdk/scrolling';
import {DOCUMENT} from '@angular/common';
import {
ChangeDetectionStrategy,
Expand All @@ -19,9 +20,13 @@ import {
Optional,
ViewEncapsulation,
OnInit,
ChangeDetectorRef,
OnDestroy,
NgZone,
} from '@angular/core';
import {CanColor, mixinColor} from '@angular/material/core';
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
import {Subscription} from 'rxjs';

/** Possible mode for a progress spinner. */
export type ProgressSpinnerMode = 'determinate' | 'indeterminate';
Expand Down Expand Up @@ -126,10 +131,14 @@ 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;
private _resizeSubscription = Subscription.EMPTY;

/**
* Element to which we should add the generated style tags for the indeterminate animation.
Expand Down Expand Up @@ -190,15 +199,19 @@ 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,
@Optional() @Inject(DOCUMENT) private _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);

Expand All @@ -223,6 +236,22 @@ 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.
// @breaking-change 14.0.0 Remove null checks for `_changeDetectorRef`,
// `viewportRuler` and `ngZone`.
if (_platform.isBrowser && _platform.SAFARI && viewportRuler && changeDetectorRef && ngZone) {
this._resizeSubscription = viewportRuler.change(150).subscribe(() => {
// 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') {
ngZone.run(() => changeDetectorRef.markForCheck());
}
});
}
}

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

ngOnDestroy() {
this._resizeSubscription.unsubscribe();
}

/** The radius of the spinner, adjusted for stroke width. */
_getCircleRadius() {
return (this.diameter - BASE_STROKE_WIDTH) / 2;
Expand Down Expand Up @@ -266,6 +299,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 @@ -338,8 +381,25 @@ export class MatSpinner extends MatProgressSpinner {
@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);
super(
elementRef,
platform,
document,
animationMode,
defaults,
changeDetectorRef,
viewportRuler,
ngZone,
);
this.mode = 'indeterminate';
}
}
20 changes: 14 additions & 6 deletions tools/public_api_guard/material/progress-spinner.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@

import { _AbstractConstructor } from '@angular/material/core';
import { CanColor } from '@angular/material/core';
import { ChangeDetectorRef } from '@angular/core';
import { _Constructor } from '@angular/material/core';
import { ElementRef } from '@angular/core';
import * as i0 from '@angular/core';
import * as i2 from '@angular/material/core';
import * as i3 from '@angular/common';
import { InjectionToken } from '@angular/core';
import { NgZone } from '@angular/core';
import { NumberInput } from '@angular/cdk/coercion';
import { OnDestroy } from '@angular/core';
import { OnInit } from '@angular/core';
import { Platform } from '@angular/cdk/platform';
import { ViewportRuler } from '@angular/cdk/scrolling';

// @public
export const MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS: InjectionToken<MatProgressSpinnerDefaultOptions>;
Expand All @@ -23,18 +27,21 @@ export const MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS: InjectionToken<MatProgressSpi
export function MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS_FACTORY(): MatProgressSpinnerDefaultOptions;

// @public
export class MatProgressSpinner extends _MatProgressSpinnerBase implements OnInit, CanColor {
constructor(elementRef: ElementRef<HTMLElement>,
_platform: Platform, _document: any, animationMode: string, defaults?: MatProgressSpinnerDefaultOptions);
export class MatProgressSpinner extends _MatProgressSpinnerBase implements OnInit, OnDestroy, CanColor {
constructor(elementRef: ElementRef<HTMLElement>, _platform: Platform, _document: any, animationMode: string, defaults?: MatProgressSpinnerDefaultOptions,
changeDetectorRef?: ChangeDetectorRef, viewportRuler?: ViewportRuler, ngZone?: NgZone);
get diameter(): number;
set diameter(size: NumberInput);
_getCircleRadius(): number;
_getCircleStrokeWidth(): number;
_getCircleTransformOrigin(svg: HTMLElement): string;
_getStrokeCircumference(): number;
_getStrokeDashOffset(): number | null;
_getViewBox(): string;
mode: ProgressSpinnerMode;
// (undocumented)
ngOnDestroy(): void;
// (undocumented)
ngOnInit(): void;
_noopAnimations: boolean;
_spinnerAnimationLabel: string;
Expand All @@ -45,7 +52,7 @@ export class MatProgressSpinner extends _MatProgressSpinnerBase implements OnIni
// (undocumented)
static ɵcmp: i0.ɵɵComponentDeclaration<MatProgressSpinner, "mat-progress-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]>;
static ɵfac: i0.ɵɵFactoryDeclaration<MatProgressSpinner, [null, null, { optional: true; }, { optional: true; }, null, null, null, null]>;
}

// @public
Expand All @@ -67,11 +74,12 @@ export class MatProgressSpinnerModule {

// @public
export class MatSpinner extends MatProgressSpinner {
constructor(elementRef: ElementRef<HTMLElement>, platform: Platform, document: any, animationMode: string, defaults?: MatProgressSpinnerDefaultOptions);
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]>;
static ɵfac: i0.ɵɵFactoryDeclaration<MatSpinner, [null, null, { optional: true; }, { optional: true; }, null, null, null, null]>;
}

// @public
Expand Down

0 comments on commit 462cb6d

Please sign in to comment.