Skip to content

Commit

Permalink
fix(api, class): selector [class] should be removed from ClassDirecti…
Browse files Browse the repository at this point in the history 
…ve. (#394)

The host `class` attribute should be considered static and should not 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 @tinayuangao
ThomasBurleson authored and tinayuangao committed Sep 7, 2017
1 parent 816d85a commit 7a48c25
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 48 deletions.
14 changes: 13 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,14 @@ 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 totalKeys = Object.keys(this._inputMap).length;
const baseValue = this._inputMap[this._baseKey];
return (totalKeys - (!!baseValue ? 1 : 0)) > 0;
}
}
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
64 changes: 24 additions & 40 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,41 +77,21 @@ 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); }
@Input('class.sm') set classSm(val: NgClassType) { this._classAdapter.cacheInput('classSm', val); }
@Input('class.md') set classMd(val: NgClassType) { this._classAdapter.cacheInput('classMd', val); }
@Input('class.lg') set classLg(val: NgClassType) { this._classAdapter.cacheInput('classLg', val); }
@Input('class.xl') set classXl(val: NgClassType) { this._classAdapter.cacheInput('classXl', 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); }
@Input('class.lg') set classLg(val: NgClassType) { this._classAdapter.cacheInput('classLg', val, true); }
@Input('class.xl') set classXl(val: NgClassType) { this._classAdapter.cacheInput('classXl', val, true); }
@Input('class.lt-sm') set classLtSm(val: NgClassType) { this._classAdapter.cacheInput('classLtSm', val); }
@Input('class.lt-md') set classLtMd(val: NgClassType) { this._classAdapter.cacheInput('classLtMd', val); }
@Input('class.lt-lg') set classLtLg(val: NgClassType) { this._classAdapter.cacheInput('classLtLg', val); }
@Input('class.lt-xl') set classLtXl(val: NgClassType) { this._classAdapter.cacheInput('classLtXl', val); }

@Input('class.lt-sm') set classLtSm(val: NgClassType) { this._classAdapter.cacheInput('classLtSm', val, true); }
@Input('class.lt-md') set classLtMd(val: NgClassType) { this._classAdapter.cacheInput('classLtMd', val, true); }
@Input('class.lt-lg') set classLtLg(val: NgClassType) { this._classAdapter.cacheInput('classLtLg', val, true); }
@Input('class.lt-xl') set classLtXl(val: NgClassType) { this._classAdapter.cacheInput('classLtXl', val, true); }

@Input('class.gt-xs') set classGtXs(val: NgClassType) { this._classAdapter.cacheInput('classGtXs', val, true); }
@Input('class.gt-sm') set classGtSm(val: NgClassType) { this._classAdapter.cacheInput('classGtSm', val, true); }
@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') || "";
}
@Input('class.gt-xs') set classGtXs(val: NgClassType) { this._classAdapter.cacheInput('classGtXs', val); }
@Input('class.gt-sm') set classGtSm(val: NgClassType) { this._classAdapter.cacheInput('classGtSm', val); }
@Input('class.gt-md') set classGtMd(val: NgClassType) { this._classAdapter.cacheInput('classGtMd', val); }
@Input('class.gt-lg') set classGtLg(val: NgClassType) { this._classAdapter.cacheInput('classGtLg', val); }

/* tslint:enable */
constructor(protected monitor: MediaMonitor,
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') || '');

// 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 @@ -157,7 +138,9 @@ export class ClassDirective extends BaseFxDirective
if (!this._classAdapter.hasMediaQueryListener) {
this._configureMQListener();
}
this._ngClassInstance.ngDoCheck();
if ( this._ngClassInstance) {
this._ngClassInstance.ngDoCheck();
}
}

ngOnDestroy() {
Expand All @@ -175,11 +158,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 @@ -190,11 +174,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 7a48c25

Please sign in to comment.