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

Commit 167cb8c

Browse files
ThomasBurlesonThomasBurleson
committed
fix(ngClass,ngStyle): support proper API usages and ChangeDetectionStrategy.OnPush strategies
* fix(ngClass): properly differentiate between `ngClass` and `class` api usages * `class.<xxx>` are destructive overrides (as seen in `NgClass::klass`) * `ngClass.<xxx>` use iterables to merge/enable/disable 0...n classes * fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush * Add support for both **OnPush** change detection strategies and for @input changes. > @see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview fixes #206.
1 parent 980d412 commit 167cb8c

File tree

6 files changed

+308
-118
lines changed

6 files changed

+308
-118
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

+81-13
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,73 @@ describe('class directive', () => {
6565
});
6666
});
6767

68-
it('should keep existing class selector', () => {
68+
it('should merge `ngClass` values with any `class` values', () => {
6969
fixture = createTestComponent(`
70-
<div class="existing-class" ngClass.xs="xs-class">
70+
<div class="class0" ngClass="class1 class2">
71+
</div>
72+
`);
73+
74+
expectNativeEl(fixture).toHaveCssClass('class0');
75+
expectNativeEl(fixture).toHaveCssClass('class1');
76+
expectNativeEl(fixture).toHaveCssClass('class2');
77+
});
78+
79+
it('should override base `class` values with responsive values', () => {
80+
fixture = createTestComponent(`
81+
<div class="class0"
82+
class.xs="class1 class2"
83+
ngClass.xs="what">
84+
</div>
85+
`);
86+
87+
expectNativeEl(fixture).toHaveCssClass('class0');
88+
expectNativeEl(fixture).not.toHaveCssClass('class1');
89+
expectNativeEl(fixture).not.toHaveCssClass('class2');
90+
91+
activateMediaQuery('xs');
92+
expectNativeEl(fixture).not.toHaveCssClass('class0');
93+
expectNativeEl(fixture).toHaveCssClass('class1');
94+
expectNativeEl(fixture).toHaveCssClass('class2');
95+
96+
// activateMediaQuery('lg');
97+
// expectNativeEl(fixture).toHaveCssClass('class0');
98+
// expectNativeEl(fixture).not.toHaveCssClass('class1');
99+
// expectNativeEl(fixture).not.toHaveCssClass('class2');
100+
});
101+
102+
it('should keep the raw existing `class` with responsive updates', () => {
103+
fixture = createTestComponent(`
104+
<div class="existing-class" ngClass="class1" ngClass.xs="xs-class">
105+
</div>
106+
`);
107+
108+
expectNativeEl(fixture).toHaveCssClass('existing-class');
109+
expectNativeEl(fixture).toHaveCssClass('class1');
110+
111+
activateMediaQuery('xs');
112+
expectNativeEl(fixture).toHaveCssClass('xs-class');
113+
expectNativeEl(fixture).toHaveCssClass('existing-class');
114+
expectNativeEl(fixture).not.toHaveCssClass('class1');
115+
116+
activateMediaQuery('lg');
117+
expectNativeEl(fixture).not.toHaveCssClass('xs-class');
118+
expectNativeEl(fixture).toHaveCssClass('existing-class');
119+
expectNativeEl(fixture).toHaveCssClass('class1');
120+
});
121+
122+
123+
it('should keep allow removal of class selector', () => {
124+
fixture = createTestComponent(`
125+
<div
126+
class="existing-class"
127+
[ngClass.xs]="{'xs-class':true, 'existing-class':false}">
71128
</div>
72129
`);
73130

74131
expectNativeEl(fixture).toHaveCssClass('existing-class');
75132
activateMediaQuery('xs');
76133
expectNativeEl(fixture).not.toHaveCssClass('existing-class');
134+
expectNativeEl(fixture).toHaveCssClass('xs-class');
77135

78136
activateMediaQuery('lg');
79137
expectNativeEl(fixture).toHaveCssClass('existing-class');
@@ -83,15 +141,22 @@ describe('class directive', () => {
83141
it('should keep existing ngClass selector', () => {
84142
// @see documentation for @angular/core ngClass =http://bit.ly/2mz0LAa
85143
fixture = createTestComponent(`
86-
<div ngClass="existing-class" ngClass.xs="existing-class xs-class">
144+
<div class="always"
145+
ngClass="existing-class"
146+
ngClass.xs="existing-class xs-class">
87147
</div>
88148
`);
89149

150+
expectNativeEl(fixture).toHaveCssClass('always');
90151
expectNativeEl(fixture).toHaveCssClass('existing-class');
152+
91153
activateMediaQuery('xs');
154+
expectNativeEl(fixture).toHaveCssClass('always');
92155
expectNativeEl(fixture).toHaveCssClass('existing-class');
156+
expectNativeEl(fixture).toHaveCssClass('xs-class');
93157

94158
activateMediaQuery('lg');
159+
expectNativeEl(fixture).toHaveCssClass('always');
95160
expectNativeEl(fixture).toHaveCssClass('existing-class');
96161
expectNativeEl(fixture).not.toHaveCssClass('xs-class');
97162
});
@@ -112,21 +177,23 @@ describe('class directive', () => {
112177

113178
it('should work with ngClass object notation', () => {
114179
fixture = createTestComponent(`
115-
<div [ngClass]="{'xs-1': hasXs1, 'xs-3': hasXs3}"
116-
[ngClass.xs]="{'xs-1': hasXs1, 'xs-2': hasXs2}">
180+
<div [ngClass]="{'x1': hasX1, 'x3': hasX3}"
181+
[ngClass.xs]="{'x1': hasX1, 'x2': hasX2}">
117182
</div>
118183
`);
119-
activateMediaQuery('xs');
120-
expectNativeEl(fixture, {hasXs1: true, hasXs2: false}).toHaveCssClass('xs-1');
121-
expectNativeEl(fixture, {hasXs1: true, hasXs2: false}).not.toHaveCssClass('xs-2');
184+
expectNativeEl(fixture, {hasX1: true, hasX2: true, hasX3: true}).toHaveCssClass('x1');
185+
expectNativeEl(fixture, {hasX1: true, hasX2: true, hasX3: true}).not.toHaveCssClass('x2');
186+
expectNativeEl(fixture, {hasX1: true, hasX2: true, hasX3: true}).toHaveCssClass('x3');
122187

123-
expectNativeEl(fixture, {hasXs1: false, hasXs2: true}).toHaveCssClass('xs-2');
124-
expectNativeEl(fixture, {hasXs1: false, hasXs2: true}).not.toHaveCssClass('xs-1');
188+
activateMediaQuery('X');
189+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: false}).toHaveCssClass('x1');
190+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: false}).not.toHaveCssClass('x2');
191+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: false}).not.toHaveCssClass('x3');
125192

126193
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');
194+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: true}).toHaveCssClass('x1');
195+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: true}).not.toHaveCssClass('x2');
196+
expectNativeEl(fixture, {hasX1: true, hasX2: false, hasX3: true}).toHaveCssClass('x3');
130197
});
131198

132199
it('should work with ngClass array notation', () => {
@@ -151,6 +218,7 @@ describe('class directive', () => {
151218
export class TestClassComponent implements OnInit {
152219
hasXs1: boolean;
153220
hasXs2: boolean;
221+
hasXs3: boolean;
154222

155223
constructor(private media: ObservableMedia) {
156224
}

0 commit comments

Comments
 (0)