fix($animate): only cancel class-based animations if the follow-up class contains CSS transition/keyframe animation code

matsko · matsko · commit f5289fe84ffc · 2013-10-23T07:08:03.000-04:00
Closes angular#4463 Closes angular#3784
diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js
@@ -493,35 +493,47 @@ angular.module('ngAnimate', ['ng'])
       */
       function performAnimation(event, className, element, parent, after, onComplete) {
         var classes = (element.attr('class') || '') + ' ' + className;
-        var animationLookup = (' ' + classes).replace(/\s+/g,'.'),
-            animations = [];
-        forEach(lookup(animationLookup), function(animation, index) {
-          animations.push({
-            start : animation[event]
-          });
-        });
-
+        var animationLookup = (' ' + classes).replace(/\s+/g,'.');
         if (!parent) {
           parent = after ? after.parent() : element.parent();
         }
+
         var disabledAnimation = { running : true };
+        var matches = lookup(animationLookup);
+        var isClassBased = event == 'addClass' || event == 'removeClass';
+        var ngAnimateState = element.data(NG_ANIMATE_STATE) || {};
 
-        //skip the animation if animations are disabled, a parent is already being animated
-        //or the element is not currently attached to the document body.
-        if ((parent.inheritedData(NG_ANIMATE_STATE) || disabledAnimation).running || animations.length === 0) {
+        //skip the animation if animations are disabled, a parent is already being animated,
+        //the element is not currently attached to the document body or then completely close
+        //the animation if any matching animations are not found at all.
+        //NOTE: IE8 + IE9 should close properly (run done()) in case a NO animation is not found.
+        if ((parent.inheritedData(NG_ANIMATE_STATE) || disabledAnimation).running || matches.length == 0) {
           done();
           return;
         }
 
-        var ngAnimateState = element.data(NG_ANIMATE_STATE) || {};
+        var animations = [];
+        //only add animations if the currently running animation is not structural
+        //or if there is no animation running at all
+        if(!ngAnimateState.running || !(isClassBased && ngAnimateState.structural)) {
+          forEach(matches, function(animation) {
+            //add the animation to the queue to if it is allowed to be cancelled
+            if(!animation.allowCancel || animation.allowCancel(element, event, className)) {
+              animations.push({
+                start : animation[event]
+              });
+            }
+          });
+        }
 
-        var isClassBased = event == 'addClass' || event == 'removeClass';
-        if(ngAnimateState.running) {
-          if(isClassBased && ngAnimateState.structural) {
-            onComplete && onComplete();
-            return;
-          }
+        //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) {
+          onComplete && onComplete();
+          return;
+        }
 
+        if(ngAnimateState.running) {
           //if an animation is currently running on the element then lets take the steps
           //to cancel that animation and fire any required callbacks
           $timeout.cancel(ngAnimateState.flagTimer);
@@ -651,6 +663,7 @@ angular.module('ngAnimate', ['ng'])
           animationIterationCountKey = 'IterationCount';
 
       var NG_ANIMATE_PARENT_KEY = '$ngAnimateKey';
+      var NG_ANIMATE_CLASS_KEY = '$$ngAnimateClasses';
       var lookupCache = {};
       var parentCounter = 0;
 
@@ -669,7 +682,7 @@ angular.module('ngAnimate', ['ng'])
       }
 
       function getElementAnimationDetails(element, cacheKey, onlyCheckTransition) {
-        var data = lookupCache[cacheKey];
+        var data = cacheKey ? lookupCache[cacheKey] : null;
         if(!data) {
           var transitionDuration = 0, transitionDelay = 0,
               animationDuration = 0, animationDelay = 0;
@@ -702,7 +715,9 @@ angular.module('ngAnimate', ['ng'])
             transitionDuration : transitionDuration,
             animationDuration : animationDuration
           };
-          lookupCache[cacheKey] = data;
+          if(cacheKey) {
+            lookupCache[cacheKey] = data;
+          }
         }
         return data;
       }
@@ -769,6 +784,7 @@ angular.module('ngAnimate', ['ng'])
             element.addClass(activeClassName);
           });
 
+          element.data(NG_ANIMATE_CLASS_KEY, className + ' ' + activeClassName);
           element.on(css3AnimationEvents, onAnimationProgress);
 
           // This will automatically be called by $animate so
