Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 0ac969a

Browse files
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.
1 parent 4652386 commit 0ac969a

File tree

2 files changed

+18
-4
lines changed

2 files changed

+18
-4
lines changed

src/ng/directive/ngClass.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,12 @@ function classDirective(name, selector) {
2929

3030
function ngClassWatchAction(newVal) {
3131
if (selector === true || scope.$index % 2 === selector) {
32-
if (oldVal && (newVal !== oldVal)) {
32+
if (oldVal && !equals(newVal,oldVal)) {
3333
removeClass(oldVal);
3434
}
3535
addClass(newVal);
3636
}
37-
oldVal = newVal;
37+
oldVal = copy(newVal);
3838
}
3939

4040

test/ng/directive/ngClassSpec.js

+16-2
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ describe('ngClass', function() {
4343
'expressions', inject(function($rootScope, $compile) {
4444
var element = $compile(
4545
'<div class="existing" ' +
46-
'ng-class="{A: conditionA, B: conditionB(), AnotB: conditionA&&!conditionB}">' +
46+
'ng-class="{A: conditionA, B: conditionB(), AnotB: conditionA&&!conditionB()}">' +
4747
'</div>')($rootScope);
4848
$rootScope.conditionA = true;
4949
$rootScope.$digest();
@@ -52,7 +52,7 @@ describe('ngClass', function() {
5252
expect(element.hasClass('B')).toBeFalsy();
5353
expect(element.hasClass('AnotB')).toBeTruthy();
5454

55-
$rootScope.conditionB = function() { return true };
55+
$rootScope.conditionB = function() { return true; };
5656
$rootScope.$digest();
5757
expect(element.hasClass('existing')).toBeTruthy();
5858
expect(element.hasClass('A')).toBeTruthy();
@@ -61,6 +61,20 @@ describe('ngClass', function() {
6161
}));
6262

6363

64+
it('should remove classes when the referenced object is the same but its property is changed',
65+
inject(function($rootScope, $compile) {
66+
var element = $compile('<div ng-class="classes"></div>')($rootScope);
67+
$rootScope.classes = { A: true, B: true };
68+
$rootScope.$digest();
69+
expect(element.hasClass('A')).toBeTruthy();
70+
expect(element.hasClass('B')).toBeTruthy();
71+
$rootScope.classes.A = false;
72+
$rootScope.$digest();
73+
expect(element.hasClass('A')).toBeFalsy();
74+
expect(element.hasClass('B')).toBeTruthy();
75+
}));
76+
77+
6478
it('should support adding multiple classes via a space delimited string', inject(function($rootScope, $compile) {
6579
element = $compile('<div class="existing" ng-class="\'A B\'"></div>')($rootScope);
6680
$rootScope.$digest();

0 commit comments

Comments
 (0)