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

Commit

Permalink
fix(ngClass): should remove classes when object is the same but prope…
Browse files Browse the repository at this point in the history
…rty 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.
  • Loading branch information
petebacondarwin committed Apr 16, 2013
1 parent 4652386 commit 0ac969a
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
4 changes: 2 additions & 2 deletions src/ng/directive/ngClass.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand Down
18 changes: 16 additions & 2 deletions test/ng/directive/ngClassSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('ngClass', function() {
'expressions', inject(function($rootScope, $compile) {
var element = $compile(
'<div class="existing" ' +
'ng-class="{A: conditionA, B: conditionB(), AnotB: conditionA&&!conditionB}">' +
'ng-class="{A: conditionA, B: conditionB(), AnotB: conditionA&&!conditionB()}">' +
'</div>')($rootScope);
$rootScope.conditionA = true;
$rootScope.$digest();
Expand All @@ -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();
Expand All @@ -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('<div ng-class="classes"></div>')($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('<div class="existing" ng-class="\'A B\'"></div>')($rootScope);
$rootScope.$digest();
Expand Down

0 comments on commit 0ac969a

Please sign in to comment.