fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush

ThomasBurleson · ThomasBurleson · commit 81c79e015c78 · 2017-03-19T10:29:38.000-05:00
@see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview fixes #206.
diff --git a/src/lib/flexbox/api/base.ts b/src/lib/flexbox/api/base.ts
@@ -22,20 +22,10 @@ export type StyleDefinition = string|{[property: string]: string|number};
 
 /** Abstract base class for the Layout API styling directives. */
 export abstract class BaseFxDirective implements OnDestroy {
-  /**
-   * Original dom Elements CSS display style
-   */
-  protected _display;
 
-  /**
-   * MediaQuery Activation Tracker
-   */
-  protected _mqActivation: ResponsiveActivation;
-
-  /**
-   *  Dictionary of input keys with associated values
-   */
-  protected _inputMap = {};
+  get hasMQListener() {
+    return !!this._mqActivation;
+  }
 
   /**
    *
@@ -46,6 +36,7 @@ export abstract class BaseFxDirective implements OnDestroy {
     this._display = this._getDisplayStyle();
   }
 
+
   // *********************************************
   // Accessor Methods
   // *********************************************
@@ -172,12 +163,15 @@ export abstract class BaseFxDirective implements OnDestroy {
   protected _listenForMediaQueryChanges(key: string,
                                         defaultValue: any,
                                         onMediaQueryChange: MediaQuerySubscriber): ResponsiveActivation { // tslint:disable-line:max-line-length
-    let keyOptions = new KeyOptions(key, defaultValue, this._inputMap);
-    return this._mqActivation = new ResponsiveActivation(
-        keyOptions,
-        this._mediaMonitor,
-        (change) => onMediaQueryChange.call(this, change)
-    );
+    if ( !this._mqActivation ) {
+      let keyOptions = new KeyOptions(key, defaultValue, this._inputMap);
+      this._mqActivation = new ResponsiveActivation(
+          keyOptions,
+          this._mediaMonitor,
+          (change) => onMediaQueryChange.call(this, change)
+      );
+    }
+    return this._mqActivation;
   }
 
   /**
@@ -201,4 +195,18 @@ export abstract class BaseFxDirective implements OnDestroy {
     return this._mqActivation.hasKeyValue(key);
   }
 
+  /**
+   * Original dom Elements CSS display style
+   */
+  protected _display;
+
+  /**
+   * MediaQuery Activation Tracker
+   */
+  protected _mqActivation: ResponsiveActivation;
+
+  /**
+   *  Dictionary of input keys with associated values
+   */
+  protected _inputMap = {};
 }
diff --git a/src/lib/flexbox/api/class.spec.ts b/src/lib/flexbox/api/class.spec.ts
@@ -65,15 +65,36 @@ describe('class directive', () => {
         });
       });
 
