From 0ac969a5ee1687cfd4517821943f34fe948bb3fc Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 16 Apr 2013 12:29:56 +0100 Subject: [PATCH] fix(ngClass): should remove classes when object is the same but property has changed If you wire up ngClass directly to an object on the scope, e.g. ng-class="myClasses", where scope.myClasses = { 'classA': true, 'classB': false }, there was a bug that changing scope.myClasses.classA = false, was not being picked up and classA was not being removed from the element's CSS classes. This fix uses angular.equals for the comparison and ensures that oldVal is a copy of (rather than a reference to) the newVal. --- src/ng/directive/ngClass.js | 4 ++-- test/ng/directive/ngClassSpec.js | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/ng/directive/ngClass.js b/src/ng/directive/ngClass.js index d731118f82b9..09efe0c80cc8 100644 --- a/src/ng/directive/ngClass.js +++ b/src/ng/directive/ngClass.js @@ -29,12 +29,12 @@ function classDirective(name, selector) { function ngClassWatchAction(newVal) { if (selector === true || scope.$index % 2 === selector) { - if (oldVal && (newVal !== oldVal)) { + if (oldVal && !equals(newVal,oldVal)) { removeClass(oldVal); } addClass(newVal); } - oldVal = newVal; + oldVal = copy(newVal); } diff --git a/test/ng/directive/ngClassSpec.js b/test/ng/directive/ngClassSpec.js index d4bd76fed688..e73713d80029 100644 --- a/test/ng/directive/ngClassSpec.js +++ b/test/ng/directive/ngClassSpec.js @@ -43,7 +43,7 @@ describe('ngClass', function() { 'expressions', inject(function($rootScope, $compile) { var element = $compile( '
' + + 'ng-class="{A: conditionA, B: conditionB(), AnotB: conditionA&&!conditionB()}">' + '
')($rootScope); $rootScope.conditionA = true; $rootScope.$digest(); @@ -52,7 +52,7 @@ describe('ngClass', function() { expect(element.hasClass('B')).toBeFalsy(); expect(element.hasClass('AnotB')).toBeTruthy(); - $rootScope.conditionB = function() { return true }; + $rootScope.conditionB = function() { return true; }; $rootScope.$digest(); expect(element.hasClass('existing')).toBeTruthy(); expect(element.hasClass('A')).toBeTruthy(); @@ -61,6 +61,20 @@ describe('ngClass', function() { })); + it('should remove classes when the referenced object is the same but its property is changed', + inject(function($rootScope, $compile) { + var element = $compile('
')($rootScope); + $rootScope.classes = { A: true, B: true }; + $rootScope.$digest(); + expect(element.hasClass('A')).toBeTruthy(); + expect(element.hasClass('B')).toBeTruthy(); + $rootScope.classes.A = false; + $rootScope.$digest(); + expect(element.hasClass('A')).toBeFalsy(); + expect(element.hasClass('B')).toBeTruthy(); + })); + + it('should support adding multiple classes via a space delimited string', inject(function($rootScope, $compile) { element = $compile('
')($rootScope); $rootScope.$digest();