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

Commit

Permalink
The host class attribute should be considered static and should not…
Browse files Browse the repository at this point in the history 
… be used as a ClassDirective selector. This means that without associated 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.
  • Loading branch information
@ThomasBurleson
ThomasBurleson committed Aug 28, 2017
1 parent 64a7c50 commit 06eea6c
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 35 deletions.
17 changes: 16 additions & 1 deletion src/lib/api/core/base-adapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ export class BaseFxDirectiveAdapter extends BaseFxDirective {
} else if (typeof source === 'string') {
this._cacheInputString(key, source);
} else {
throw new Error('Invalid class value provided. Did you want to cache the raw value?');
throw new Error(
`Invalid class value '${key}' provided. Did you want to cache the raw value?`
);
}
}

Expand Down Expand Up @@ -124,4 +126,17 @@ export class BaseFxDirectiveAdapter extends BaseFxDirective {
protected _cacheInputString(key = '', source?: string) {
this._inputMap[key] = source;
}

/**
* Does this directive have 1 or more responsive keys defined
* Note: we exclude the 'baseKey' key (which is NOT considered responsive)
*/
public get usesResponsiveAPI() {
const numKeys = Object.keys(this._inputMap).length;
let hasKeys = numKeys > 0;
if ((numKeys == 1) && !!this._inputMap[this._baseKey]) {
hasKeys = false;
}
return hasKeys;
}
}
37 changes: 30 additions & 7 deletions src/lib/api/ext/class.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,44 @@ describe('class directive', () => {
});

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

expectNativeEl(fixture).toHaveCssClass('class0');
expectNativeEl(fixture).not.toHaveCssClass('class1');
expectNativeEl(fixture).not.toHaveCssClass('what');
expectNativeEl(fixture).not.toHaveCssClass('class2');

// the CSS classes listed in the string (space delimited) are added,
matchMedia.activate('xs');
expectNativeEl(fixture).toHaveCssClass('class0');
expectNativeEl(fixture).toHaveCssClass('what');
expectNativeEl(fixture).toHaveCssClass('class2');

matchMedia.activate('lg');
expectNativeEl(fixture).toHaveCssClass('class0');
expectNativeEl(fixture).not.toHaveCssClass('what');
expectNativeEl(fixture).not.toHaveCssClass('class2');
});

it('should override base `class` values with responsive values', () => {
createTestComponent(`
<div class="class0" [ngClass.xs]="{'what':true, 'class2':true, 'class0':false}"></div>
`);

expectNativeEl(fixture).toHaveCssClass('class0');
expectNativeEl(fixture).not.toHaveCssClass('what');
expectNativeEl(fixture).not.toHaveCssClass('class2');

// Object keys are CSS classes that get added when the expression given in
// the value evaluates to a truthy value, otherwise they are removed.
matchMedia.activate('xs');
expectNativeEl(fixture).not.toHaveCssClass('class0');
expectNativeEl(fixture).toHaveCssClass('class1');
expectNativeEl(fixture).toHaveCssClass('what');
expectNativeEl(fixture).toHaveCssClass('class2');

// matchMedia.activate('lg');
// expectNativeEl(fixture).toHaveCssClass('class0');
// expectNativeEl(fixture).not.toHaveCssClass('class1');
// expectNativeEl(fixture).not.toHaveCssClass('class2');
matchMedia.activate('lg');
expectNativeEl(fixture).toHaveCssClass('class0');
expectNativeEl(fixture).not.toHaveCssClass('what');
expectNativeEl(fixture).not.toHaveCssClass('class2');
});