-  it('should keep existing class selector', () => {
+  it('should NOT keep the existing class', () => {
     fixture = createTestComponent(`
         <div class="existing-class" ngClass.xs="xs-class">
         </div>
     `);
 
     expectNativeEl(fixture).toHaveCssClass('existing-class');
+
     activateMediaQuery('xs');
     expectNativeEl(fixture).not.toHaveCssClass('existing-class');
+    expectNativeEl(fixture).toHaveCssClass('xs-class');
+
+    activateMediaQuery('lg');
+    expectNativeEl(fixture).toHaveCssClass('existing-class');
+    expectNativeEl(fixture).not.toHaveCssClass('xs-class');
+  });
+
+
+  it('should keep allow removal of class selector', () => {
+    fixture = createTestComponent(`
+        <div 
+            class="existing-class" 
+            [ngClass.xs]="{'xs-class':true, 'existing-class':false}">
+        </div>
+    `);
+
+    expectNativeEl(fixture).toHaveCssClass('existing-class');
+    activateMediaQuery('xs');
+    expectNativeEl(fixture).not.toHaveCssClass('existing-class');
+    expectNativeEl(fixture).toHaveCssClass('xs-class');
 
     activateMediaQuery('lg');
     expectNativeEl(fixture).toHaveCssClass('existing-class');
@@ -90,6 +111,7 @@ describe('class directive', () => {
     expectNativeEl(fixture).toHaveCssClass('existing-class');
     activateMediaQuery('xs');
     expectNativeEl(fixture).toHaveCssClass('existing-class');
+    expectNativeEl(fixture).toHaveCssClass('xs-class');
 
     activateMediaQuery('lg');
     expectNativeEl(fixture).toHaveCssClass('existing-class');
@@ -112,21 +134,23 @@ describe('class directive', () => {
 
   it('should work with ngClass object notation', () => {
     fixture = createTestComponent(`
-        <div [ngClass]="{'xs-1': hasXs1, 'xs-3': hasXs3}" 
-             [ngClass.xs]="{'xs-1': hasXs1, 'xs-2': hasXs2}">
+        <div [ngClass]="{'x1': hasX1, 'x3': hasX3}" 
+             [ngClass.xs]="{'x1': hasX1, 'x2': hasX2}">
         </div>
     `);
-    activateMediaQuery('xs');
-    expectNativeEl(fixture, {hasXs1: true, hasXs2: false}).toHaveCssClass('xs-1');
-    expectNativeEl(fixture, {hasXs1: true, hasXs2: false}).not.toHaveCssClass('xs-2');
+    expectNativeEl(fixture, {hasX1: true, hasX2: true, hasX3: true}).toHaveCssClass('x1');
+    expectNativeEl(fixture, {hasX1: true, hasX2: true, hasX3: true}).not.toHaveCssClass('x2');
+    expectNativeEl(fixture, {hasX1: true, hasX2: true, hasX3: true}).toHaveCssClass('x3');
 
-    expectNativeEl(fixture, {hasXs1: false, hasXs2: true}).toHaveCssClass('xs-2');
-    expectNativeEl(fixture, {hasXs1: false, hasXs2: true}).not.toHaveCssClass('xs-1');
+    activateMediaQuery('X');
+    expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: false}).toHaveCssClass('x1');
+    expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: false}).not.toHaveCssClass('x2');
+    expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: false}).not.toHaveCssClass('x3');
 
     activateMediaQuery('md');
-    expectNativeEl(fixture, {hasXs1: true, hasX2: false, hasXs3: true}).toHaveCssClass('xs-3');
-    expectNativeEl(fixture, {hasXs1: true, hasX2: false, hasXs3: true}).not.toHaveCssClass('xs-2');
-    expectNativeEl(fixture, {hasXs1: true, hasX2: false, hasXs3: true}).toHaveCssClass('xs-1');
+    expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: true}).toHaveCssClass('x1');
+    expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: true}).not.toHaveCssClass('x2');
+    expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: true}).toHaveCssClass('x3');
   });
 
   it('should work with ngClass array notation', () => {
@@ -151,6 +175,7 @@ describe('class directive', () => {
 export class TestClassComponent implements OnInit {
   hasXs1: boolean;
   hasXs2: boolean;
+  hasXs3: boolean;
 
   constructor(private media: ObservableMedia) {
   }
diff --git a/src/lib/flexbox/api/class.ts b/src/lib/flexbox/api/class.ts
@@ -9,20 +9,18 @@ import {
   Directive,
   ElementRef,
   Input,
+  DoCheck,
   OnDestroy,
-  OnInit,
   Renderer,
-  OnChanges,
-  SimpleChanges,
   IterableDiffers,
-  KeyValueDiffers
+  KeyValueDiffers, SimpleChanges, OnChanges
 } from '@angular/core';
 import {NgClass} from '@angular/common';
 
 import {BaseFxDirectiveAdapter} from './base-adapter';
-import {BreakPointRegistry} from './../../media-query/breakpoints/break-point-registry';
 import {MediaChange} from '../../media-query/media-change';
 import {MediaMonitor} from '../../media-query/media-monitor';
+import {BreakPointRegistry} from './../../media-query/breakpoints/break-point-registry';
 
 /** NgClass allowed inputs **/
 export type NgClassType = string | string[] | Set<string> | {[klass: string]: any};
@@ -42,7 +40,7 @@ export type NgClassType = string | string[] | Set<string> | {[klass: string]: an
     [ngClass.gt-xs], [ngClass.gt-sm], [ngClass.gt-md], [ngClass.gt-lg]  
   `
 })
-export class ClassDirective extends NgClass implements OnInit, OnChanges, OnDestroy {
+export class ClassDirective extends NgClass implements DoCheck, OnChanges, OnDestroy {
 
   /**
    * Intercept ngClass assignments so we cache the default classes
@@ -98,33 +96,54 @@ export class ClassDirective extends NgClass implements OnInit, OnChanges, OnDest
     this._base = new BaseFxDirectiveAdapter(monitor, _ngEl, _renderer);
   }
 
+  // ******************************************************************
+  // Lifecycle Hookks
+  // ******************************************************************
+
   /**
-   * For @Input changes on the current mq activation property, see onMediaQueryChanges()
+   * For @Input changes on the current mq activation property
    */
   ngOnChanges(changes: SimpleChanges) {
     const changed = this._bpRegistry.items.some(it => {
       return (`ngClass${it.suffix}` in changes) || (`class${it.suffix}` in changes);
     });
-    if (changed || this._base.mqActivation) {
-      this._updateClass();
+    return changed && this._updateClass();
+  }
+
+  /**
+   * For ChangeDetectionStrategy.onPush and ngOnChanges() updates
+   */
+  ngDoCheck() {
+    if (!this._base.hasMQListener) {
+      this.configureMQListener();
     }
+    super.ngDoCheck();
   }
 
+  ngOnDestroy() {
+    this._base.ngOnDestroy();
+  }
+
+  // ******************************************************************
+  // Internal Methods
+  // ******************************************************************
+
   /**
-   * After the initial onChanges, build an mqActivation object that bridges
+   * Build an mqActivation object that bridges
    * mql change events to onMediaQueryChange handlers
    */
-  ngOnInit() {
+  protected configureMQListener() {
     this._base.listenForMediaQueryChanges('class', '', (changes: MediaChange) => {
       this._updateClass(changes.value);
-    });
-    this._updateClass();
-  }
 
-  ngOnDestroy() {
-    this._base.ngOnDestroy();
+      // trigger NgClass::_applyIterableChanges()
+      super.ngDoCheck();
+    });
   }
 
+  /**
+   *
+   */
   protected _updateClass(value?: NgClassType) {
     let clazz = value || this._base.queryInput("class") || '';
     if (this._base.mqActivation) {
diff --git a/src/lib/flexbox/api/style.ts b/src/lib/flexbox/api/style.ts
@@ -10,11 +10,11 @@ import {
   ElementRef,
   Input,
   OnDestroy,
-  OnInit,
-  OnChanges,
+  DoCheck,
   Renderer,
   KeyValueDiffers,
-  SimpleChanges, SecurityContext
+  SimpleChanges, OnChanges,
+  SecurityContext
 } from '@angular/core';
 import {NgStyle} from '@angular/common';
 
@@ -47,7 +47,7 @@ import {
     [ngStyle.gt-xs], [ngStyle.gt-sm], [ngStyle.gt-md], [ngStyle.gt-lg] 
   `
 })
-export class StyleDirective extends NgStyle implements OnInit, OnChanges, OnDestroy {
+export class StyleDirective extends NgStyle implements DoCheck, OnChanges, OnDestroy {
 
   /**
    * Intercept ngStyle assignments so we cache the default styles
@@ -110,32 +110,51 @@ export class StyleDirective extends NgStyle implements OnInit, OnChanges, OnDest
     this._base.cacheInput('style', _ngEl.nativeElement.getAttribute("style"), true);
   }
 
+  // ******************************************************************
+  // Lifecycle Hookks
+  // ******************************************************************
+
   /**
-   * For @Input changes on the current mq activation property, see onMediaQueryChanges()
+   * For @Input changes on the current mq activation property
    */
   ngOnChanges(changes: SimpleChanges) {
     const changed = this._bpRegistry.items.some(it => {
       return (`ngStyle${it.suffix}` in changes) || (`style${it.suffix}` in changes);
     });
-    if (changed || this._base.mqActivation) {
-      this._updateStyle();
-    }
+    return changed && this._updateStyle();
   }
 
   /**
-   * After the initial onChanges, build an mqActivation object that bridges
-   * mql change events to onMediaQueryChange handlers
+   * For ChangeDetectionStrategy.onPush and ngOnChanges() updates
    */
-  ngOnInit() {
-    this._base.listenForMediaQueryChanges('style', '', (changes: MediaChange) => {
-      this._updateStyle(changes.value);
-    });
+  ngDoCheck() {
+    if (!this._base.hasMQListener) {
+      this.configureMQListener();
+    }
+    super.ngDoCheck();
   }
 
   ngOnDestroy() {
     this._base.ngOnDestroy();
   }
 
+  // ******************************************************************
+  // Internal Methods
+  // ******************************************************************
+
+  /**
+     * Build an mqActivation object that bridges
+     * mql change events to onMediaQueryChange handlers
+     */
+    protected configureMQListener() {
+      this._base.listenForMediaQueryChanges('style', '', (changes: MediaChange) => {
+        this._updateStyle(changes.value);
+
+        // trigger NgClass::_applyIterableChanges()
+        super.ngDoCheck();
+      });
+    }
+
   // ************************************************************************
   // Private Internal Methods
   // ************************************************************************
@@ -162,8 +181,14 @@ export class StyleDirective extends NgStyle implements OnInit, OnChanges, OnDest
    */
   protected _buildAdapter(monitor: MediaMonitor, _ngEl: ElementRef, _renderer: Renderer) {
     this._base = new BaseFxDirectiveAdapter(monitor, _ngEl, _renderer);
+    this._buildCacheInterceptor();
+  }
+
 
-    // Build intercept to convert raw strings to ngStyleMap
+  /**
+   * Build intercept to convert raw strings to ngStyleMap
+   */
+  protected _buildCacheInterceptor() {
     let cacheInput = this._base.cacheInput.bind(this._base);
     this._base.cacheInput = (key?: string, source?: any, cacheRaw = false, merge = true) => {
       let styles = this._buildStyleMap(source);
@@ -173,7 +198,6 @@ export class StyleDirective extends NgStyle implements OnInit, OnChanges, OnDest
       cacheInput(key, styles, cacheRaw);
     };
   }
-
   /**
    * Convert raw strings to ngStyleMap; which is required by ngStyle
    * NOTE: Raw string key-value pairs MUST be delimited by `;`
