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

Commit b399886

Browse files
ThomasBurlesonThomasBurleson
committed
fix(api, class): remove ‘[class]’ selector
The host `class` attribute should be considered static and should not be used as a ClassDirective selector. This means that with responsive APIs, the ClassDirective will not be instantiated for elements using only the `class` attribute. * Use the `class` attribute as a responsive fallback value only. * Improve support of the ngClass directive for string and object assignments. Fixes #393.
1 parent 2cb3714 commit b399886

File tree

3 files changed

+57
-35
lines changed

3 files changed

+57
-35
lines changed

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

+16-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ export class BaseFxDirectiveAdapter extends BaseFxDirective {
7373
} else if (typeof source === 'string') {
7474
this._cacheInputString(key, source);
7575
} else {
76-
throw new Error('Invalid class value provided. Did you want to cache the raw value?');
76+
throw new Error(
77+
`Invalid class value '${key}' provided. Did you want to cache the raw value?`
78+
);
7779
}
7880
}
7981

@@ -124,4 +126,17 @@ export class BaseFxDirectiveAdapter extends BaseFxDirective {
124126
protected _cacheInputString(key = '', source?: string) {
125127
this._inputMap[key] = source;
126128
}
129+
130+
/**
131+
* Does this directive have 1 or more responsive keys defined
132+
* Note: we exclude the 'baseKey' key (which is NOT considered responsive)
133+
*/
134+
public get usesResponsiveAPI() {
135+
const numKeys = Object.keys(this._inputMap).length;
136+
let hasKeys = numKeys > 0;
137+
if ((numKeys == 1) && !!this._inputMap[this._baseKey]) {
138+
hasKeys = false;
139+
}
140+
return hasKeys;
141+
}
127142
}

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

+30-7
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,44 @@ describe('class directive', () => {
6565
});
6666

6767
it('should override base `class` values with responsive values', () => {
68-
createTestComponent(`<div class="class0" class.xs="class1 class2" ngClass.xs="what"></div>`);
68+
createTestComponent(`<div class="class0" ngClass.xs="what class2"></div>`);
6969

7070
expectNativeEl(fixture).toHaveCssClass('class0');
71-
expectNativeEl(fixture).not.toHaveCssClass('class1');
71+
expectNativeEl(fixture).not.toHaveCssClass('what');
7272
expectNativeEl(fixture).not.toHaveCssClass('class2');
7373

74+
// the CSS classes listed in the string (space delimited) are added,
75+
matchMedia.activate('xs');
76+
expectNativeEl(fixture).toHaveCssClass('class0');
77+
expectNativeEl(fixture).toHaveCssClass('what');
78+
expectNativeEl(fixture).toHaveCssClass('class2');
79+
80+
matchMedia.activate('lg');
81+
expectNativeEl(fixture).toHaveCssClass('class0');
82+
expectNativeEl(fixture).not.toHaveCssClass('what');
83+
expectNativeEl(fixture).not.toHaveCssClass('class2');
84+
});
85+
86+
it('should override base `class` values with responsive values', () => {
87+
createTestComponent(`
88+
<div class="class0" [ngClass.xs]="{'what':true, 'class2':true, 'class0':false}"></div>
89+
`);
90+
91+
expectNativeEl(fixture).toHaveCssClass('class0');
92+
expectNativeEl(fixture).not.toHaveCssClass('what');
93+
expectNativeEl(fixture).not.toHaveCssClass('class2');
94+
95+
// Object keys are CSS classes that get added when the expression given in
96+
// the value evaluates to a truthy value, otherwise they are removed.
7497
matchMedia.activate('xs');
7598
expectNativeEl(fixture).not.toHaveCssClass('class0');
76-
expectNativeEl(fixture).toHaveCssClass('class1');
99+
expectNativeEl(fixture).toHaveCssClass('what');
77100
expectNativeEl(fixture).toHaveCssClass('class2');
78101

79-
// matchMedia.activate('lg');
80-
// expectNativeEl(fixture).toHaveCssClass('class0');
81-
// expectNativeEl(fixture).not.toHaveCssClass('class1');
82-
// expectNativeEl(fixture).not.toHaveCssClass('class2');
102+
matchMedia.activate('lg');
103+
expectNativeEl(fixture).toHaveCssClass('class0');
104+
expectNativeEl(fixture).not.toHaveCssClass('what');
105+
expectNativeEl(fixture).not.toHaveCssClass('class2');
83106
});
84107

