fix(progress-spinner): animation node inserted into wrong style root when using ngIf with ShadowDom encapsulation (#16982)

If the progress spinner is inside the shadow DOM, we attach the `style` tag with the animation to the shadow root, however since we do the insertion inside the constructor, the spinner might not have been moved into the shadow root yet. These changes move the logic into `ngOnInit` which fires late enough for the node to be in the correct place.

Author: crisbeto

src/material/progress-spinner/BUILD.bazel

@@ -51,6 +51,7 @@ ng_test_library(
     deps = [
         ":progress-spinner",
         "//src/cdk/platform",
+        "@npm//@angular/common",
         "@npm//@angular/platform-browser",
     ],
 )

src/material/progress-spinner/progress-spinner.spec.ts

@@ -1,7 +1,8 @@
 import {TestBed, async, inject} from '@angular/core/testing';
-import {Component, ViewEncapsulation} from '@angular/core';
+import {Component, ViewEncapsulation, ViewChild, ElementRef} from '@angular/core';
 import {By} from '@angular/platform-browser';
 import {Platform} from '@angular/cdk/platform';
+import {CommonModule} from '@angular/common';
 import {_getShadowRoot} from './progress-spinner';
 import {
   MatProgressSpinnerModule,
@@ -14,7 +15,7 @@ describe('MatProgressSpinner', () => {
 
   beforeEach(async(() => {
     TestBed.configureTestingModule({
-      imports: [MatProgressSpinnerModule],
+      imports: [MatProgressSpinnerModule, CommonModule],
       declarations: [
         BasicProgressSpinner,
         IndeterminateProgressSpinner,
@@ -25,6 +26,7 @@ describe('MatProgressSpinner', () => {
         SpinnerWithColor,
         ProgressSpinnerWithStringValues,
         IndeterminateSpinnerInShadowDom,
+        IndeterminateSpinnerInShadowDomWithNgIf,
       ],
     }).compileComponents();
   }));
@@ -404,6 +406,28 @@ describe('MatProgressSpinner', () => {
     expect(shadowRoot.querySelectorAll('style[mat-spinner-animation="61"]').length).toBe(1);
   });
 
+  it('should add the indeterminate animation style tag to the Shadow root if the element is ' +
+    'inside an ngIf', () => {
+      // The test is only relevant in browsers that support Shadow DOM.
+      if (!supportsShadowDom) {
+        return;
+      }
+
+      const fixture = TestBed.createComponent(IndeterminateSpinnerInShadowDomWithNgIf);
+      fixture.componentInstance.diameter = 27;
+      fixture.detectChanges();
+
+      const spinner = fixture.componentInstance.spinner.nativeElement;
+      const shadowRoot = _getShadowRoot(spinner, document) as HTMLElement;
+
+      expect(shadowRoot.querySelector('style[mat-spinner-animation="27"]')).toBeTruthy();
+
+      fixture.componentInstance.diameter = 15;
+      fixture.detectChanges();
+
+      expect(shadowRoot.querySelector('style[mat-spinner-animation="27"]')).toBeTruthy();
+    });
+
 });
 
 
@@ -454,3 +478,19 @@ class ProgressSpinnerWithStringValues { }
 class IndeterminateSpinnerInShadowDom {
   diameter: number;
 }
+
+@Component({
+  template: `
+    <div *ngIf="true">
+      <mat-progress-spinner mode="indeterminate" [diameter]="diameter"></mat-progress-spinner>
+    </div>
+  `,
+  encapsulation: ViewEncapsulation.ShadowDom,
+})
+class IndeterminateSpinnerInShadowDomWithNgIf {
+  @ViewChild(MatProgressSpinner, {read: ElementRef, static: false})
+  spinner: ElementRef<HTMLElement>;
+
+  diameter: number;
+}
+

src/material/progress-spinner/progress-spinner.ts

@@ -18,6 +18,7 @@ import {
   Input,
   Optional,
   ViewEncapsulation,
+  OnInit,
 } from '@angular/core';
 import {CanColor, CanColorCtor, mixinColor} from '@angular/material/core';
 import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
@@ -123,7 +124,7 @@ const INDETERMINATE_ANIMATION_TEMPLATE = `
   changeDetection: ChangeDetectionStrategy.OnPush,
   encapsulation: ViewEncapsulation.None,
 })
-export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements CanColor {
+export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements OnInit, CanColor {
   private _diameter = BASE_SIZE;
   private _value = 0;
   private _strokeWidth: number;
@@ -153,13 +154,9 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
   set diameter(size: number) {
     this._diameter = coerceNumberProperty(size);
 
-    if (!this._fallbackAnimation) {
-      const trackedDiameters = MatProgressSpinner._diameters;
-      const diametersForElement = trackedDiameters.get(this._styleRoot);
-
-      if (!diametersForElement || !diametersForElement.has(this._diameter)) {
-        this._attachStyleNode();
-      }
+    // If this is set before `ngOnInit`, the style root may not have been resolved yet.
+    if (!this._fallbackAnimation && this._styleRoot) {
+      this._attachStyleNode();
     }
   }
 
@@ -201,7 +198,6 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
       trackedDiameters.set(_document.head, new Set<number>([BASE_SIZE]));
     }
 
-    this._styleRoot = _getShadowRoot(_elementRef.nativeElement, _document) || _document.head;
     this._fallbackAnimation = platform.EDGE || platform.TRIDENT;
     this._noopAnimations = animationMode === 'NoopAnimations' &&
         (!!defaults && !defaults._forceAnimations);
@@ -215,13 +211,23 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
         this.strokeWidth = defaults.strokeWidth;
       }
     }
+  }
+
+  ngOnInit() {
+    const element = this._elementRef.nativeElement;
+
+    // Note that we need to look up the root node in ngOnInit, rather than the constructor, because
+    // Angular seems to create the element outside the shadow root and then moves it inside, if the
+    // node is inside an `ngIf` and a ShadowDom-encapsulated component.
+    this._styleRoot = _getShadowRoot(element, this._document) || this._document.head;
+    this._attachStyleNode();
 
     // On IE and Edge, we can't animate the `stroke-dashoffset`
     // reliably so we fall back to a non-spec animation.
     const animationClass =
       `mat-progress-spinner-indeterminate${this._fallbackAnimation ? '-fallback' : ''}-animation`;
 
-    _elementRef.nativeElement.classList.add(animationClass);
+    element.classList.add(animationClass);
   }
 
   /** The radius of the spinner, adjusted for stroke width. */
@@ -261,22 +267,24 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
 
   /** Dynamically generates a style tag containing the correct animation for this diameter. */
   private _attachStyleNode(): void {
-    const styleTag: HTMLStyleElement = this._document.createElement('style');
     const styleRoot = this._styleRoot;
     const currentDiameter = this._diameter;
     const diameters = MatProgressSpinner._diameters;
     let diametersForElement = diameters.get(styleRoot);
 
-    styleTag.setAttribute('mat-spinner-animation', currentDiameter + '');
-    styleTag.textContent = this._getAnimationText();
-    styleRoot.appendChild(styleTag);
+    if (!diametersForElement || !diametersForElement.has(currentDiameter)) {
+      const styleTag: HTMLStyleElement = this._document.createElement('style');
+      styleTag.setAttribute('mat-spinner-animation', currentDiameter + '');
+      styleTag.textContent = this._getAnimationText();
+      styleRoot.appendChild(styleTag);
 
-    if (!diametersForElement) {
-      diametersForElement = new Set<number>();
-      diameters.set(styleRoot, diametersForElement);
-    }
+      if (!diametersForElement) {
+        diametersForElement = new Set<number>();
+        diameters.set(styleRoot, diametersForElement);
+      }
 
-    diametersForElement.add(currentDiameter);
+      diametersForElement.add(currentDiameter);
+    }
   }
 
   /** Generates animation styles adjusted for the spinner's diameter. */

tools/public_api_guard/material/progress-spinner.d.ts

@@ -2,7 +2,7 @@ export declare const MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS: InjectionToken<MatPro
 
 export declare function MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS_FACTORY(): MatProgressSpinnerDefaultOptions;
 
-export declare class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements CanColor {
+export declare class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements OnInit, CanColor {
     readonly _circleRadius: number;
     readonly _circleStrokeWidth: number;
     _elementRef: ElementRef<HTMLElement>;
@@ -15,6 +15,7 @@ export declare class MatProgressSpinner extends _MatProgressSpinnerMixinBase imp
     strokeWidth: number;
     value: number;
     constructor(_elementRef: ElementRef<HTMLElement>, platform: Platform, _document: any, animationMode: string, defaults?: MatProgressSpinnerDefaultOptions);
+    ngOnInit(): void;
 }
 
 export interface MatProgressSpinnerDefaultOptions {