@@ -778,6 +794,7 @@ angular.module('ngAnimate', ['ng'])
             element.off(css3AnimationEvents, onAnimationProgress);
             element.removeClass(className);
             element.removeClass(activeClassName);
+            element.removeData(NG_ANIMATE_CLASS_KEY);
 
             // Only when the animation is cancelled is the done()
             // function not called for this animation therefore
@@ -811,6 +828,35 @@ angular.module('ngAnimate', ['ng'])
       }
 
       return {
+        allowCancel : function(element, event, className) {
+          //always cancel the current animation if it is a
+          //structural animation
+          var oldClasses = element.data(NG_ANIMATE_CLASS_KEY);
+          if(!oldClasses || ['enter','leave','move'].indexOf(event) >= 0) {
+            return true;
+          }
+
+          var parent = element.parent();
+          var clone = angular.element(element[0].cloneNode());
+
+          //make the element super hidden and override any CSS style values
+          clone.attr('style','position:absolute; top:-9999px; left:-9999px');
+          clone.removeAttr('id');
+          clone.html('');
+
+          angular.forEach(oldClasses.split(' '), function(klass) {
+            clone.removeClass(klass);
+          });
+
+          var suffix = event == 'addClass' ? '-add' : '-remove';
+          clone.addClass(suffixClasses(className, suffix));
+          parent.append(clone);
+
+          var timings = getElementAnimationDetails(clone);
+          clone.remove();
+
+          return Math.max(timings.transitionDuration, timings.animationDuration) > 0;
+        },
         enter : function(element, done) {
           return animate(element, 'ng-enter', done);
         },
diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js
@@ -746,8 +746,8 @@ describe("ngAnimate", function() {
           expect(element.hasClass('ng-enter')).toBe(true);
           expect(element.hasClass('ng-enter-active')).toBe(true);
           browserTrigger(element,'transitionend', { timeStamp: Date.now() + 22000, elapsedTime: 22000 });
+          $timeout.flush();
         }
-        $timeout.flush();
         expect(element.hasClass('abc')).toBe(true);
 
         $rootScope.klass = 'xyz';
@@ -760,8 +760,8 @@ describe("ngAnimate", function() {
           expect(element.hasClass('ng-enter')).toBe(true);
           expect(element.hasClass('ng-enter-active')).toBe(true);
           browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000, elapsedTime: 11000 });
+          $timeout.flush();
         }
-        $timeout.flush();
         expect(element.hasClass('xyz')).toBe(true);
       }));
 
@@ -1920,4 +1920,40 @@ describe("ngAnimate", function() {
       expect(count).toBe(40);
     });
   });
+
+  it("should cancel an ongoing class-based animation only if the new class contains transition/animation CSS code",
+    inject(function($compile, $rootScope, $animate, $sniffer) {
+
+    if (!$sniffer.transitions) return;
+
+    ss.addRule('.green-add', '-webkit-transition:1s linear all;' +
+                                     'transition:1s linear all;');
+
+    ss.addRule('.blue-add', 'background:blue;');
+
+    ss.addRule('.red-add', '-webkit-transition:1s linear all;' +
+                                   'transition:1s linear all;');
+
+    ss.addRule('.yellow-add', '-webkit-animation: some_animation 4s linear 1s 2 alternate;' +
+                                      'animation: some_animation 4s linear 1s 2 alternate;');
+
+    var element = $compile('<div></div>')($rootScope);
+    $rootElement.append(element);
+    jqLite($document[0].body).append($rootElement);
+
+    $animate.addClass(element, 'green');
+    expect(element.hasClass('green-add')).toBe(true);
+ 
+    $animate.addClass(element, 'blue');
+    expect(element.hasClass('blue')).toBe(true); 
+    expect(element.hasClass('green-add')).toBe(true); //not cancelled
+
+    $animate.addClass(element, 'red');
+    expect(element.hasClass('green-add')).toBe(false);
+    expect(element.hasClass('red-add')).toBe(true);
+
+    $animate.addClass(element, 'yellow');
+    expect(element.hasClass('red-add')).toBe(false); 
+    expect(element.hasClass('yellow-add')).toBe(true);
+  }));
 });
