From 6b8bbe4d90640542eed5607a8c91f6b977b1d6c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Matias=20Niemel=C3=A4?= <matias@yearofmoo.com>
Date: Thu, 14 Nov 2013 23:45:22 -0500
Subject: [PATCH] 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 #4960
Closes #4944
---
 src/.jshintrc                    |  3 ++-
 src/Angular.js                   | 24 +++++++++++++++++-
 src/ng/compile.js                | 20 ++-------------
 src/ng/directive/ngClass.js      | 23 ++++++++++++-----
 test/ng/directive/ngClassSpec.js | 43 ++++++++++++++++++++++++++++++++
 5 files changed, 87 insertions(+), 26 deletions(-)

diff --git a/src/.jshintrc b/src/.jshintrc
index e29b09f3a848..2467d667e896 100644
--- a/src/.jshintrc
+++ b/src/.jshintrc
@@ -100,6 +100,7 @@
     "assertNotHasOwnProperty": false,
     "getter": false,
     "getBlockElements": false,
+    "tokenDifference": false,
 
     /* AngularPublic.js */
     "version": false,
@@ -162,4 +163,4 @@
     "nullFormCtrl": false
 
   }
-}
\ No newline at end of file
+}
diff --git a/src/Angular.js b/src/Angular.js
index 8409f971f2ca..b27f4b068a01 100644
--- a/src/Angular.js
+++ b/src/Angular.js
@@ -80,7 +80,8 @@
     -assertArgFn,
     -assertNotHasOwnProperty,
     -getter,
-    -getBlockElements
+    -getBlockElements,
+    -tokenDifference
 
 */
 
@@ -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;
+}
diff --git a/src/ng/compile.js b/src/ng/compile.js
index 4d83f379d22f..ce3d0514e7bb 100644
--- a/src/ng/compile.js
+++ b/src/ng/compile.js
@@ -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,
@@ -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;
-        }
       },
 
 
diff --git a/src/ng/directive/ngClass.js b/src/ng/directive/ngClass.js
index a0d23a286e28..10ef7fd1f94b 100644
--- a/src/ng/directive/ngClass.js
+++ b/src/ng/directive/ngClass.js
@@ -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])));
               }
             }
           });
@@ -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) {
diff --git a/test/ng/directive/ngClassSpec.js b/test/ng/directive/ngClassSpec.js
index ab996e4d6f1e..62733c85beb7 100644
--- a/test/ng/directive/ngClassSpec.js
+++ b/test/ng/directive/ngClassSpec.js
@@ -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);
+    });
+  });
 });