From c0cb733a280f89fc6d55e2aeac34ea0493c9d834 Mon Sep 17 00:00:00 2001 From: Thomas Burleson Date: Fri, 17 Mar 2017 11:59:39 -0500 Subject: [PATCH] fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush @see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview fixes #206. --- src/lib/flexbox/api/base-adapter.spec.ts | 2 +- src/lib/flexbox/api/base-adapter.ts | 32 +++++++++++- src/lib/flexbox/api/base.ts | 46 ++++++++++-------- src/lib/flexbox/api/class.spec.ts | 47 +++++++++++++----- src/lib/flexbox/api/class.ts | 59 +++++++++++++--------- src/lib/flexbox/api/style.ts | 62 ++++++++++++++++-------- 6 files changed, 174 insertions(+), 74 deletions(-) diff --git a/src/lib/flexbox/api/base-adapter.spec.ts b/src/lib/flexbox/api/base-adapter.spec.ts index 1ed4d650a..6790472ce 100644 --- a/src/lib/flexbox/api/base-adapter.spec.ts +++ b/src/lib/flexbox/api/base-adapter.spec.ts @@ -20,7 +20,7 @@ export class MockElementRef extends ElementRef { describe('BaseFxDirectiveAdapter class', () => { let component; beforeEach(() => { - component = new BaseFxDirectiveAdapter(null, new MockElementRef(), null); + component = new BaseFxDirectiveAdapter(null, null, new MockElementRef(), null); }); describe('cacheInput', () => { it('should call _cacheInputArray when source is an array', () => { diff --git a/src/lib/flexbox/api/base-adapter.ts b/src/lib/flexbox/api/base-adapter.ts index 73c6340da..993807690 100644 --- a/src/lib/flexbox/api/base-adapter.ts +++ b/src/lib/flexbox/api/base-adapter.ts @@ -1,12 +1,31 @@ +/** + * @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 {ElementRef, Renderer} from '@angular/core'; + import {BaseFxDirective} from './base'; import {ResponsiveActivation} from './../responsive/responsive-activation'; import {MediaQuerySubscriber} from '../../media-query/media-change'; +import {MediaMonitor} from '../../media-query/media-monitor'; + /** * Adapter to the BaseFxDirective abstract class so it can be used via composition. * @see BaseFxDirective */ export class BaseFxDirectiveAdapter extends BaseFxDirective { + + get activeKey() { + let mqa = this._mqActivation; + let key = mqa ? mqa.activatedInputKey : this._baseKey; + // SimpleChanges uses 'klazz' instead of 'class' as a key + return (key == 'class') ? 'klazz' : key; + } + get inputMap() { return this._inputMap; } @@ -18,11 +37,22 @@ export class BaseFxDirectiveAdapter extends BaseFxDirective { return this._mqActivation; } + /** + * + */ + constructor(protected _baseKey: string, + protected _mediaMonitor: MediaMonitor, + protected _elementRef: ElementRef, + protected _renderer: Renderer ) { + super(_mediaMonitor, _elementRef, _renderer); + } + + /** * @see BaseFxDirective._queryInput */ queryInput(key) { - return this._queryInput(key); + return key ? this._queryInput(key) : undefined; } /** diff --git a/src/lib/flexbox/api/base.ts b/src/lib/flexbox/api/base.ts index dc7a2e8bf..9dc4de6d7 100644 --- 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 index 68cb2ac47..c27b88a1f 100644 --- a/src/lib/flexbox/api/class.spec.ts +++ b/src/lib/flexbox/api/class.spec.ts @@ -65,14 +65,16 @@ describe('class directive', () => { }); }); - it('should keep existing class selector', () => { + it('should NOT keep the existing class', () => { fixture = createTestComponent(`
`); expectNativeEl(fixture).toHaveCssClass('existing-class'); + activateMediaQuery('xs'); + expectNativeEl(fixture).toHaveCssClass('xs-class'); expectNativeEl(fixture).not.toHaveCssClass('existing-class'); activateMediaQuery('lg'); @@ -80,6 +82,25 @@ describe('class directive', () => { expectNativeEl(fixture).not.toHaveCssClass('xs-class'); }); + + it('should keep allow removal of class selector', () => { + fixture = createTestComponent(` +
+
+ `); + + 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 existing ngClass selector', () => { // @see documentation for @angular/core ngClass =http://bit.ly/2mz0LAa fixture = createTestComponent(` @@ -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(` -
+
`); - 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 index 5daee8752..8e514cf07 100644 --- a/src/lib/flexbox/api/class.ts +++ b/src/lib/flexbox/api/class.ts @@ -9,18 +9,15 @@ 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'; @@ -42,7 +39,7 @@ export type NgClassType = string | string[] | Set | {[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 @@ -72,7 +69,7 @@ export class ClassDirective extends NgClass implements OnInit, OnChanges, OnDest @Input('ngClass.gt-lg') set ngClassGtLg(val: NgClassType) { this._base.cacheInput('classGtLg', val, true); }; /** Deprecated selectors */ - @Input('class') set classBase(val: NgClassType) { this._base.cacheInput('class', val, true); } + @Input('class') set klazz(val: NgClassType) { this._base.cacheInput('class', val, true); } @Input('class.xs') set classXs(val: NgClassType) { this._base.cacheInput('classXs', val, true); } @Input('class.sm') set classSm(val: NgClassType) { this._base.cacheInput('classSm', val, true); }; @Input('class.md') set classMd(val: NgClassType) { this._base.cacheInput('classMd', val, true);}; @@ -91,40 +88,58 @@ export class ClassDirective extends NgClass implements OnInit, OnChanges, OnDest /* tslint:enable */ constructor(protected monitor: MediaMonitor, - protected _bpRegistry: BreakPointRegistry, _iterableDiffers: IterableDiffers, _keyValueDiffers: KeyValueDiffers, _ngEl: ElementRef, _renderer: Renderer) { super(_iterableDiffers, _keyValueDiffers, _ngEl, _renderer); - this._base = new BaseFxDirectiveAdapter(monitor, _ngEl, _renderer); + this._base = new BaseFxDirectiveAdapter("class", 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(); + const changed = (this._base.activeKey in changes); + return !changes || (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 index 13aab3a1e..943c350d2 100644 --- 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,49 @@ 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(); - } + const changed = (this._base.activeKey in changes); + 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 // ************************************************************************ @@ -161,9 +178,15 @@ export class StyleDirective extends NgStyle implements OnInit, OnChanges, OnDest * which property value should be used for the style update */ protected _buildAdapter(monitor: MediaMonitor, _ngEl: ElementRef, _renderer: Renderer) { - this._base = new BaseFxDirectiveAdapter(monitor, _ngEl, _renderer); + this._base = new BaseFxDirectiveAdapter("style", 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 +196,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 `;`