Skip to content

Commit

Permalink
fix(common): allow null/undefined to be passed to ngClass input (#39280
Browse files Browse the repository at this point in the history
…) (#46906)

With strict template type checking, a null/undefined value will raise an
error. However the implementation is completely fine with it, and it
would be pointless to "fix" it at the callsite and convert to e.g. an
empty string. Allow all of the "supported types" to be passed in
directly to ngClass.

Fixes #39280

PR Close #46906
  • Loading branch information
imirkin authored and AndrewKushnir committed Aug 1, 2022
1 parent f364378 commit e2ab99b
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 4 deletions.
2 changes: 1 addition & 1 deletion goldens/public-api/common/index.md
Expand Up @@ -403,7 +403,7 @@ export class NgClass implements DoCheck {
// (undocumented)
set ngClass(value: string | string[] | Set<string> | {
[klass: string]: any;
});
} | null | undefined);
// (undocumented)
ngDoCheck(): void;
// (undocumented)
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/directives/ng_class.ts
Expand Up @@ -61,7 +61,7 @@ export class NgClass implements DoCheck {
}

@Input('ngClass')
set ngClass(value: string|string[]|Set<string>|{[klass: string]: any}) {
set ngClass(value: string|string[]|Set<string>|{[klass: string]: any}|null|undefined) {
this._removeClasses(this._rawClass);
this._applyClasses(this._initialClasses);

Expand Down
45 changes: 43 additions & 2 deletions packages/common/test/directives/ng_class_spec.ts
Expand Up @@ -115,6 +115,17 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';
detectChangesAndExpectClassName('bar');
}));

it('should remove active classes when expression evaluates to undefined', waitForAsync(() => {
fixture = createTestComponent('<div [ngClass]="objExpr"></div>');

detectChangesAndExpectClassName('foo');

getComponent().objExpr = undefined;
detectChangesAndExpectClassName('');

getComponent().objExpr = {'foo': false, 'bar': true};
detectChangesAndExpectClassName('bar');
}));

it('should allow multiple classes per expression', waitForAsync(() => {
fixture = createTestComponent('<div [ngClass]="objExpr"></div>');
Expand Down Expand Up @@ -246,6 +257,15 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';
detectChangesAndExpectClassName('');
}));

it('should remove active classes when switching from string to undefined',
waitForAsync(() => {
fixture = createTestComponent(`<div [ngClass]="strExpr"></div>`);
detectChangesAndExpectClassName('foo');

getComponent().strExpr = undefined;
detectChangesAndExpectClassName('');
}));

it('should take initial classes into account when switching from string to null',
waitForAsync(() => {
fixture = createTestComponent(`<div class="foo" [ngClass]="strExpr"></div>`);
Expand All @@ -255,6 +275,15 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';
detectChangesAndExpectClassName('foo');
}));

it('should take initial classes into account when switching from string to undefined',
waitForAsync(() => {
fixture = createTestComponent(`<div class="foo" [ngClass]="strExpr"></div>`);
detectChangesAndExpectClassName('foo');

getComponent().strExpr = undefined;
detectChangesAndExpectClassName('foo');
}));

it('should ignore empty and blank strings', waitForAsync(() => {
fixture = createTestComponent(`<div class="foo" [ngClass]="strExpr"></div>`);
getComponent().strExpr = '';
Expand All @@ -275,6 +304,9 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';

getComponent().objExpr = null;
detectChangesAndExpectClassName('init foo');

getComponent().objExpr = undefined;
detectChangesAndExpectClassName('init foo');
}));

it('should co-operate with the interpolated class attribute', waitForAsync(() => {
Expand All @@ -289,6 +321,9 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';

getComponent().objExpr = null;
detectChangesAndExpectClassName(`init foo`);

getComponent().objExpr = undefined;
detectChangesAndExpectClassName(`init foo`);
}));

it('should co-operate with the interpolated class attribute when interpolation changes',
Expand All @@ -315,6 +350,9 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';

getComponent().objExpr = null;
detectChangesAndExpectClassName(`init foo`);

getComponent().objExpr = undefined;
detectChangesAndExpectClassName(`init foo`);
}));

it('should co-operate with the class attribute and class.name binding', waitForAsync(() => {
Expand Down Expand Up @@ -351,6 +389,9 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';

cmp.objExpr = null;
detectChangesAndExpectClassName('init baz');

cmp.objExpr = undefined;
detectChangesAndExpectClassName('init baz');
}));
});

Expand Down Expand Up @@ -419,8 +460,8 @@ class TestComponent {
items: any[]|undefined;
arrExpr: string[] = ['foo'];
setExpr: Set<string> = new Set<string>();
objExpr: {[klass: string]: any}|null = {'foo': true, 'bar': false};
strExpr: string|null = 'foo';
objExpr: {[klass: string]: any}|null|undefined = {'foo': true, 'bar': false};
strExpr: string|null|undefined = 'foo';

constructor() {
this.setExpr.add('foo');
Expand Down

0 comments on commit e2ab99b

Please sign in to comment.