Skip to content

Commit 82d7913

Browse files
matskojamesdaily
authored andcommitted
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 Closes angular#4944
1 parent 4e43e7d commit 82d7913

File tree

5 files changed

+87
-26
lines changed

5 files changed

+87
-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

@@ -1350,3 +1351,24 @@ function getBlockElements(block) {
13501351

13511352
return jqLite(elements);
13521353
}
1354+
1355+
/**
1356+
* Return the string difference between tokens of the original string compared to the old string
1357+
* @param {str1} string original string value
1358+
* @param {str2} string new string value
1359+
*/
1360+
function tokenDifference(str1, str2) {
1361+
var values = '',
1362+
tokens1 = str1.split(/\s+/),
1363+
tokens2 = str2.split(/\s+/);
1364+
1365+
outer:
1366+
for(var i=0;i<tokens1.length;i++) {
1367+
var token = tokens1[i];
1368+
for(var j=0;j<tokens2.length;j++) {
1369+
if(token == tokens2[j]) continue outer;
1370+
}
1371+
values += (values.length > 0 ? ' ' : '') + token;
1372+
}
1373+
return values;
1374+
}

src/ng/compile.js

+2-18
Original file line numberDiff line numberDiff line change
@@ -688,8 +688,8 @@ function $CompileProvider($provide) {
688688
if(key == 'class') {
689689
value = value || '';
690690
var current = this.$$element.attr('class') || '';
691-
this.$removeClass(tokenDifference(current, value).join(' '));
692-
this.$addClass(tokenDifference(value, current).join(' '));
691+
this.$removeClass(tokenDifference(current, value));
692+
this.$addClass(tokenDifference(value, current));
693693
} else {
694694
var booleanKey = getBooleanAttrName(this.$$element[0], key),
695695
normalizedVal,
@@ -747,22 +747,6 @@ function $CompileProvider($provide) {
747747
$exceptionHandler(e);
748748
}
749749
});
750-
751-
function tokenDifference(str1, str2) {
752-
var values = [],
753-
tokens1 = str1.split(/\s+/),
754-
tokens2 = str2.split(/\s+/);
755-
756-
outer:
757-
for(var i=0;i<tokens1.length;i++) {
758-
var token = tokens1[i];
759-
for(var j=0;j<tokens2.length;j++) {
760-
if(token == tokens2[j]) continue outer;
761-
}
762-
values.push(token);
763-
}
764-
return values;
765-
}
766750
},
767751

768752

src/ng/directive/ngClass.js

+17-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,33 @@ 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+
} else {
48+
addClass(newClasses);
3749
}
38-
addClass(newVal);
3950
}
4051
oldVal = copy(newVal);
4152
}
4253

4354

4455
function removeClass(classVal) {
45-
attr.$removeClass(flattenClasses(classVal));
56+
attr.$removeClass(classVal);
4657
}
4758

4859

4960
function addClass(classVal) {
50-
attr.$addClass(flattenClasses(classVal));
61+
attr.$addClass(classVal);
5162
}
5263

5364
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)