fix($animateCss): ensure that an object is always returned even when no animation is set to run

matsko · matsko · commit d5683d21165e · 2015-05-07T14:33:20.000-07:00
Before in RC0 and RC1 $animateCss would not return anything if a
CSS-based animation was not detected. This was a messy API decision
which resulted in the user having to have an if statement to handle the
failure case. This patch ensures that an animator object with the start()
and end() functions is always returned. If an animation is not detected
then the preperatory CSS styles and classes are removed immediately and
the element is cleaned up, however a "dump" animator object is still
returned which allows for callbacks and promises to be applied.

The returned object now also contains a `valid` property which can be
examined to determine whether an animation is set to run on the element.

BREAKING CHANGE: The $animateCss service will now always return an
object even if the animation is not set to run. If your code is using
$animateCss then please consider the following code change:

```
// before
var animator = $animateCss(element, { ... });
if (!animator) {
  continueApp();
  return;
}
var runner = animator.start();
runner.done(continueApp);
runner.then(continueApp);

// now
var animator = $animateCss(element, { ... });
var runner = animator.start();
runner.done(continueApp);
runner.then(continueApp);
```
diff --git a/src/ngAnimate/animateCss.js b/src/ngAnimate/animateCss.js
@@ -39,16 +39,12 @@
  *   return {
  *     enter: function(element, doneFn) {
  *       var height = element[0].offsetHeight;
- *       var animation = $animateCss(element, {
+ *       var animator = $animateCss(element, {
  *         from: { height:'0px' },
  *         to: { height:height + 'px' },
  *         duration: 1 // one second
  *       });
- *
- *       // if no possible animation can be triggered due
- *       // to the combination of options then `animation`
- *       // will be returned as undefined
- *       animation.start().done(doneFn);
+ *       animator.start().done(doneFn);
  *     }
  *   }
  * }]);
@@ -71,18 +67,14 @@
  *   return {
  *     enter: function(element, doneFn) {
  *       var height = element[0].offsetHeight;
- *       var animation = $animateCss(element, {
+ *       var animator = $animateCss(element, {
  *         addClass: 'red large-text pulse-twice',
  *         easing: 'ease-out',
  *         from: { height:'0px' },
  *         to: { height:height + 'px' },
  *         duration: 1 // one second
  *       });
- *
- *       // if no possible animation can be triggered due
- *       // to the combination of options then `animation`
- *       // will be returned as undefined
- *       animation.start().done(doneFn);
+ *       animator.start().done(doneFn);
  *     }
  *   }
  * }]);
@@ -122,10 +114,11 @@
  * styles using the `from` and `to` properties.
  *
  * ```js
- * var animation = $animateCss(element, {
+ * var animator = $animateCss(element, {
  *   from: { background:'red' },
  *   to: { background:'blue' }
  * });
+ * animator.start();
  * ```
  *
  * ```css
@@ -158,10 +151,10 @@
  * added and removed on the element). Once `$animateCss` is called it will return an object with the following properties:
  *
  * ```js
- * var animation = $animateCss(element, { ... });
+ * var animator = $animateCss(element, { ... });
  * ```
  *
- * Now what do the contents of our `animation` variable look like:
+ * Now what do the contents of our `animator` variable look like:
  *
  * ```js
  * {
@@ -185,18 +178,11 @@
  * The example below should put this into perspective:
  *
  * ```js
- * var animation = $animateCss(element, { ... });
- *
- * // remember that if there is no CSS animation detected on the element
- * // then the value returned from $animateCss will be null
- * if (animation) {
- *   animation.start().done(function() {
- *     // yaay the animation is over
- *     doneCallback();
- *   });
- * } else {
+ * var animator = $animateCss(element, { ... });
+ * animator.start().done(function() {
+ *   // yaay the animation is over
  *   doneCallback();
- * }
+ * });
  * ```
  *
  * @param {DOMElement} element the element that will be animated
@@ -223,7 +209,7 @@
  * `stagger` option value of `0.1` is used then there will be a stagger delay of `600ms`)
  * `applyClassesEarly` - Whether or not the classes being added or removed will be used when detecting the animation. This is set by `$animate` when enter/leave/move animations are fired to ensure that the CSS classes are resolved in time. (Note that this will prevent any transitions from occuring on the classes being added and removed.)
  *
