Skip to content

Commit

Permalink
fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush
Browse files Browse the repository at this point in the history 
@see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview

fixes #206.
  • Loading branch information
@ThomasBurleson
ThomasBurleson committed Mar 19, 2017
1 parent 980d412 commit cf60c0a
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 74 deletions.
2 changes: 1 addition & 1 deletion src/lib/flexbox/api/base-adapter.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
32 changes: 31 additions & 1 deletion src/lib/flexbox/api/base-adapter.ts
View file
Original file line number Diff line number Diff line change
@@ -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;
}
Expand All @@ -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;
}

/**
Expand Down
46 changes: 27 additions & 19 deletions src/lib/flexbox/api/base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
*
Expand All @@ -46,6 +36,7 @@ export abstract class BaseFxDirective implements OnDestroy {
this._display = this._getDisplayStyle();
}


// *********************************************
// Accessor Methods
// *********************************************
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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 = {};
}
47 changes: 36 additions & 11 deletions src/lib/flexbox/api/class.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,42 @@ 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).toHaveCssClass('xs-class');
expectNativeEl(fixture).not.toHaveCssClass('existing-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');
expectNativeEl(fixture).not.toHaveCssClass('xs-class');
});

it('should keep existing ngClass selector', () => {
// @see documentation for @angular/core ngClass =http://bit.ly/2mz0LAa
fixture = createTestComponent(`
Expand All @@ -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');
Expand All @@ -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', () => {
Expand All @@ -151,6 +175,7 @@ describe('class directive', () => {
export class TestClassComponent implements OnInit {
hasXs1: boolean;
hasXs2: boolean;
hasXs3: boolean;

constructor(private media: ObservableMedia) {
}
Expand Down
59 changes: 37 additions & 22 deletions src/lib/flexbox/api/class.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -42,7 +39,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
Expand Down Expand Up @@ -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);};
Expand All @@ -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 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) {
Expand Down

0 comments on commit cf60c0a

Please sign in to comment.