it('should keep the raw existing `class` with responsive updates', () => {
Expand Down
38 changes: 11 additions & 27 deletions src/lib/api/ext/class.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type NgClassType = string | string[] | Set<string> | {[klass: string]: an
*/
@Directive({
selector: `
[class], [class.xs], [class.sm], [class.md], [class.lg], [class.xl],
[class.xs], [class.sm], [class.md], [class.lg], [class.xl],
[class.lt-sm], [class.lt-md], [class.lt-lg], [class.lt-xl],
[class.gt-xs], [class.gt-sm], [class.gt-md], [class.gt-lg],
Expand Down Expand Up @@ -77,18 +77,6 @@ export class ClassDirective extends BaseFxDirective

/** Deprecated selectors */

/**
* Base class selector values get applied immediately and are considered destructive overwrites to
* all previous class assignments
*
* Delegate to NgClass:klass setter and cache value for base fallback from responsive APIs.
*/
@Input('class')
set classBase(val: string) {
this._classAdapter.cacheInput('_rawClass', val, true);
this._ngClassInstance.klass = val;
}

@Input('class.xs') set classXs(val: NgClassType) { this._classAdapter.cacheInput('classXs', val, true); }
@Input('class.sm') set classSm(val: NgClassType) { this._classAdapter.cacheInput('classSm', val, true); }
@Input('class.md') set classMd(val: NgClassType) { this._classAdapter.cacheInput('classMd', val, true); }
Expand All @@ -105,14 +93,6 @@ export class ClassDirective extends BaseFxDirective
@Input('class.gt-md') set classGtMd(val: NgClassType) { this._classAdapter.cacheInput('classGtMd', val, true); }
@Input('class.gt-lg') set classGtLg(val: NgClassType) { this._classAdapter.cacheInput('classGtLg', val, true); }

/**
* Initial value of the `class` attribute; used as
* fallback and will be merged with nay `ngClass` values
*/
get initialClasses() : string {
return this._classAdapter.queryInput('_rawClass') || "";
}

/* tslint:enable */
constructor(protected monitor: MediaMonitor,
_ngEl: ElementRef, _renderer: Renderer2, _oldRenderer: Renderer,
Expand All @@ -121,8 +101,9 @@ export class ClassDirective extends BaseFxDirective

super(monitor, _ngEl, _renderer);

this._classAdapter = new BaseFxDirectiveAdapter('class', monitor, _ngEl, _renderer);
this._ngClassAdapter = new BaseFxDirectiveAdapter('ngClass', monitor, _ngEl, _renderer);
this._classAdapter = new BaseFxDirectiveAdapter('class', monitor, _ngEl, _renderer);
this._classAdapter.cacheInput('class', _ngEl.nativeElement.getAttribute('class') || '', true);

// Create an instance NgClass Directive instance only if `ngClass=""` has NOT been defined on
// the same host element; since the responsive variations may be defined...
Expand Down Expand Up @@ -156,7 +137,9 @@ export class ClassDirective extends BaseFxDirective
if (!this._classAdapter.hasMediaQueryListener) {
this._configureMQListener();
}
this._ngClassInstance.ngDoCheck();
if ( this._ngClassInstance) {
this._ngClassInstance.ngDoCheck();
}
}

ngOnDestroy() {
Expand All @@ -174,11 +157,12 @@ export class ClassDirective extends BaseFxDirective
* mql change events to onMediaQueryChange handlers
*/
protected _configureMQListener() {
this._classAdapter.listenForMediaQueryChanges('class', '', (changes: MediaChange) => {
const value = this._classAdapter.queryInput('class');
this._classAdapter.listenForMediaQueryChanges('class', value, (changes: MediaChange) => {
this._updateKlass(changes.value);
});

this._ngClassAdapter.listenForMediaQueryChanges('ngClass', '', (changes: MediaChange) => {
this._ngClassAdapter.listenForMediaQueryChanges('ngClass', value, (changes: MediaChange) => {
this._updateNgClass(changes.value);
this._ngClassInstance.ngDoCheck(); // trigger NgClass::_applyIterableChanges()
});
Expand All @@ -189,11 +173,11 @@ export class ClassDirective extends BaseFxDirective
* ::ngDoCheck() is not needed
*/
protected _updateKlass(value?: NgClassType) {
let klass = value || this._classAdapter.queryInput('class') || '';
let klass = value || this._classAdapter.queryInput('class');
if (this._classAdapter.mqActivation) {
klass = this._classAdapter.mqActivation.activatedInput;
}
this._ngClassInstance.klass = klass || this.initialClasses;
this._ngClassInstance.klass = klass;
}

/**
Expand Down

0 comments on commit 06eea6c

Please sign in to comment.