- * @return {null|object} an object with a start method and details about the animation. If no animation is detected then a value of `null` will be returned.
+ * @return {object} an object with start and end methods and details about the animation.
  *
  * * `start` - The method to start the animation. This will return a `Promise` when called.
  * * `end` - This method will cancel the animation and remove all applied CSS classes and styles.
@@ -538,8 +524,7 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
       var maxDurationTime;
 
       if (options.duration === 0 || (!$sniffer.animations && !$sniffer.transitions)) {
-        close();
-        return;
+        return closeAndReturnNoopAnimator();
       }
 
       var method = options.event && isArray(options.event)
@@ -586,8 +571,7 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
       // there is no way we can trigger an animation since no styles and
       // no classes are being applied which would then trigger a transition
       if (!hasToStyles && !setupClasses) {
-        close();
-        return false;
+        return closeAndReturnNoopAnimator();
       }
 
       var cacheKey, stagger;
@@ -682,8 +666,7 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
       }
 
       if (maxDuration === 0 && !flags.recalculateTimingStyles) {
-        close();
-        return false;
+        return closeAndReturnNoopAnimator();
       }
 
       // we need to recalculate the delay value since we used a pre-emptive negative
@@ -711,6 +694,7 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
 
       // TODO(matsko): for 1.5 change this code to have an animator object for better debugging
       return {
+        $$willAnimate: true,
         end: endFn,
         start: function() {
           if (animationClosed) return;
@@ -790,6 +774,23 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
         }
       }
 
