Skip to content

Commit

Permalink
fix(ngClass): ensure that ngClass only adds/removes the changed classes
Browse files Browse the repository at this point in the history
ngClass works by removing all the former classes and then adding all the
new classes to the element during each watch change operation. This may
cause transition animations to never render. The ngClass directive will
now only add and remove the classes that change during each watch operation.

Closes angular#4960
Closes angular#4944
  • Loading branch information
matsko authored and jamesdaily committed Jan 27, 2014
1 parent 3959317 commit 8de25b6
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 26 deletions.
3 changes: 2 additions & 1 deletion src/.jshintrc
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
"assertNotHasOwnProperty": false,
"getter": false,
"getBlockElements": false,
"tokenDifference": false,

/* AngularPublic.js */
"version": false,
Expand Down Expand Up @@ -162,4 +163,4 @@
"nullFormCtrl": false

}
}
}
24 changes: 23 additions & 1 deletion src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@
-assertArgFn,
-assertNotHasOwnProperty,
-getter,
-getBlockElements
-getBlockElements,
-tokenDifference
*/

Expand Down Expand Up @@ -1350,3 +1351,24 @@ function getBlockElements(block) {

return jqLite(elements);
}

/**
* Return the string difference between tokens of the original string compared to the old string
* @param {str1} string original string value
* @param {str2} string new string value
*/
function tokenDifference(str1, str2) {
var values = '',
tokens1 = str1.split(/\s+/),
tokens2 = str2.split(/\s+/);

outer:
for(var i=0;i<tokens1.length;i++) {
var token = tokens1[i];
for(var j=0;j<tokens2.length;j++) {
if(token == tokens2[j]) continue outer;
}
values += (values.length > 0 ? ' ' : '') + token;
}
return values;
}
20 changes: 2 additions & 18 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -688,8 +688,8 @@ function $CompileProvider($provide) {
if(key == 'class') {
value = value || '';
var current = this.$$element.attr('class') || '';
this.$removeClass(tokenDifference(current, value).join(' '));
this.$addClass(tokenDifference(value, current).join(' '));
this.$removeClass(tokenDifference(current, value));
this.$addClass(tokenDifference(value, current));
} else {
var booleanKey = getBooleanAttrName(this.$$element[0], key),
normalizedVal,
Expand Down Expand Up @@ -747,22 +747,6 @@ function $CompileProvider($provide) {
$exceptionHandler(e);
}
});

function tokenDifference(str1, str2) {
var values = [],
tokens1 = str1.split(/\s+/),
tokens2 = str2.split(/\s+/);

outer:
for(var i=0;i<tokens1.length;i++) {
var token = tokens1[i];
for(var j=0;j<tokens2.length;j++) {
if(token == tokens2[j]) continue outer;
}
values.push(token);
}
return values;
}
},


Expand Down
23 changes: 17 additions & 6 deletions src/ng/directive/ngClass.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ function classDirective(name, selector) {
var mod = $index & 1;
if (mod !== old$index & 1) {
if (mod === selector) {
addClass(scope.$eval(attr[name]));
addClass(flattenClasses(scope.$eval(attr[name])));
} else {
removeClass(scope.$eval(attr[name]));
removeClass(flattenClasses(scope.$eval(attr[name])));
}
}
});
Expand All @@ -32,22 +32,33 @@ function classDirective(name, selector) {

function ngClassWatchAction(newVal) {
if (selector === true || scope.$index % 2 === selector) {
var newClasses = flattenClasses(newVal || '');
if (oldVal && !equals(newVal,oldVal)) {
removeClass(oldVal);
var oldClasses = flattenClasses(oldVal);
var toRemove = tokenDifference(oldClasses, newClasses);
if(toRemove.length > 0) {
removeClass(toRemove);
}

var toAdd = tokenDifference(newClasses, oldClasses);
if(toAdd.length > 0) {
addClass(toAdd);
}
} else {
addClass(newClasses);
}
addClass(newVal);
}
oldVal = copy(newVal);
}


function removeClass(classVal) {
attr.$removeClass(flattenClasses(classVal));
attr.$removeClass(classVal);
}


function addClass(classVal) {
attr.$addClass(flattenClasses(classVal));
attr.$addClass(classVal);
}

function flattenClasses(classVal) {
Expand Down
43 changes: 43 additions & 0 deletions test/ng/directive/ngClassSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -411,4 +411,47 @@ describe('ngClass animations', function() {
expect(enterComplete).toBe(true);
});
});

it("should not remove classes if they're going to be added back right after", function() {
module('mock.animate');

inject(function($rootScope, $compile, $animate) {
var className;

$rootScope.one = true;
$rootScope.two = true;
$rootScope.three = true;

var element = angular.element('<div ng-class="{one:one, two:two, three:three}"></div>');
$compile(element)($rootScope);
$rootScope.$digest();

//this fires twice due to the class observer firing
className = $animate.flushNext('addClass').params[1];
className = $animate.flushNext('addClass').params[1];
expect(className).toBe('one two three');

expect($animate.queue.length).toBe(0);

$rootScope.three = false;
$rootScope.$digest();

className = $animate.flushNext('removeClass').params[1];
expect(className).toBe('three');

expect($animate.queue.length).toBe(0);

$rootScope.two = false;
$rootScope.three = true;
$rootScope.$digest();

className = $animate.flushNext('removeClass').params[1];
expect(className).toBe('two');

className = $animate.flushNext('addClass').params[1];
expect(className).toBe('three');

expect($animate.queue.length).toBe(0);
});
});
});

0 comments on commit 8de25b6

Please sign in to comment.