Skip to content
This repository was archived by the owner on Jan 6, 2025. It is now read-only.

Commit 2c67ed4

Browse files
ThomasBurlesonThomasBurleson
committed
fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush
@see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview fixes #206.
1 parent 980d412 commit 2c67ed4

File tree

6 files changed

+178
-72
lines changed

6 files changed

+178
-72
lines changed

‎src/lib/flexbox/api/base-adapter.spec.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export class MockElementRef extends ElementRef {
2020
describe('BaseFxDirectiveAdapter class', () => {
2121
let component;
2222
beforeEach(() => {
23-
component = new BaseFxDirectiveAdapter(null, new MockElementRef(), null);
23+
component = new BaseFxDirectiveAdapter(null, null, new MockElementRef(), null);
2424
});
2525
describe('cacheInput', () => {
2626
it('should call _cacheInputArray when source is an array', () => {

‎src/lib/flexbox/api/base-adapter.ts

+37-1
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,37 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
import {ElementRef, Renderer} from '@angular/core';
9+
110
import {BaseFxDirective} from './base';
211
import {ResponsiveActivation} from './../responsive/responsive-activation';
312
import {MediaQuerySubscriber} from '../../media-query/media-change';
13+
import {MediaMonitor} from '../../media-query/media-monitor';
14+
415

516
/**
617
* Adapter to the BaseFxDirective abstract class so it can be used via composition.
718
* @see BaseFxDirective
819
*/
920
export class BaseFxDirectiveAdapter extends BaseFxDirective {
21+
22+
/**
23+
* Accessor to determine which @Input property is "active"
24+
* e.g. which property value will be used.
25+
*/
26+
get activeKey() {
27+
let mqa = this._mqActivation;
28+
let key = mqa ? mqa.activatedInputKey : this._baseKey;
29+
// ClassDirective::SimpleChanges uses 'klazz'
30+
// instead of 'class' as a key
31+
return (key === 'class') ? 'klazz' : key;
32+
}
33+
34+
/** Hash map of all @Input keys/values defined/used */
1035
get inputMap() {
1136
return this._inputMap;
1237
}
@@ -18,11 +43,22 @@ export class BaseFxDirectiveAdapter extends BaseFxDirective {
1843
return this._mqActivation;
1944
}
2045

46+
/**
47+
* BaseFxDirectiveAdapter constructor
48+
*/
49+
constructor(protected _baseKey: string, // non-responsive @Input property name
50+
protected _mediaMonitor: MediaMonitor,
51+
protected _elementRef: ElementRef,
52+
protected _renderer: Renderer ) {
53+
super(_mediaMonitor, _elementRef, _renderer);
54+
}
55+
56+
2157
/**
2258
* @see BaseFxDirective._queryInput
2359
*/
2460
queryInput(key) {
25-
return this._queryInput(key);
61+
return key ? this._queryInput(key) : undefined;
2662
}
2763

2864
/**

‎src/lib/flexbox/api/base.ts

+25-19
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,10 @@ export type StyleDefinition = string|{[property: string]: string|number};
2222

2323
/** Abstract base class for the Layout API styling directives. */
2424
export abstract class BaseFxDirective implements OnDestroy {
25-
/**
26-
* Original dom Elements CSS display style
27-
*/
28-
protected _display;
2925

30-
/**
31-
* MediaQuery Activation Tracker
32-
*/
33-
protected _mqActivation: ResponsiveActivation;
34-
35-
/**
36-
* Dictionary of input keys with associated values
37-
*/
38-
protected _inputMap = {};
26+
get hasMediaQueryListener() {
27+
return !!this._mqActivation;
28+
}
3929

4030
/**
4131
*
@@ -46,6 +36,7 @@ export abstract class BaseFxDirective implements OnDestroy {
4636
this._display = this._getDisplayStyle();
4737
}
4838

39+
4940
// *********************************************
5041
// Accessor Methods
5142
// *********************************************
@@ -172,12 +163,15 @@ export abstract class BaseFxDirective implements OnDestroy {
172163
protected _listenForMediaQueryChanges(key: string,
173164
defaultValue: any,
174165
onMediaQueryChange: MediaQuerySubscriber): ResponsiveActivation { // tslint:disable-line:max-line-length
175-
let keyOptions = new KeyOptions(key, defaultValue, this._inputMap);
176-
return this._mqActivation = new ResponsiveActivation(
177-
keyOptions,
178-
this._mediaMonitor,
179-
(change) => onMediaQueryChange.call(this, change)
180-
);
166+
if ( !this._mqActivation ) {
167+
let keyOptions = new KeyOptions(key, defaultValue, this._inputMap);
168+
this._mqActivation = new ResponsiveActivation(
169+
keyOptions,
170+
this._mediaMonitor,
171+
(change) => onMediaQueryChange(change)
172+
);
173+
}
174+
return this._mqActivation;
181175
}
182176

183177
/**
@@ -201,4 +195,16 @@ export abstract class BaseFxDirective implements OnDestroy {
201195
return this._mqActivation.hasKeyValue(key);
202196
}
203197

198+
/** Original dom Elements CSS display style */
199+
protected _display;
200+
201+
/**
202+
* MediaQuery Activation Tracker
203+
*/
204+
protected _mqActivation: ResponsiveActivation;
205+
206+
/**
207+
* Dictionary of input keys with associated values
208+
*/
209+
protected _inputMap = {};
204210
}

‎src/lib/flexbox/api/class.spec.ts

+36-11
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,42 @@ describe('class directive', () => {
6565
});
6666
});
6767

68-
it('should keep existing class selector', () => {
68+
it('should NOT keep the existing class', () => {
6969
fixture = createTestComponent(`
7070
<div class="existing-class" ngClass.xs="xs-class">
7171
</div>
7272
`);
7373

7474
expectNativeEl(fixture).toHaveCssClass('existing-class');
75+
7576
activateMediaQuery('xs');
77+
expectNativeEl(fixture).toHaveCssClass('xs-class');
7678
expectNativeEl(fixture).not.toHaveCssClass('existing-class');
7779

7880
activateMediaQuery('lg');
7981
expectNativeEl(fixture).toHaveCssClass('existing-class');
8082
expectNativeEl(fixture).not.toHaveCssClass('xs-class');
8183
});
8284

85+
86+
it('should keep allow removal of class selector', () => {
87+
fixture = createTestComponent(`
88+
<div
89+
class="existing-class"
90+
[ngClass.xs]="{'xs-class':true, 'existing-class':false}">
91+
</div>
92+
`);
93+
94+
expectNativeEl(fixture).toHaveCssClass('existing-class');
95+
activateMediaQuery('xs');
96+
expectNativeEl(fixture).not.toHaveCssClass('existing-class');
97+
expectNativeEl(fixture).toHaveCssClass('xs-class');
98+
99+
activateMediaQuery('lg');
100+
expectNativeEl(fixture).toHaveCssClass('existing-class');
101+
expectNativeEl(fixture).not.toHaveCssClass('xs-class');
102+
});
103+
83104
it('should keep existing ngClass selector', () => {
84105
// @see documentation for @angular/core ngClass =http://bit.ly/2mz0LAa
85106
fixture = createTestComponent(`
@@ -90,6 +111,7 @@ describe('class directive', () => {
90111
expectNativeEl(fixture).toHaveCssClass('existing-class');
91112
activateMediaQuery('xs');
92113
expectNativeEl(fixture).toHaveCssClass('existing-class');
114+
expectNativeEl(fixture).toHaveCssClass('xs-class');
93115

94116
activateMediaQuery('lg');
95117
expectNativeEl(fixture).toHaveCssClass('existing-class');
@@ -112,21 +134,23 @@ describe('class directive', () => {
112134

113135
it('should work with ngClass object notation', () => {
114136
fixture = createTestComponent(`
115-
<div [ngClass]="{'xs-1': hasXs1, 'xs-3': hasXs3}"
116-
[ngClass.xs]="{'xs-1': hasXs1, 'xs-2': hasXs2}">
137+
<div [ngClass]="{'x1': hasX1, 'x3': hasX3}"
138+
[ngClass.xs]="{'x1': hasX1, 'x2': hasX2}">
117139
</div>
118140
`);
119-
activateMediaQuery('xs');
120-
expectNativeEl(fixture, {hasXs1: true, hasXs2: false}).toHaveCssClass('xs-1');
121-
expectNativeEl(fixture, {hasXs1: true, hasXs2: false}).not.toHaveCssClass('xs-2');
141+
expectNativeEl(fixture, {hasX1: true, hasX2: true, hasX3: true}).toHaveCssClass('x1');
142+
expectNativeEl(fixture, {hasX1: true, hasX2: true, hasX3: true}).not.toHaveCssClass('x2');
143+
expectNativeEl(fixture, {hasX1: true, hasX2: true, hasX3: true}).toHaveCssClass('x3');
122144

123-
expectNativeEl(fixture, {hasXs1: false, hasXs2: true}).toHaveCssClass('xs-2');
124-
expectNativeEl(fixture, {hasXs1: false, hasXs2: true}).not.toHaveCssClass('xs-1');
145+
activateMediaQuery('X');
146+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: false}).toHaveCssClass('x1');
147+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: false}).not.toHaveCssClass('x2');
148+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: false}).not.toHaveCssClass('x3');
125149

126150
activateMediaQuery('md');
127-
expectNativeEl(fixture, {hasXs1: true, hasX2: false, hasXs3: true}).toHaveCssClass('xs-3');
128-
expectNativeEl(fixture, {hasXs1: true, hasX2: false, hasXs3: true}).not.toHaveCssClass('xs-2');
129-
expectNativeEl(fixture, {hasXs1: true, hasX2: false, hasXs3: true}).toHaveCssClass('xs-1');
151+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: true}).toHaveCssClass('x1');
152+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: true}).not.toHaveCssClass('x2');
153+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: true}).toHaveCssClass('x3');
130154
});
131155

132156
it('should work with ngClass array notation', () => {
@@ -151,6 +175,7 @@ describe('class directive', () => {
151175
export class TestClassComponent implements OnInit {
152176
hasXs1: boolean;
153177
hasXs2: boolean;
178+
hasXs3: boolean;
154179

155180
constructor(private media: ObservableMedia) {
156181
}

‎src/lib/flexbox/api/class.ts

+38-22
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,15 @@ import {
99
Directive,
1010
ElementRef,
1111
Input,
12+
DoCheck,
1213
OnDestroy,
13-
OnInit,
1414
Renderer,
15-
OnChanges,
16-
SimpleChanges,
1715
IterableDiffers,
18-
KeyValueDiffers
16+
KeyValueDiffers, SimpleChanges, OnChanges
1917
} from '@angular/core';
2018
import {NgClass} from '@angular/common';
2119

2220
import {BaseFxDirectiveAdapter} from './base-adapter';
23-
import {BreakPointRegistry} from './../../media-query/breakpoints/break-point-registry';
2421
import {MediaChange} from '../../media-query/media-change';
2522
import {MediaMonitor} from '../../media-query/media-monitor';
2623

@@ -42,7 +39,7 @@ export type NgClassType = string | string[] | Set<string> | {[klass: string]: an
4239
[ngClass.gt-xs], [ngClass.gt-sm], [ngClass.gt-md], [ngClass.gt-lg]
4340
`
4441
})
45-
export class ClassDirective extends NgClass implements OnInit, OnChanges, OnDestroy {
42+
export class ClassDirective extends NgClass implements DoCheck, OnChanges, OnDestroy {
4643

4744
/**
4845
* Intercept ngClass assignments so we cache the default classes
@@ -72,7 +69,7 @@ export class ClassDirective extends NgClass implements OnInit, OnChanges, OnDest
7269
@Input('ngClass.gt-lg') set ngClassGtLg(val: NgClassType) { this._base.cacheInput('classGtLg', val, true); };
7370

7471
/** Deprecated selectors */
75-
@Input('class') set classBase(val: NgClassType) { this._base.cacheInput('class', val, true); }
72+
@Input('class') set klazz(val: NgClassType) { this._base.cacheInput('class', val, true); }
7673
@Input('class.xs') set classXs(val: NgClassType) { this._base.cacheInput('classXs', val, true); }
7774
@Input('class.sm') set classSm(val: NgClassType) { this._base.cacheInput('classSm', val, true); };
7875
@Input('class.md') set classMd(val: NgClassType) { this._base.cacheInput('classMd', val, true);};
@@ -91,42 +88,61 @@ export class ClassDirective extends NgClass implements OnInit, OnChanges, OnDest
9188

9289
/* tslint:enable */
9390
constructor(protected monitor: MediaMonitor,
94-
protected _bpRegistry: BreakPointRegistry,
9591
_iterableDiffers: IterableDiffers, _keyValueDiffers: KeyValueDiffers,
9692
_ngEl: ElementRef, _renderer: Renderer) {
9793
super(_iterableDiffers, _keyValueDiffers, _ngEl, _renderer);
98-
this._base = new BaseFxDirectiveAdapter(monitor, _ngEl, _renderer);
94+
this._base = new BaseFxDirectiveAdapter("class", monitor, _ngEl, _renderer);
9995
}
10096

97+
// ******************************************************************
98+
// Lifecycle Hookks
99+
// ******************************************************************
100+
101101
/**
102-
* For @Input changes on the current mq activation property, see onMediaQueryChanges()
102+
* For @Input changes on the current mq activation property
103103
*/
104104
ngOnChanges(changes: SimpleChanges) {
105-
const changed = this._bpRegistry.items.some(it => {
106-
return (`ngClass${it.suffix}` in changes) || (`class${it.suffix}` in changes);
107-
});
108-
if (changed || this._base.mqActivation) {
105+
if (this._base.activeKey in changes) {
109106
this._updateClass();
110107
}
111108
}
112109

113110
/**
114-
* After the initial onChanges, build an mqActivation object that bridges
115-
* mql change events to onMediaQueryChange handlers
111+
* For ChangeDetectionStrategy.onPush and ngOnChanges() updates
116112
*/
117-
ngOnInit() {
118-
this._base.listenForMediaQueryChanges('class', '', (changes: MediaChange) => {
119-
this._updateClass(changes.value);
120-
});
121-
this._updateClass();
113+
ngDoCheck() {
114+
if (!this._base.hasMediaQueryListener) {
115+
this._configureMQListener();
116+
}
117+
super.ngDoCheck();
122118
}
123119

124120
ngOnDestroy() {
125121
this._base.ngOnDestroy();
126122
}
127123

124+
// ******************************************************************
125+
// Internal Methods
126+
// ******************************************************************
127+
128+
/**
129+
* Build an mqActivation object that bridges
130+
* mql change events to onMediaQueryChange handlers
131+
*/
132+
protected _configureMQListener() {
133+
this._base.listenForMediaQueryChanges('class', '', (changes: MediaChange) => {
134+
this._updateClass(changes.value);
135+
136+
// trigger NgClass::_applyIterableChanges()
137+
super.ngDoCheck();
138+
});
139+
}
140+
141+
/**
142+
*
143+
*/
128144
protected _updateClass(value?: NgClassType) {
129-
let clazz = value || this._base.queryInput("class") || '';
145+
let clazz = value || this._base.queryInput('class') || '';
130146
if (this._base.mqActivation) {
131147
clazz = this._base.mqActivation.activatedInput;
132148
}

0 commit comments

Comments
 (0)