fix(ngAnimate): ensure nested class-based animations are spaced out w…

…ith a RAF

Prior to this fix any nested class-based animations (animations that are
triggered with addClass/removeClass or ngClass) would cancel each other
out when nested in DOM structure. This fix ensures that the nested
animations are spaced out with sequenced RAFs so that parent CSS classes
are applied prior to any ancestor animations that are scheduled to run.

Closes #11812

angularFiles.js

@@ -88,6 +88,7 @@ var angularFiles = {
   'angularModules': {
     'ngAnimate': [
       'src/ngAnimate/shared.js',
+      'src/ngAnimate/rafScheduler.js',
       'src/ngAnimate/animateChildrenDirective.js',
       'src/ngAnimate/animateCss.js',
       'src/ngAnimate/animateCssDriver.js',

src/ngAnimate/animateCss.js

@@ -392,9 +392,9 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
   var gcsStaggerLookup = createLocalCacheLookup();
 
   this.$get = ['$window', '$$jqLite', '$$AnimateRunner', '$timeout',
-               '$document', '$sniffer', '$$rAF',
+               '$document', '$sniffer', '$$rAFScheduler',
        function($window,   $$jqLite,   $$AnimateRunner,   $timeout,
-                $document,   $sniffer,   $$rAF) {
+                $document,   $sniffer,   $$rAFScheduler) {
 
     var applyAnimationClasses = applyAnimationClassesFactory($$jqLite);
 
@@ -452,15 +452,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
     }
 
     var bod = getDomNode($document).body;
-    var cancelLastRAFRequest;
     var rafWaitQueue = [];
     function waitUntilQuiet(callback) {
-      if (cancelLastRAFRequest) {
-        cancelLastRAFRequest(); //cancels the request
-      }
       rafWaitQueue.push(callback);
-      cancelLastRAFRequest = $$rAF(function() {
-        cancelLastRAFRequest = null;
+      $$rAFScheduler.waitUntilQuiet(function() {
         gcsLookup.flush();
         gcsStaggerLookup.flush();
 

src/ngAnimate/animateQueue.js

@@ -361,7 +361,9 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
         return runner;
       }
 
-      closeParentClassBasedAnimations(parent);
+      if (isStructural) {
+        closeParentClassBasedAnimations(parent);
+      }
 
       // the counter keeps track of cancelled animations
       var counter = (existingAnimation.counter || 0) + 1;
@@ -420,7 +422,9 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
             ? 'setClass'
             : animationDetails.event;
 
-        closeParentClassBasedAnimations(parentElement);
+        if (animationDetails.structural) {
+          closeParentClassBasedAnimations(parentElement);
+        }
 
         markElementAnimationState(element, RUNNING_STATE);
         var realRunner = $$animation(element, event, animationDetails.options);

src/ngAnimate/animation.js

@@ -19,12 +19,16 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
     return element.data(RUNNER_STORAGE_KEY);
   }
 
-  this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner',
-       function($$jqLite,   $rootScope,   $injector,   $$AnimateRunner) {
+  this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$rAFScheduler',
+       function($$jqLite,   $rootScope,   $injector,   $$AnimateRunner,   $$rAFScheduler) {
 
     var animationQueue = [];
     var applyAnimationClasses = applyAnimationClassesFactory($$jqLite);
 
+    var totalPendingClassBasedAnimations = 0;
+    var totalActiveClassBasedAnimations = 0;
+    var classBasedAnimationsQueue = [];
+
     // TODO(matsko): document the signature in a better way
     return function(element, event, options) {
       options = prepareAnimationOptions(options);
@@ -53,12 +57,19 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
         options.tempClasses = null;
       }
 
+      var classBasedIndex;
+      if (!isStructural) {
+        classBasedIndex = totalPendingClassBasedAnimations;
+        totalPendingClassBasedAnimations += 1;
+      }
+
       animationQueue.push({
         // this data is used by the postDigest code and passed into
         // the driver step function
         element: element,
         classes: classes,
         event: event,
+        classBasedIndex: classBasedIndex,
         structural: isStructural,
         options: options,
         beforeStart: beforeStart,
@@ -73,6 +84,10 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
       if (animationQueue.length > 1) return runner;
 
       $rootScope.$$postDigest(function() {
+        totalActiveClassBasedAnimations = totalPendingClassBasedAnimations;
+        totalPendingClassBasedAnimations = 0;
+        classBasedAnimationsQueue.length = 0;
+
         var animations = [];
         forEach(animationQueue, function(entry) {
           // the element was destroyed early on which removed the runner
@@ -87,23 +102,58 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
         animationQueue.length = 0;
 
         forEach(groupAnimations(animations), function(animationEntry) {
-          // it's important that we apply the `ng-animate` CSS class and the
-          // temporary classes before we do any driver invoking since these
-          // CSS classes may be required for proper CSS detection.
-          animationEntry.beforeStart();
-
-          var operation = invokeFirstDriver(animationEntry);
-          var triggerAnimationStart = operation && operation.start; /// TODO(matsko): only recognize operation.start()
-
-          var closeFn = animationEntry.close;
-          if (!triggerAnimationStart) {
-            closeFn();
+          if (animationEntry.structural) {
+            triggerAnimationStart();
           } else {
-            var animationRunner = triggerAnimationStart();
-            animationRunner.done(function(status) {
-              closeFn(!status);
+            classBasedAnimationsQueue.push({
+              node: getDomNode(animationEntry.element),
+              fn: triggerAnimationStart
             });
-            updateAnimationRunners(animationEntry, animationRunner);
+
+            if (animationEntry.classBasedIndex === totalActiveClassBasedAnimations - 1) {
+              // we need to sort each of the animations in order of parent to child
+              // relationships. This ensures that the child classes are applied at the
+              // right time.
+              classBasedAnimationsQueue = classBasedAnimationsQueue.sort(function(a,b) {
+                return b.node.contains(a.node);
+              }).map(function(entry) {
+                return entry.fn;
+              });
+
+              $$rAFScheduler(classBasedAnimationsQueue);
+            }
+          }
+
+          function triggerAnimationStart() {
+            // it's important that we apply the `ng-animate` CSS class and the
+            // temporary classes before we do any driver invoking since these
+            // CSS classes may be required for proper CSS detection.
+            animationEntry.beforeStart();
+
+            var startAnimationFn, closeFn = animationEntry.close;
+
+            // in the event that the element was removed before the digest runs or
+            // during the RAF sequencing then we should not trigger the animation.
+            var targetElement = animationEntry.anchors
+                ? (animationEntry.from.element || animationEntry.to.element)
+                : animationEntry.element;
+
+            if (getRunner(targetElement)) {
+              var operation = invokeFirstDriver(animationEntry);
+              if (operation) {
+                startAnimationFn = operation.start;
+              }
+            }
+
+            if (!startAnimationFn) {
+              closeFn();
+            } else {
+              var animationRunner = startAnimationFn();
+              animationRunner.done(function(status) {
+                closeFn(!status);
+              });
+              updateAnimationRunners(animationEntry, animationRunner);
+            }
           }
         });
       });
@@ -175,7 +225,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
           var lookupKey = from.animationID.toString();
           if (!anchorGroups[lookupKey]) {
             var group = anchorGroups[lookupKey] = {
-              // TODO(matsko): double-check this code
+              structural: true,
               beforeStart: function() {
                 fromAnimation.beforeStart();
                 toAnimation.beforeStart();

src/ngAnimate/module.js

@@ -3,6 +3,7 @@
 /* global angularAnimateModule: true,
 
    $$rAFMutexFactory,
+   $$rAFSchedulerFactory,
    $$AnimateChildrenDirective,
    $$AnimateRunnerFactory,
    $$AnimateQueueProvider,
@@ -742,6 +743,7 @@ angular.module('ngAnimate', [])
   .directive('ngAnimateChildren', $$AnimateChildrenDirective)
 
   .factory('$$rAFMutex', $$rAFMutexFactory)
+  .factory('$$rAFScheduler', $$rAFSchedulerFactory)
 
   .factory('$$AnimateRunner', $$AnimateRunnerFactory)
 

src/ngAnimate/rafScheduler.js

@@ -0,0 +1,59 @@
+'use strict';
+
+var $$rAFSchedulerFactory = ['$$rAF', function($$rAF) {
+  var tickQueue = [];
+  var cancelFn;
+
+  function scheduler(tasks) {
+    // we make a copy since RAFScheduler mutates the state
+    // of the passed in array variable and this would be difficult
+    // to track down on the outside code
+    tickQueue.push([].concat(tasks));
+    nextTick();
+  }
+
+  /* waitUntilQuiet does two things:
+   * 1. It will run the FINAL `fn` value only when an uncancelled RAF has passed through
+   * 2. It will delay the next wave of tasks from running until the quiet `fn` has run.
+   *
+   * The motivation here is that animation code can request more time from the scheduler
+   * before the next wave runs. This allows for certain DOM properties such as classes to
+   * be resolved in time for the next animation to run.
+   */
+  scheduler.waitUntilQuiet = function(fn) {
+    if (cancelFn) cancelFn();
+
+    cancelFn = $$rAF(function() {
+      cancelFn = null;
+      fn();
+      nextTick();
+    });
+  };
+
+  return scheduler;
+
+  function nextTick() {
+    if (!tickQueue.length) return;
+
+    var updatedQueue = [];
+    for (var i = 0; i < tickQueue.length; i++) {
+      var innerQueue = tickQueue[i];
+      runNextTask(innerQueue);
+      if (innerQueue.length) {
+        updatedQueue.push(innerQueue);
+      }
+    }
+    tickQueue = updatedQueue;
+
+    if (!cancelFn) {
+      $$rAF(function() {
+        if (!cancelFn) nextTick();
+      });
+    }
+  }
+
+  function runNextTask(tasks) {
+    var nextTask = tasks.shift();
+    nextTask();
+  }
+}];

test/ngAnimate/animationSpec.js

@@ -288,6 +288,90 @@ describe('$$animation', function() {
         };
       }));
 
+      it('should space out multiple ancestorial class-based animations with a RAF in between',
+        inject(function($rootScope, $$animation, $$rAF) {
+
+        var parent = element;
+        element = jqLite('<div></div>');
+        parent.append(element);
+
+        var child = jqLite('<div></div>');
+        element.append(child);
+
+        $$animation(parent, 'addClass', { addClass: 'blue' });
+        $$animation(element, 'addClass', { addClass: 'red' });
+        $$animation(child, 'addClass', { addClass: 'green' });
+
+        $rootScope.$digest();
+
+        expect(captureLog.length).toBe(1);
+        expect(capturedAnimation.options.addClass).toBe('blue');
+
+        $$rAF.flush();
+        expect(captureLog.length).toBe(2);
+        expect(capturedAnimation.options.addClass).toBe('red');
+
+        $$rAF.flush();
+        expect(captureLog.length).toBe(3);
+        expect(capturedAnimation.options.addClass).toBe('green');
+      }));
+
+      it('should properly cancel out pending animations that are spaced with a RAF request before the digest completes',
+        inject(function($rootScope, $$animation, $$rAF) {
+
+        var parent = element;
+        element = jqLite('<div></div>');
+        parent.append(element);
+
+        var child = jqLite('<div></div>');
+        element.append(child);
+
+        var r1 = $$animation(parent, 'addClass', { addClass: 'blue' });
+        var r2 = $$animation(element, 'addClass', { addClass: 'red' });
+        var r3 = $$animation(child, 'addClass', { addClass: 'green' });
+
+        r2.end();
+
+        $rootScope.$digest();
+
+        expect(captureLog.length).toBe(1);
+        expect(capturedAnimation.options.addClass).toBe('blue');
+
+        $$rAF.flush();
+
+        expect(captureLog.length).toBe(2);
+        expect(capturedAnimation.options.addClass).toBe('green');
+      }));
+
+      it('should properly cancel out pending animations that are spaced with a RAF request after the digest completes',
+        inject(function($rootScope, $$animation, $$rAF) {
+
+        var parent = element;
+        element = jqLite('<div></div>');
+        parent.append(element);
+
+        var child = jqLite('<div></div>');
+        element.append(child);
+
+        var r1 = $$animation(parent, 'addClass', { addClass: 'blue' });
+        var r2 = $$animation(element, 'addClass', { addClass: 'red' });
+        var r3 = $$animation(child, 'addClass', { addClass: 'green' });
+
+        $rootScope.$digest();
+
+        r2.end();
+
+        expect(captureLog.length).toBe(1);
+        expect(capturedAnimation.options.addClass).toBe('blue');
+
+        $$rAF.flush();
+        expect(captureLog.length).toBe(1);
+
+        $$rAF.flush();
+        expect(captureLog.length).toBe(2);
+        expect(capturedAnimation.options.addClass).toBe('green');
+      }));
+
       they('should return a runner that object that contains a $prop() function',
         ['end', 'cancel', 'then'], function(method) {
         inject(function($$animation) {
@@ -513,7 +597,7 @@ describe('$$animation', function() {
       }));
 
       it("should not group animations into an anchored animation if enter/leave events are NOT used",
-        inject(function($$animation, $rootScope) {
+        inject(function($$animation, $rootScope, $$rAF) {
 
         fromElement.addClass('shared-class');
         fromElement.attr('ng-animate-ref', '1');
@@ -528,6 +612,7 @@ describe('$$animation', function() {
         });
 
         $rootScope.$digest();
+        $$rAF.flush();
         expect(captureLog.length).toBe(2);
       }));