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

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 #4960
Closes #4944
  • Loading branch information
matsko committed Nov 20, 2013
1 parent 7067a8f commit 6b8bbe4
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 6b8bbe4

Please sign in to comment.