Skip to content

Commit b0fdadd

Browse files
committed
fix(ngClass): ensure that ngClass only adds/removes the changed classes
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
1 parent 7b41285 commit b0fdadd

File tree

5 files changed

+88
-26
lines changed

5 files changed

+88
-26
lines changed

src/.jshintrc

+2-1
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@
100100
"assertNotHasOwnProperty": false,
101101
"getter": false,
102102
"getBlockElements": false,
103+
"tokenDifference": false,
103104

104105
/* AngularPublic.js */
105106
"version": false,
@@ -162,4 +163,4 @@
162163
"nullFormCtrl": false
163164

164165
}
165-
}
166+
}

src/Angular.js

+23-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@
8080
-assertArgFn,
8181
-assertNotHasOwnProperty,
8282
-getter,
83-
-getBlockElements
83+
-getBlockElements,
84+
-tokenDifference
8485
8586
*/
8687

@@ -1338,3 +1339,24 @@ function getBlockElements(block) {
13381339

13391340
return jqLite(elements);
13401341
}
1342+
1343+
/**
1344+
* Return the string difference between tokens of the original string compared to the old string
1345+
* @param {str1} string original string value
1346+
* @param {str2} string new string value
1347+
*/
1348+
function tokenDifference(str1, str2) {
1349+
var values = '',
1350+
tokens1 = str1.split(/\s+/),
1351+
tokens2 = str2.split(/\s+/);
1352+
1353+
outer:
1354+
for(var i=0;i<tokens1.length;i++) {
1355+
var token = tokens1[i];
1356+
for(var j=0;j<tokens2.length;j++) {
1357+
if(token == tokens2[j]) continue outer;
1358+
}
1359+
values += (values.length > 0 ? ' ' : '') + token;
1360+
}
1361+
return values;
1362+
}

src/ng/compile.js

+2-18
Original file line numberDiff line numberDiff line change
@@ -677,8 +677,8 @@ function $CompileProvider($provide) {
677677
if(key == 'class') {
678678
value = value || '';
679679
var current = this.$$element.attr('class') || '';
680-
this.$removeClass(tokenDifference(current, value).join(' '));
681-
this.$addClass(tokenDifference(value, current).join(' '));
680+
this.$removeClass(tokenDifference(current, value));
681+
this.$addClass(tokenDifference(value, current));
682682
} else {
683683
var booleanKey = getBooleanAttrName(this.$$element[0], key),
684684
normalizedVal,
@@ -736,22 +736,6 @@ function $CompileProvider($provide) {
736736
$exceptionHandler(e);
737737
}
738738
});
739-
740-
function tokenDifference(str1, str2) {
741-
var values = [],
742-
tokens1 = str1.split(/\s+/),
743-
tokens2 = str2.split(/\s+/);
744-
745-
outer:
746-
for(var i=0;i<tokens1.length;i++) {
747-
var token = tokens1[i];
748-
for(var j=0;j<tokens2.length;j++) {
749-
if(token == tokens2[j]) continue outer;
750-
}
751-
values.push(token);
752-
}
753-
return values;
754-
}
755739
},
756740

757741

src/ng/directive/ngClass.js

+18-6
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ function classDirective(name, selector) {
2121
var mod = $index & 1;
2222
if (mod !== old$index & 1) {
2323
if (mod === selector) {
24-
addClass(scope.$eval(attr[name]));
24+
addClass(flattenClasses(scope.$eval(attr[name])));
2525
} else {
26-
removeClass(scope.$eval(attr[name]));
26+
removeClass(flattenClasses(scope.$eval(attr[name])));
2727
}
2828
}
2929
});
@@ -32,22 +32,34 @@ function classDirective(name, selector) {
3232

3333
function ngClassWatchAction(newVal) {
3434
if (selector === true || scope.$index % 2 === selector) {
35+
var newClasses = flattenClasses(newVal || '');
3536
if (oldVal && !equals(newVal,oldVal)) {
36-
removeClass(oldVal);
37+
var oldClasses = flattenClasses(oldVal);
38+
var toRemove = tokenDifference(oldClasses, newClasses);
39+
if(toRemove.length > 0) {
40+
removeClass(toRemove);
41+
}
42+
43+
var toAdd = tokenDifference(newClasses, oldClasses);
44+
if(toAdd.length > 0) {
45+
addClass(toAdd);
46+
}
47+
}
48+
else {
49+
addClass(newClasses);
3750
}
38-
addClass(newVal);
3951
}
4052
oldVal = copy(newVal);
4153
}
4254

4355

4456
function removeClass(classVal) {
45-
attr.$removeClass(flattenClasses(classVal));
57+
attr.$removeClass(classVal);
4658
}
4759

4860

4961
function addClass(classVal) {
50-
attr.$addClass(flattenClasses(classVal));
62+
attr.$addClass(classVal);
5163
}
5264

5365
function flattenClasses(classVal) {

test/ng/directive/ngClassSpec.js

+43
Original file line numberDiff line numberDiff line change
@@ -411,4 +411,47 @@ describe('ngClass animations', function() {
411411
expect(enterComplete).toBe(true);
412412
});
413413
});
414+
415+
it("should not remove classes if they're going to be added back right after", function() {
416+
module('mock.animate');
417+
418+
inject(function($rootScope, $compile, $animate) {
419+
var className;
420+
421+
$rootScope.one = true;
422+
$rootScope.two = true;
423+
$rootScope.three = true;
424+
425+
var element = angular.element('<div ng-class="{one:one, two:two, three:three}"></div>');
426+
$compile(element)($rootScope);
427+
$rootScope.$digest();
428+
429+
//this fires twice due to the class observer firing
430+
className = $animate.flushNext('addClass').params[1];
431+
className = $animate.flushNext('addClass').params[1];
432+
expect(className).toBe('one two three');
433+
434+
expect($animate.queue.length).toBe(0);
435+
436+
$rootScope.three = false;
437+
$rootScope.$digest();
438+
439+
className = $animate.flushNext('removeClass').params[1];
440+
expect(className).toBe('three');
441+
442+
expect($animate.queue.length).toBe(0);
443+
444+
$rootScope.two = false;
445+
$rootScope.three = true;
446+
$rootScope.$digest();
447+
448+
className = $animate.flushNext('removeClass').params[1];
449+
expect(className).toBe('two');
450+
451+
className = $animate.flushNext('addClass').params[1];
452+
expect(className).toBe('three');
453+
454+
expect($animate.queue.length).toBe(0);
455+
});
456+
});
414457
});

0 commit comments

Comments
 (0)