85108
it('should keep the raw existing `class` with responsive updates', () => {

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

+11-27
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export type NgClassType = string | string[] | Set<string> | {[klass: string]: an
3535
*/
3636
@Directive({
3737
selector: `
38-
[class], [class.xs], [class.sm], [class.md], [class.lg], [class.xl],
38+
[class.xs], [class.sm], [class.md], [class.lg], [class.xl],
3939
[class.lt-sm], [class.lt-md], [class.lt-lg], [class.lt-xl],
4040
[class.gt-xs], [class.gt-sm], [class.gt-md], [class.gt-lg],
4141
@@ -77,18 +77,6 @@ export class ClassDirective extends BaseFxDirective
7777

7878
/** Deprecated selectors */
7979

80-
/**
81-
* Base class selector values get applied immediately and are considered destructive overwrites to
82-
* all previous class assignments
83-
*
84-
* Delegate to NgClass:klass setter and cache value for base fallback from responsive APIs.
85-
*/
86-
@Input('class')
87-
set classBase(val: string) {
88-
this._classAdapter.cacheInput('_rawClass', val, true);
89-
this._ngClassInstance.klass = val;
90-
}
91-
9280
@Input('class.xs') set classXs(val: NgClassType) { this._classAdapter.cacheInput('classXs', val, true); }
9381
@Input('class.sm') set classSm(val: NgClassType) { this._classAdapter.cacheInput('classSm', val, true); }
9482
@Input('class.md') set classMd(val: NgClassType) { this._classAdapter.cacheInput('classMd', val, true); }
@@ -105,14 +93,6 @@ export class ClassDirective extends BaseFxDirective
10593
@Input('class.gt-md') set classGtMd(val: NgClassType) { this._classAdapter.cacheInput('classGtMd', val, true); }
10694
@Input('class.gt-lg') set classGtLg(val: NgClassType) { this._classAdapter.cacheInput('classGtLg', val, true); }
10795

108-
/**
109-
* Initial value of the `class` attribute; used as
110-
* fallback and will be merged with nay `ngClass` values
111-
*/
112-
get initialClasses() : string {
113-
return this._classAdapter.queryInput('_rawClass') || "";
114-
}
115-
11696
/* tslint:enable */
11797
constructor(protected monitor: MediaMonitor,
11898
_ngEl: ElementRef, _renderer: Renderer2,
@@ -121,8 +101,9 @@ export class ClassDirective extends BaseFxDirective
121101

122102
super(monitor, _ngEl, _renderer);
123103

124-
this._classAdapter = new BaseFxDirectiveAdapter('class', monitor, _ngEl, _renderer);
125104
this._ngClassAdapter = new BaseFxDirectiveAdapter('ngClass', monitor, _ngEl, _renderer);
105+
this._classAdapter = new BaseFxDirectiveAdapter('class', monitor, _ngEl, _renderer);
106+
this._classAdapter.cacheInput('class', _ngEl.nativeElement.getAttribute('class') || '', true);
126107

127108
// Create an instance NgClass Directive instance only if `ngClass=""` has NOT been defined on
128109
// the same host element; since the responsive variations may be defined...
@@ -157,7 +138,9 @@ export class ClassDirective extends BaseFxDirective
157138
if (!this._classAdapter.hasMediaQueryListener) {
158139
this._configureMQListener();
159140
}
160-
this._ngClassInstance.ngDoCheck();
141+
if ( this._ngClassInstance) {
142+
this._ngClassInstance.ngDoCheck();
143+
}
161144
}
162145

163146
ngOnDestroy() {
@@ -175,11 +158,12 @@ export class ClassDirective extends BaseFxDirective
175158
* mql change events to onMediaQueryChange handlers
176159
*/
177160
protected _configureMQListener() {
178-
this._classAdapter.listenForMediaQueryChanges('class', '', (changes: MediaChange) => {
161+
const value = this._classAdapter.queryInput('class');
162+
this._classAdapter.listenForMediaQueryChanges('class', value, (changes: MediaChange) => {
179163
this._updateKlass(changes.value);
180164
});
181165

182-
this._ngClassAdapter.listenForMediaQueryChanges('ngClass', '', (changes: MediaChange) => {
166+
this._ngClassAdapter.listenForMediaQueryChanges('ngClass', value, (changes: MediaChange) => {
183167
this._updateNgClass(changes.value);
184168
this._ngClassInstance.ngDoCheck(); // trigger NgClass::_applyIterableChanges()
185169
});
@@ -190,11 +174,11 @@ export class ClassDirective extends BaseFxDirective
190174
* ::ngDoCheck() is not needed
191175
*/
192176
protected _updateKlass(value?: NgClassType) {
193-
let klass = value || this._classAdapter.queryInput('class') || '';
177+
let klass = value || this._classAdapter.queryInput('class');
194178
if (this._classAdapter.mqActivation) {
195179
klass = this._classAdapter.mqActivation.activatedInput;
196180
}
197-
this._ngClassInstance.klass = klass || this.initialClasses;
181+
this._ngClassInstance.klass = klass;
198182
}
199183

200184
/**

0 commit comments

Comments
 (0)