fix(api): defer getComputedStyle() calls until ngOnInit phase

Previously, the Show/Hide directives would query the `display` style during directive instantiation.

For elements instantiated during `*ngFor`, constructor calls to getComputedStyle() will incorrectly return “” due to failures in queries for styles in CSS stylesheets or inline styles. Deferring these types of queries to the ngOnInit() lifecycle event, enables valid response to queries for both inline styles and stylesheets.

Fixes #310.

src/lib/flexbox/api/base.ts

@@ -9,21 +9,20 @@ import {
   ElementRef, OnDestroy, SimpleChanges, OnChanges,
   SimpleChange, Renderer2
 } from '@angular/core';
-import {ɵgetDOM as getDom} from '@angular/platform-browser';
 
-import {applyCssPrefixes} from '../../utils/auto-prefixer';
 import {buildLayoutCSS} from '../../utils/layout-validator';
+import {
+  StyleDefinition,
+  lookupStyle,
+  lookupInlineStyle,
+  applyStyleToElement,
+  applyStyleToElements
+} from '../../utils/style-utils';
 
 import {ResponsiveActivation, KeyOptions} from '../responsive/responsive-activation';
 import {MediaMonitor} from '../../media-query/media-monitor';
 import {MediaQuerySubscriber} from '../../media-query/media-change';
 
-/**
- * Definition of a css style. Either a property name (e.g. "flex-basis") or an object
- * map of property name and value (e.g. {display: 'none', flex-order: 5}).
- */
-export type StyleDefinition = string | { [property: string]: string | number };
-
 /** Abstract base class for the Layout API styling directives. */
 export abstract class BaseFxDirective implements OnDestroy, OnChanges {
 
@@ -66,7 +65,6 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
   constructor(protected _mediaMonitor: MediaMonitor,
               protected _elementRef: ElementRef,
               protected _renderer: Renderer2) {
-    this._display = this._getDisplayStyle();
   }
 
   // *********************************************
@@ -92,6 +90,15 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
   // Lifecycle Methods
   // *********************************************
 
+  /**
+   * Use post-component-initialization event to perform extra
+   * querying such as computed Display style
+   */
+  ngOnInit() {
+    this._display = this._getDisplayStyle();
+    this._hasInitialized = true;
+  }
+
   ngOnChanges(change: SimpleChanges) {
     throw new Error(`BaseFxDirective::ngOnChanges should be overridden in subclass: ${change}`);
   }
@@ -124,10 +131,7 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
    */
   protected _getDisplayStyle(source?: HTMLElement): string {
     let element: HTMLElement = source || this._elementRef.nativeElement;
-    let value = this._lookupStyle(element, 'display');
-
-    // Note: 'inline' is the default of all elements, unless UA stylesheet overrides
-    return value ? value.trim() : 'inline';
+    return lookupStyle(element, 'display');
   }
 
   /**
@@ -140,74 +144,32 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
     let value = 'row';
 
     if (target) {
-      value = this._lookupStyle(target, 'flex-direction') || 'row';
+      value = lookupStyle(target, 'flex-direction') || 'row';
+      let hasInlineValue = lookupInlineStyle(target, 'flex-direction');
 
-      let hasInlineValue = getDom().getStyle(target, 'flex-direction');
       if (!hasInlineValue && addIfMissing) {
-        this._applyStyleToElements(buildLayoutCSS(value), [target]);
+        applyStyleToElements(this._renderer, buildLayoutCSS(value), [target]);
       }
     }
 
     return value.trim();
   }
 
-  /**
-   * Determine the inline or inherited CSS style
-   */
-  protected _lookupStyle(element: HTMLElement, styleName: string): any {
-    let value = '';
-    try {
-      if (element) {
-        let immediateValue = getDom().getStyle(element, styleName);
-        value = immediateValue || getDom().getComputedStyle(element).getPropertyValue(styleName);
-      }
-    } catch (e) {
-      // TODO: platform-server throws an exception for getComputedStyle
-    }
-    return value;
-  }
-
-  /**
-   * Applies the styles to the element. The styles object map may contain an array of values. Each
-   * value will be added as element style.
-   */
-  protected _applyMultiValueStyleToElement(styles: {}, element: any) {
-    Object.keys(styles).forEach(key => {
-      const values = Array.isArray(styles[key]) ? styles[key] : [styles[key]];
-      for (let value of values) {
-        this._renderer.setStyle(element, key, value);
-      }
-    });
-  }
-
   /**
    * Applies styles given via string pair or object map to the directive element.
    */
   protected _applyStyleToElement(style: StyleDefinition,
                                  value?: string | number,
                                  nativeElement?: any) {
-    let styles = {};
     let element = nativeElement || this._elementRef.nativeElement;
-
-    if (typeof style === 'string') {
-      styles[style] = value;
-      style = styles;
-    }
-
-    styles = applyCssPrefixes(style);
-
-    this._applyMultiValueStyleToElement(styles, element);
+    applyStyleToElement(this._renderer, element, style, value);
   }
 
   /**
-   * Applies styles given via string pair or object map to the directive element.
+   * Applies styles given via string pair or object map to the directive's element.
    */
   protected _applyStyleToElements(style: StyleDefinition, elements: HTMLElement[ ]) {
-    let styles = applyCssPrefixes(style);
-
-    elements.forEach(el => {
-      this._applyMultiValueStyleToElement(styles, el);
-    });
+    applyStyleToElements(this._renderer, style, elements || []);
   }
 
   /**
@@ -264,6 +226,10 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
     return this._mqActivation.hasKeyValue(key);
   }
 
+  protected get hasInitialized() {
+    return this._hasInitialized;
+  }
+
   /** Original dom Elements CSS display style */
   protected _display;
 
@@ -277,4 +243,11 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
    */
   protected _inputMap = {};
 
+  /**
+   * Has the `ngOnInit()` method fired
+   *
+   * Used to allow *ngFor tasks to finish and support queries like
+   * getComputedStyle() during ngOnInit().
+   */
+  protected _hasInitialized = false;
 }

