Skip to content

fix(material/progress-spinner): animation not working on some zoom levels in Safari #23674

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

Merged
merged 1 commit into from
Jan 15, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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