fix($animate): skip unnecessary addClass/removeClass animations

matsko · matsko · commit 76b628bcb351 · 2013-10-24T17:35:57.000-04:00
Skip addClass animations if the element already contains the class that is being added to element. Also skip removeClass animations if the element does not contain the class that is being removed. Closes angular#4401 Closes angular#2332
diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js
@@ -541,6 +541,16 @@ angular.module('ngAnimate', ['ng'])
           (ngAnimateState.done || noop)();
         }
 
+        //There is no point in perform a class-based animation if the element already contains
+        //(on addClass) or doesn't contain (on removeClass) the className being animated.
+        //The reason why this is being called after the previous animations are cancelled
+        //is so that the CSS classes present on the element can be properly examined.
+        if((event == 'addClass'    && element.hasClass(className)) ||
+           (event == 'removeClass' && !element.hasClass(className))) {
+          onComplete && onComplete();
+          return;
+        }
+
         element.data(NG_ANIMATE_STATE, {
           running:true,
           structural:!isClassBased,
diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js
@@ -345,6 +345,7 @@ describe("ngAnimate", function() {
 
         $animate.enabled(true);
 
+        element.addClass('ng-hide');
         $animate.removeClass(element, 'ng-hide');
         expect(element.text()).toBe('memento');
       }));
@@ -616,7 +617,7 @@ describe("ngAnimate", function() {
             ss.addRule('.ng-hide-add', style);
             ss.addRule('.ng-hide-remove', style);
 
-            element = $compile(html('<div>1</div>'))($rootScope);
+            element = $compile(html('<div class="ng-hide">1</div>'))($rootScope);
             element.addClass('custom');
 
             $animate.removeClass(element, 'ng-hide');
@@ -627,6 +628,7 @@ describe("ngAnimate", function() {
               expect(element.hasClass('ng-hide-remove-active')).toBe(true);
             }
 
+            element.removeClass('ng-hide');
             $animate.addClass(element, 'ng-hide');
             expect(element.hasClass('ng-hide-remove')).toBe(false); //added right away
 
@@ -1052,21 +1054,59 @@ describe("ngAnimate", function() {
     });
 
     describe("addClass / removeClass", function() {
+      var captured;
       beforeEach(function() {
         module(function($animateProvider, $provide) {
           $animateProvider.register('.klassy', function($timeout) {
             return {
               addClass : function(element, className, done) {
+                captured = 'addClass-' + className;
                 $timeout(done, 500, false);
               },
               removeClass : function(element, className, done) {
+                captured = 'removeClass-' + className;
                 $timeout(done, 3000, false);
               }
             }
           });
         });
       });
 
+      it("should not perform an animation, and the followup DOM operation, if the class is " +
+         "already present during addClass or not present during removeClass on the element",
+        inject(function($animate, $rootScope, $sniffer, $rootElement, $timeout) {
+
+        var element = jqLite('<div class="klassy"></div>');
+        $rootElement.append(element);
+        body.append($rootElement);
+
+        //skipped animations
+        captured = 'none';
+        $animate.removeClass(element, 'some-class');
+        expect(element.hasClass('some-class')).toBe(false);
+        expect(captured).toBe('none');
+
+        element.addClass('some-class');
+
+        captured = 'nothing';
+        $animate.addClass(element, 'some-class');
+        expect(captured).toBe('nothing');
+        expect(element.hasClass('some-class')).toBe(true);
+
+        //actual animations
+        captured = 'none';
+        $animate.removeClass(element, 'some-class');
+        $timeout.flush();
+        expect(element.hasClass('some-class')).toBe(false);
+        expect(captured).toBe('removeClass-some-class');
+
+        captured = 'nothing';
+        $animate.addClass(element, 'some-class');
+        $timeout.flush();
+        expect(element.hasClass('some-class')).toBe(true);
+        expect(captured).toBe('addClass-some-class');
+      }));
+
       it("should add and remove CSS classes after an animation even if no animation is present",
         inject(function($animate, $rootScope, $sniffer, $rootElement) {
 
