ngClass + ngAnimate fixes

src/.jshintrc

@@ -100,6 +100,7 @@
     "assertNotHasOwnProperty": false,
     "getter": false,
     "getBlockElements": false,
+    "tokenDifference": false,
 
     /* AngularPublic.js */
     "version": false,
@@ -162,4 +163,4 @@
     "nullFormCtrl": false
 
   }
-}
+}

src/Angular.js

@@ -80,7 +80,8 @@
     -assertArgFn,
     -assertNotHasOwnProperty,
     -getter,
-    -getBlockElements
+    -getBlockElements,
+    -tokenDifference
 
 */
 
@@ -1338,3 +1339,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;
+}

src/ng/compile.js

@@ -677,8 +677,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,
@@ -736,22 +736,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;
-        }
       },
 
 

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,34 @@ 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) {

src/ngAnimate/animate.js

@@ -563,7 +563,7 @@ angular.module('ngAnimate', ['ng'])
         //the animation if any matching animations are not found at all.
         //NOTE: IE8 + IE9 should close properly (run closeAnimation()) in case a NO animation is not found.
         if (animationsDisabled(element, parentElement) || matches.length === 0) {
-          domOperation();
+          fireDOMOperation();
           closeAnimation();
           return;
         }
@@ -596,7 +596,7 @@ angular.module('ngAnimate', ['ng'])
         //this would mean that an animation was not allowed so let the existing
         //animation do it's thing and close this one early
         if(animations.length === 0) {
-          domOperation();
+          fireDOMOperation();
           fireDoneCallbackAsync();
           return;
         }
@@ -616,7 +616,7 @@ angular.module('ngAnimate', ['ng'])
         //is so that the CSS classes present on the element can be properly examined.
         if((animationEvent == 'addClass'    && element.hasClass(className)) ||
            (animationEvent == 'removeClass' && !element.hasClass(className))) {
-          domOperation();
+          fireDOMOperation();
           fireDoneCallbackAsync();
           return;
         }
@@ -627,6 +627,7 @@ angular.module('ngAnimate', ['ng'])
 
         element.data(NG_ANIMATE_STATE, {
           running:true,
+          className:className,
           structural:!isClassBased,
           animations:animations,
           done:onBeforeAnimationsComplete
@@ -637,7 +638,7 @@ angular.module('ngAnimate', ['ng'])
         invokeRegisteredAnimationFns(animations, 'before', onBeforeAnimationsComplete);
 
         function onBeforeAnimationsComplete(cancelled) {
-          domOperation();
+          fireDOMOperation();
           if(cancelled === true) {
             closeAnimation();
             return;
@@ -695,6 +696,15 @@ angular.module('ngAnimate', ['ng'])
           doneCallback && $timeout(doneCallback, 0, false);
         }
 
+        //it is less complicated to use a flag than managing and cancelling
+        //timeouts containing multiple callbacks.
+        function fireDOMOperation() {
+          if(!fireDOMOperation.hasBeenRun) {
+            fireDOMOperation.hasBeenRun = true;
+            domOperation();
+          }
+        }
+
         function closeAnimation() {
           if(!closeAnimation.hasBeenRun) {
             closeAnimation.hasBeenRun = true;

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);
+    });
+  });
 });

test/ngAnimate/animateSpec.js

@@ -2599,4 +2599,38 @@ describe("ngAnimate", function() {
     });
   });
 
+  it('should only perform the DOM operation once',
+    inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) {
+
+    if (!$sniffer.transitions) return;
+
+    ss.addRule('.base-class', '-webkit-transition:1s linear all;' +
+                                      'transition:1s linear all;');
+
+    $animate.enabled(true);
+
+    var element = $compile('<div class="base-class one two"></div>')($rootScope);
+    $rootElement.append(element);
+    jqLite($document[0].body).append($rootElement);
+
+    $animate.removeClass(element, 'base-class one two');
+
+    //still true since we're before the reflow
+    expect(element.hasClass('base-class')).toBe(true);
+
+    //this will cancel the remove animation
+    $animate.addClass(element, 'base-class one two');
+
+    //the cancellation was a success and the class was added right away
+    //since there was no successive animation for the after animation
+    expect(element.hasClass('base-class')).toBe(true);
+
+    //the reflow...
+    $timeout.flush();
+
+    //the reflow DOM operation was commenced but it ran before so it
+    //shouldn't run agaun
+    expect(element.hasClass('base-class')).toBe(true);
+  }));
+
 });