+      function closeAndReturnNoopAnimator() {
+        runner = new $$AnimateRunner({
+          end: endFn,
+          cancel: cancelFn
+        });
+
+        close();
+
+        return {
+          $$willAnimate: false,
+          start: function() {
+            return runner;
+          },
+          end: endFn
+        };
+      }
+
       function start() {
         if (animationClosed) return;
 
diff --git a/src/ngAnimate/animateCssDriver.js b/src/ngAnimate/animateCssDriver.js
@@ -130,11 +130,15 @@ var $$AnimateCssDriverProvider = ['$$animationProvider', function($$animationPro
       }
 
       function prepareOutAnimation() {
-        return $animateCss(clone, {
+        var animator = $animateCss(clone, {
           addClass: NG_OUT_ANCHOR_CLASS_NAME,
           delay: true,
           from: calculateAnchorStyles(outAnchor)
         });
+
+        // read the comment within `prepareRegularAnimation` to understand
+        // why this check is necessary
+        return animator.$$willAnimate ? animator : null;
       }
 
       function getClassVal(element) {
@@ -146,12 +150,16 @@ var $$AnimateCssDriverProvider = ['$$animationProvider', function($$animationPro
         var toAdd = getUniqueValues(endingClasses, startingClasses);
         var toRemove = getUniqueValues(startingClasses, endingClasses);
 
-        return $animateCss(clone, {
+        var animator = $animateCss(clone, {
           to: calculateAnchorStyles(inAnchor),
           addClass: NG_IN_ANCHOR_CLASS_NAME + ' ' + toAdd,
           removeClass: NG_OUT_ANCHOR_CLASS_NAME + ' ' + toRemove,
           delay: true
         });
+
+        // read the comment within `prepareRegularAnimation` to understand
+        // why this check is necessary
+        return animator.$$willAnimate ? animator : null;
       }
 
       function end() {
@@ -232,7 +240,13 @@ var $$AnimateCssDriverProvider = ['$$animationProvider', function($$animationPro
         options.onDone = animationDetails.domOperation;
       }
 
-      return $animateCss(element, options);
+      var animator = $animateCss(element, options);
+
+      // the driver lookup code inside of $$animation attempts to spawn a
+      // driver one by one until a driver returns a.$$willAnimate animator object.
+      // $animateCss will always return an object, however, it will pass in
+      // a flag as a hint as to whether an animation was detected or not
+      return animator.$$willAnimate ? animator : null;
     }
   }];
 }];
diff --git a/src/ngAnimate/module.js b/src/ngAnimate/module.js
@@ -363,17 +363,12 @@
  * myModule.animation('.slide', ['$animateCss', function($animateCss) {
  *   return {
  *     enter: function(element, doneFn) {
- *       var animation = $animateCss(element, {
- *         event: 'enter'
- *       });
- *
- *       if (animation) {
- *         // this will trigger `.slide.ng-enter` and `.slide.ng-enter-active`.
- *         var runner = animation.start();
- *         runner.done(doneFn);
- *       } else { //no CSS animation was detected
- *         doneFn();
- *       }
+*        // this will trigger `.slide.ng-enter` and `.slide.ng-enter-active`.
+ *       var runner = $animateCss(element, {
+ *         event: 'enter',
+ *         structural: true
+ *       }).start();
+*        runner.done(doneFn);
  *     }
  *   }
  * }]
@@ -389,18 +384,14 @@
  * myModule.animation('.slide', ['$animateCss', function($animateCss) {
  *   return {
  *     enter: function(element, doneFn) {
- *       var animation = $animateCss(element, {
+ *       var runner = $animateCss(element, {
  *         event: 'enter',
  *         addClass: 'maroon-setting',
  *         from: { height:0 },
  *         to: { height: 200 }
- *       });
+ *       }).start();
  *
- *       if (animation) {
- *         animation.start().done(doneFn);
- *       } else {
- *         doneFn();
- *       }
+ *       runner.done(doneFn);
  *     }
  *   }
  * }]
diff --git a/test/ngAnimate/animateCssDriverSpec.js b/test/ngAnimate/animateCssDriverSpec.js
@@ -58,6 +58,7 @@ describe("ngAnimate $$animateCssDriver", function() {
           });
 
           return {
+            $$willAnimate: true,
             start: function() {
               return runner;
             }
@@ -124,7 +125,9 @@ describe("ngAnimate $$animateCssDriver", function() {
 
       it("should not return anything if no animation is detected", function() {
         module(function($provide) {
-          $provide.value('$animateCss', noop);
+          $provide.value('$animateCss', function() {
+            return { $$willAnimate: false };
+          });
         });
         inject(function() {
           var runner = driver({
@@ -151,6 +154,7 @@ describe("ngAnimate $$animateCssDriver", function() {
           $provide.factory('$animateCss', function($q, $$AnimateRunner) {
             return function() {
               return {
+                $$willAnimate: true,
                 start: function() {
                   return new $$AnimateRunner({
                     end: function() {
@@ -190,6 +194,7 @@ describe("ngAnimate $$animateCssDriver", function() {
               var type = options.event || 'anchor';
               closeLog[type] = closeLog[type] || [];
               return {
+                $$willAnimate: true,
                 start: function() {
                   return new $$AnimateRunner({
                     end: function() {
@@ -252,6 +257,7 @@ describe("ngAnimate $$animateCssDriver", function() {
           $provide.factory('$animateCss', function($$AnimateRunner) {
             return function(element, details) {
               return {
+                $$willAnimate: true,
                 start: function() {
                   animationLog.push([element, details.event]);
                   return new $$AnimateRunner();
@@ -429,14 +435,13 @@ describe("ngAnimate $$animateCssDriver", function() {
           $provide.factory('$animateCss', function($$AnimateRunner) {
             return function(element, options) {
               var addClass = (options.addClass || '').trim();
-              if (addClass === expectedClass) {
-                return {
-                  start: function() {
-                    animationStarted = addClass;
-                    return runner = new $$AnimateRunner();
-                  }
-                };
-              }
+              return {
+                $$willAnimate: addClass === expectedClass,
+                start: function() {
+                  animationStarted = addClass;
+                  return runner = new $$AnimateRunner();
+                }
+              };
             };
           });
         });
diff --git a/test/ngAnimate/animateCssSpec.js b/test/ngAnimate/animateCssSpec.js
