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 22, 2017
1 parent 980d412 commit d2517b8
Show file tree
Hide file tree
Showing 6 changed files with 225 additions and 80 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
38 changes: 37 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,37 @@
/**
* @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 {

/**
* Accessor to determine which @Input property is "active"
* e.g. which property value will be used.
*/
get activeKey() {
let mqa = this._mqActivation;
let key = mqa ? mqa.activatedInputKey : this._baseKey;
// ClassDirective::SimpleChanges uses 'klazz'
// instead of 'class' as a key
return (key === 'class') ? 'klazz' : key;
}

/** Hash map of all @Input keys/values defined/used */
get inputMap() {
return this._inputMap;
}
Expand All @@ -18,11 +43,22 @@ export class BaseFxDirectiveAdapter extends BaseFxDirective {
return this._mqActivation;
}

/**
* BaseFxDirectiveAdapter constructor
*/
constructor(protected _baseKey: string, // non-responsive @Input property name
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
44 changes: 25 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 hasMediaQueryListener() {
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(change)
);
}
return this._mqActivation;
}

/**
Expand All @@ -201,4 +195,16 @@ 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 = {};
}
71 changes: 58 additions & 13 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,50 @@ describe('class directive', () => {
});
});

it('should keep existing class selector', () => {
it('should merge `ngClass` values with any `class` values', () => {
fixture = createTestComponent(`
<div class="existing-class" ngClass.xs="xs-class">
<div class="class0" ngClass="class1 class2">
</div>
`);

expectNativeEl(fixture).toHaveCssClass('class0');
expectNativeEl(fixture).toHaveCssClass('class1');
expectNativeEl(fixture).toHaveCssClass('class2');
});

it('should keep the raw existing `class` with responsive updates', () => {
fixture = createTestComponent(`
<div class="existing-class" ngClass="class1" ngClass.xs="xs-class">
</div>
`);

expectNativeEl(fixture).toHaveCssClass('existing-class');
expectNativeEl(fixture).toHaveCssClass('class1');

activateMediaQuery('xs');
expectNativeEl(fixture).toHaveCssClass('xs-class');
expectNativeEl(fixture).toHaveCssClass('existing-class');
expectNativeEl(fixture).not.toHaveCssClass('class1');

activateMediaQuery('lg');
expectNativeEl(fixture).not.toHaveCssClass('xs-class');
expectNativeEl(fixture).toHaveCssClass('existing-class');
expectNativeEl(fixture).toHaveCssClass('class1');
});


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 @@ -83,15 +118,22 @@ describe('class directive', () => {
it('should keep existing ngClass selector', () => {
// @see documentation for @angular/core ngClass =http://bit.ly/2mz0LAa
fixture = createTestComponent(`
<div ngClass="existing-class" ngClass.xs="existing-class xs-class">
<div class="always"
ngClass="existing-class"
ngClass.xs="existing-class xs-class">
</div>
`);

expectNativeEl(fixture).toHaveCssClass('always');
expectNativeEl(fixture).toHaveCssClass('existing-class');

activateMediaQuery('xs');
expectNativeEl(fixture).toHaveCssClass('always');
expectNativeEl(fixture).toHaveCssClass('existing-class');
expectNativeEl(fixture).toHaveCssClass('xs-class');

activateMediaQuery('lg');
expectNativeEl(fixture).toHaveCssClass('always');
expectNativeEl(fixture).toHaveCssClass('existing-class');
expectNativeEl(fixture).not.toHaveCssClass('xs-class');
});
Expand All @@ -112,21 +154,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 +195,7 @@ describe('class directive', () => {
export class TestClassComponent implements OnInit {
hasXs1: boolean;
hasXs2: boolean;
hasXs3: boolean;

constructor(private media: ObservableMedia) {
}
Expand Down
Loading

0 comments on commit d2517b8

Please sign in to comment.