fix(ngAnimate): ensure that repeated structural calls during pre-digest function

matsko · matsko · commit 2327f5a0a7e0 · 2015-05-20T20:34:25.000-07:00
Prior to this fix if `$animate.enter()` or `$animate.leave()` was called before a digest was issued then the element may not be cancelled early enough. This fix ensures that the previous structural animation is cancelled immediately when a follow-up animation is kicked off. Closes angular#11867
diff --git a/src/ngAnimate/animateQueue.js b/src/ngAnimate/animateQueue.js
@@ -292,6 +292,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
         structural: isStructural,
         element: element,
         event: event,
+        close: close,
         options: options,
         runner: runner
       };
@@ -311,8 +312,17 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
         var cancelAnimationFlag = isAllowed('cancel', element, newAnimation, existingAnimation);
         if (cancelAnimationFlag) {
           if (existingAnimation.state === RUNNING_STATE) {
+            // this will end the animation right away and it is safe
+            // to do so since the animation is already running and the
+            // runner callback code will run in async
             existingAnimation.runner.end();
+          } else if (existingAnimation.structural) {
+            // this means that the animation is queued into a digest, but
+            // hasn't started yet. Therefore it is safe to run the close
+            // method which will call the runner methods in async.
+            existingAnimation.close();
           } else {
+            // this will merge the existing animation options into this new follow-up animation
             mergeAnimationOptions(element, newAnimation.options, existingAnimation.options);
           }
         } else {
diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js
@@ -1003,6 +1003,18 @@ describe("animations", function() {
         expect(enterComplete).toBe(true);
       }));
 
+      it('should cancel the previous structural animation if a follow-up structural animation takes over before the postDigest',
+        inject(function($animate, $$rAF) {
+
+        var enterDone = jasmine.createSpy('enter animation done');
+        $animate.enter(element, parent).done(enterDone);
+        expect(enterDone).not.toHaveBeenCalled();
+
+        $animate.leave(element);
+        $$rAF.flush();
+        expect(enterDone).toHaveBeenCalled();
+      }));
+
       it('should skip the class-based animation entirely if there is an active structural animation',
         inject(function($animate, $rootScope) {
 
@@ -1152,12 +1164,20 @@ describe("animations", function() {
       it('class-based animations, however it should also cancel former structural animations in the process',
         inject(function($animate, $rootScope) {
 
-        element.addClass('green');
+        element.addClass('green lime');
 
         $animate.enter(element, parent);
         $animate.addClass(element, 'red');
         $animate.removeClass(element, 'green');
+
         $animate.leave(element);
+        $animate.addClass(element, 'pink');
+        $animate.removeClass(element, 'lime');
+
+        expect(element).toHaveClass('red');
+        expect(element).not.toHaveClass('green');
+        expect(element).not.toHaveClass('pink');
+        expect(element).toHaveClass('lime');
 
         $rootScope.$digest();
 
@@ -1168,8 +1188,8 @@ describe("animations", function() {
         expect(element.parent()[0]).toEqual(parent[0]);
 
         options = capturedAnimation[2];
-        expect(options.addClass).toEqual('red');
-        expect(options.removeClass).toEqual('green');
+        expect(options.addClass).toEqual('pink');
+        expect(options.removeClass).toEqual('lime');
       }));
 
       it('should retain the instance to the very first runner object when multiple element-level animations are issued',
