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 c05e6b8
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 62 deletions.
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,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');
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
51 changes: 35 additions & 16 deletions src/lib/flexbox/api/class.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
56 changes: 40 additions & 16 deletions src/lib/flexbox/api/style.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
// ************************************************************************
Expand All @@ -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);
Expand All @@ -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 `;`
Expand Down

0 comments on commit c05e6b8

Please sign in to comment.