From ed25a29cc8f50ae9e21ee49941c7c574b4b41b9e Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Mon, 10 Aug 2015 12:25:46 +0200 Subject: [PATCH] fix(NgClass): take initial classes into account during cleanup Closes #3557 --- modules/angular2/src/directives/ng_class.ts | 42 +++- .../angular2/test/directives/ng_class_spec.ts | 190 ++++++++++++++---- 2 files changed, 177 insertions(+), 55 deletions(-) diff --git a/modules/angular2/src/directives/ng_class.ts b/modules/angular2/src/directives/ng_class.ts index b0dbc59a37..0a2f331790 100644 --- a/modules/angular2/src/directives/ng_class.ts +++ b/modules/angular2/src/directives/ng_class.ts @@ -33,16 +33,24 @@ import {ListWrapper, StringMapWrapper, isListLikeIterable} from 'angular2/src/fa @Directive({ selector: '[ng-class]', lifecycle: [LifecycleEvent.onCheck, LifecycleEvent.onDestroy], - properties: ['rawClass: ng-class'] + properties: ['rawClass: ng-class', 'initialClasses: class'] }) export class NgClass { private _differ: any; private _mode: string; - _rawClass; + private _initialClasses = []; + private _rawClass; constructor(private _iterableDiffers: IterableDiffers, private _keyValueDiffers: KeyValueDiffers, private _ngEl: ElementRef, private _renderer: Renderer) {} + set initialClasses(v) { + this._applyInitialClasses(true); + this._initialClasses = isPresent(v) && isString(v) ? v.split(' ') : []; + this._applyInitialClasses(false); + this._applyClasses(this._rawClass, false); + } + set rawClass(v) { this._cleanupClasses(this._rawClass); @@ -59,6 +67,8 @@ export class NgClass { this._differ = this._keyValueDiffers.find(v).create(null); this._mode = 'keyValue'; } + } else { + this._differ = null; } } @@ -78,15 +88,8 @@ export class NgClass { onDestroy(): void { this._cleanupClasses(this._rawClass); } private _cleanupClasses(rawClassVal): void { - if (isPresent(rawClassVal)) { - if (isListLikeIterable(rawClassVal)) { - ListWrapper.forEach(rawClassVal, (className) => { this._toggleClass(className, false); }); - } else { - StringMapWrapper.forEach(rawClassVal, (expVal, className) => { - if (expVal) this._toggleClass(className, false); - }); - } - } + this._applyClasses(rawClassVal, true); + this._applyInitialClasses(false); } private _applyKeyValueChanges(changes: any): void { @@ -104,6 +107,23 @@ export class NgClass { changes.forEachRemovedItem((record) => { this._toggleClass(record.item, false); }); } + private _applyInitialClasses(isCleanup: boolean) { + ListWrapper.forEach(this._initialClasses, + (className) => { this._toggleClass(className, !isCleanup); }); + } + + private _applyClasses(rawClassVal, isCleanup: boolean) { + if (isPresent(rawClassVal)) { + if (isListLikeIterable(rawClassVal)) { + ListWrapper.forEach(rawClassVal, (className) => this._toggleClass(className, !isCleanup)); + } else { + StringMapWrapper.forEach(rawClassVal, (expVal, className) => { + if (expVal) this._toggleClass(className, !isCleanup); + }); + } + } + } + private _toggleClass(className: string, enabled): void { this._renderer.setElementClass(this._ngEl, className, enabled); } diff --git a/modules/angular2/test/directives/ng_class_spec.ts b/modules/angular2/test/directives/ng_class_spec.ts index 7e54f12d6a..2e267dfba2 100644 --- a/modules/angular2/test/directives/ng_class_spec.ts +++ b/modules/angular2/test/directives/ng_class_spec.ts @@ -127,6 +127,25 @@ export function main() { rootTC.componentInstance.objExpr = {baz: true}; detectChangesAndCheck(rootTC, 'ng-binding baz'); + async.done(); + }); + })); + + it('should remove active classes when expression evaluates to null', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + var template = '
'; + + tcb.overrideTemplate(TestComponent, template) + .createAsync(TestComponent) + .then((rootTC) => { + detectChangesAndCheck(rootTC, 'ng-binding foo'); + + rootTC.componentInstance.objExpr = null; + detectChangesAndCheck(rootTC, 'ng-binding'); + + rootTC.componentInstance.objExpr = {'foo': false, 'bar': true}; + detectChangesAndCheck(rootTC, 'ng-binding bar'); + async.done(); }); })); @@ -181,6 +200,22 @@ export function main() { rootTC.componentInstance.arrExpr = ['bar']; detectChangesAndCheck(rootTC, 'ng-binding bar'); + async.done(); + }); + })); + + it('should take initial classes into account when a reference changes', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + var template = '
'; + + tcb.overrideTemplate(TestComponent, template) + .createAsync(TestComponent) + .then((rootTC) => { + detectChangesAndCheck(rootTC, 'foo ng-binding'); + + rootTC.componentInstance.arrExpr = ['bar']; + detectChangesAndCheck(rootTC, 'ng-binding foo bar'); + async.done(); }); })); @@ -220,7 +255,6 @@ export function main() { }); })); - it('should remove active classes when switching from string to null', inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { var template = `
`; @@ -236,65 +270,133 @@ export function main() { async.done(); }); })); + + it('should take initial classes into account when switching from string to null', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + var template = `
`; + + tcb.overrideTemplate(TestComponent, template) + .createAsync(TestComponent) + .then((rootTC) => { + detectChangesAndCheck(rootTC, 'foo ng-binding'); + + rootTC.componentInstance.strExpr = null; + detectChangesAndCheck(rootTC, 'ng-binding foo'); + + async.done(); + }); + })); + }); - it('should remove active classes when expression evaluates to null', - inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { - var template = '
'; + describe('cooperation with other class-changing constructs', () => { - tcb.overrideTemplate(TestComponent, template) - .createAsync(TestComponent) - .then((rootTC) => { - detectChangesAndCheck(rootTC, 'ng-binding foo'); + it('should co-operate with the class attribute', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + var template = '
'; - rootTC.componentInstance.objExpr = null; - detectChangesAndCheck(rootTC, 'ng-binding'); + tcb.overrideTemplate(TestComponent, template) + .createAsync(TestComponent) + .then((rootTC) => { + StringMapWrapper.set(rootTC.componentInstance.objExpr, 'bar', true); + detectChangesAndCheck(rootTC, 'init foo ng-binding bar'); - rootTC.componentInstance.objExpr = {'foo': false, 'bar': true}; - detectChangesAndCheck(rootTC, 'ng-binding bar'); + StringMapWrapper.set(rootTC.componentInstance.objExpr, 'foo', false); + detectChangesAndCheck(rootTC, 'init ng-binding bar'); - async.done(); - }); - })); + rootTC.componentInstance.objExpr = null; + detectChangesAndCheck(rootTC, 'init ng-binding foo'); - it('should co-operate with the class attribute', - inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { - var template = '
'; + async.done(); + }); + })); - tcb.overrideTemplate(TestComponent, template) - .createAsync(TestComponent) - .then((rootTC) => { - StringMapWrapper.set(rootTC.componentInstance.objExpr, 'bar', true); - detectChangesAndCheck(rootTC, 'init foo ng-binding bar'); + it('should co-operate with the interpolated class attribute', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + var template = `
`; - StringMapWrapper.set(rootTC.componentInstance.objExpr, 'foo', false); - detectChangesAndCheck(rootTC, 'init ng-binding bar'); + tcb.overrideTemplate(TestComponent, template) + .createAsync(TestComponent) + .then((rootTC) => { + StringMapWrapper.set(rootTC.componentInstance.objExpr, 'bar', true); + detectChangesAndCheck(rootTC, `{{'init foo'}} ng-binding init foo bar`); - async.done(); - }); - })); + StringMapWrapper.set(rootTC.componentInstance.objExpr, 'foo', false); + detectChangesAndCheck(rootTC, `{{'init foo'}} ng-binding init bar`); - it('should co-operate with the class attribute and class.name binding', - inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { - var template = '
'; + rootTC.componentInstance.objExpr = null; + detectChangesAndCheck(rootTC, `{{'init foo'}} ng-binding init foo`); - tcb.overrideTemplate(TestComponent, template) - .createAsync(TestComponent) - .then((rootTC) => { - detectChangesAndCheck(rootTC, 'init foo ng-binding baz'); + async.done(); + }); + })); - StringMapWrapper.set(rootTC.componentInstance.objExpr, 'bar', true); - detectChangesAndCheck(rootTC, 'init foo ng-binding baz bar'); + it('should co-operate with the class attribute and binding to it', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + var template = `
`; - StringMapWrapper.set(rootTC.componentInstance.objExpr, 'foo', false); - detectChangesAndCheck(rootTC, 'init ng-binding baz bar'); + tcb.overrideTemplate(TestComponent, template) + .createAsync(TestComponent) + .then((rootTC) => { + StringMapWrapper.set(rootTC.componentInstance.objExpr, 'bar', true); + detectChangesAndCheck(rootTC, `init ng-binding foo bar`); - rootTC.componentInstance.condition = false; - detectChangesAndCheck(rootTC, 'init ng-binding bar'); + StringMapWrapper.set(rootTC.componentInstance.objExpr, 'foo', false); + detectChangesAndCheck(rootTC, `init ng-binding bar`); - async.done(); - }); - })); + rootTC.componentInstance.objExpr = null; + detectChangesAndCheck(rootTC, `init ng-binding foo`); + + async.done(); + }); + })); + + it('should co-operate with the class attribute and class.name binding', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + var template = + '
'; + + tcb.overrideTemplate(TestComponent, template) + .createAsync(TestComponent) + .then((rootTC) => { + detectChangesAndCheck(rootTC, 'init foo ng-binding baz'); + + StringMapWrapper.set(rootTC.componentInstance.objExpr, 'bar', true); + detectChangesAndCheck(rootTC, 'init foo ng-binding baz bar'); + + StringMapWrapper.set(rootTC.componentInstance.objExpr, 'foo', false); + detectChangesAndCheck(rootTC, 'init ng-binding baz bar'); + + rootTC.componentInstance.condition = false; + detectChangesAndCheck(rootTC, 'init ng-binding bar'); + + async.done(); + }); + })); + + it('should co-operate with initial class and class attribute binding when binding changes', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + var template = '
'; + + tcb.overrideTemplate(TestComponent, template) + .createAsync(TestComponent) + .then((rootTC) => { + detectChangesAndCheck(rootTC, 'init ng-binding foo'); + + StringMapWrapper.set(rootTC.componentInstance.objExpr, 'bar', true); + detectChangesAndCheck(rootTC, 'init ng-binding foo bar'); + + rootTC.componentInstance.strExpr = 'baz'; + detectChangesAndCheck(rootTC, 'init ng-binding bar baz foo'); + + rootTC.componentInstance.objExpr = null; + detectChangesAndCheck(rootTC, 'init ng-binding baz'); + + async.done(); + }); + })); + + }); }) }