src/lib/flexbox/api/flex-align.ts

@@ -77,6 +77,8 @@ export class FlexAlignDirective extends BaseFxDirective implements OnInit, OnCha
    * mql change events to onMediaQueryChange handlers
    */
   ngOnInit() {
+    super.ngOnInit();
+
     this._listenForMediaQueryChanges('align', 'stretch', (changes: MediaChange) => {
       this._updateWithValue(changes.value);
     });

src/lib/flexbox/api/flex-offset.ts

@@ -95,6 +95,8 @@ export class FlexOffsetDirective extends BaseFxDirective implements OnInit, OnCh
    * mql change events to onMediaQueryChange handlers
    */
   ngOnInit() {
+    super.ngOnInit();
+
     this._listenForMediaQueryChanges('offset', 0 , (changes: MediaChange) => {
       this._updateWithValue(changes.value);
     });

src/lib/flexbox/api/flex-order.ts

@@ -74,6 +74,8 @@ export class FlexOrderDirective extends BaseFxDirective implements OnInit, OnCha
    * mql change events to onMediaQueryChange handlers
    */
   ngOnInit() {
+    super.ngOnInit();
+
     this._listenForMediaQueryChanges('order', '0', (changes: MediaChange) => {
       this._updateWithValue(changes.value);
     });

src/lib/flexbox/api/flex.ts

@@ -118,6 +118,8 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
    * mql change events to onMediaQueryChange handlers
    */
   ngOnInit() {
+    super.ngOnInit();
+
     this._listenForMediaQueryChanges('flex', '', (changes: MediaChange) => {
       this._updateStyle(changes.value);
     });

src/lib/flexbox/api/layout-align.ts

@@ -91,6 +91,8 @@ export class LayoutAlignDirective extends BaseFxDirective implements OnInit, OnC
    * mql change events to onMediaQueryChange handlers
    */
   ngOnInit() {
+    super.ngOnInit();
+
     this._listenForMediaQueryChanges('align', 'start stretch', (changes: MediaChange) => {
       this._updateWithValue(changes.value);
     });

src/lib/flexbox/api/layout-wrap.ts

@@ -89,6 +89,8 @@ export class LayoutWrapDirective extends BaseFxDirective implements OnInit, OnCh
    * mql change events to onMediaQueryChange handlers
    */
   ngOnInit() {
+    super.ngOnInit();
+
     this._listenForMediaQueryChanges('wrap', 'wrap', (changes: MediaChange) => {
       this._updateWithValue(changes.value);
     });

src/lib/flexbox/api/layout.ts

@@ -97,6 +97,8 @@ export class LayoutDirective extends BaseFxDirective implements OnInit, OnChange
    * mql change events to onMediaQueryChange handlers
    */
   ngOnInit() {
+    super.ngOnInit();
+
     this._listenForMediaQueryChanges('layout', 'row', (changes: MediaChange) => {
       this._updateWithDirection(changes.value);
     });

src/lib/flexbox/api/show-hide.ts

@@ -108,7 +108,6 @@ export class ShowHideDirective extends BaseFxDirective implements OnInit, OnChan
 
     super(monitor, elRef, renderer);
 
-    this._display = this._getDisplayStyle();  // re-invoke override to use `this._layout`
     if (_layout) {
       /**
        * The Layout can set the display:flex (and incorrectly affect the Hide/Show directives.
@@ -138,7 +137,7 @@ export class ShowHideDirective extends BaseFxDirective implements OnInit, OnChan
    * Then conditionally override with the mq-activated Input's current value
    */
   ngOnChanges(changes: SimpleChanges) {
-    if (changes['show'] != null || this._mqActivation) {
+    if (this.hasInitialized && (changes['show'] != null || this._mqActivation)) {
       this._updateWithValue();
     }
   }
@@ -148,8 +147,9 @@ export class ShowHideDirective extends BaseFxDirective implements OnInit, OnChan
    * mql change events to onMediaQueryChange handlers
    */
   ngOnInit() {
-    let value = this._getDefaultVal('show', true);
+    super.ngOnInit();
 
+    let value = this._getDefaultVal('show', true);
     // Build _mqActivation controller
     this._listenForMediaQueryChanges('show', value, (changes: MediaChange) => {
       this._updateWithValue(changes.value);

src/lib/utils/style-utils.spec.ts

@@ -0,0 +1,75 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+import {Component} from '@angular/core';
+import {CommonModule} from '@angular/common';
+import {TestBed} from '@angular/core/testing';
+
+import {customMatchers} from './testing/custom-matchers';
+import {makeExpectDOMFrom} from './testing/helpers';
+
+describe('style-utils directive', () => {
+  let expectDOMFrom = makeExpectDOMFrom(() => TestLayoutComponent);
+
+  beforeEach(() => {
+    jasmine.addMatchers(customMatchers);
+
+    // Configure testbed to prepare services
+    TestBed.configureTestingModule({
+      imports: [CommonModule],
+      declarations: [TestLayoutComponent]
+    });
+  });
+
+  describe('testing display styles', () => {
+
+    it('should default to "display:block" for <div></div>', () => {
+      expectDOMFrom(`
+        <div></div>
+      `).toHaveStyle({'display': 'block'});
+    });
+
+    it('should find to "display" for inline style <div></div>', () => {
+      expectDOMFrom(`
+        <div style="display: flex;"></div>
+      `).toHaveStyle({'display': 'flex'});
+    });
+
+    it('should find `display` from html style element', () => {
+      expectDOMFrom(`
+        <style>
+          div.special { display: inline-block; }
+        </style>
+        <div class="special"></div>
+      `).toHaveStyle({'display': 'inline-block'});
+    });
+
+    it('should find `display` from component styles', () => {
+      let expectStyledDOM = makeExpectDOMFrom(() => TestLayoutComponent, [
+          'div.extra { display:table; }'
+      ]);
+      expectStyledDOM(`
+        <div class="extra"></div>
+      `).toHaveStyle({'display': 'table'});
+    });
+
+  });
+
+
+});
+
+
+// *****************************************************************
+// Template Component
+// *****************************************************************
+
+@Component({
+  selector: 'test-style-utils',
+  template: `<span>PlaceHolder Template HTML</span>`
+})
+export class TestLayoutComponent {
+}