fix($animateCss): avoid flicker caused by temporary classes

Currently, this only affects `ngHide`/`ngShow` animations (but the problem was
more general and could theoretically affect other animations in the future).
Adding the temporary classes immediately, while forcing a reflow before starting
the animation, could cause flickering.

This commit fixes the issue, by not applying the temporary classes until the
animation actually starts. Since `tempClasses` was an undocumented, private
feature (currently only used for `ngHide`/`ngShow`), no other built-in or custom
animations will be affected.

Alternative approach to angular#15463.

Fixes angular#14015
Closes angular#15463

Author: gkalpak

src/ngAnimate/animateCss.js

@@ -835,6 +835,11 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro
           return;
         }
 
+        if (options.tempClasses) {
+          $$jqLite.addClass(element, options.tempClasses);
+          options.tempClasses = null;
+        }
+
         // even though we only pause keyframe animations here the pause flag
         // will still happen when transitions are used. Only the transition will
         // not be paused since that is not possible. If the animation ends when

src/ngAnimate/animateJs.js

@@ -90,6 +90,11 @@ var $$AnimateJsProvider = ['$animateProvider', /** @this */ function($animatePro
             return runner;
           }
 
+          if (options.tempClasses) {
+            $$jqLite.addClass(element, options.tempClasses);
+            options.tempClasses = null;
+          }
+
           runner = new $$AnimateRunner();
           var closeActiveAnimations;
           var chain = [];

src/ngAnimate/animation.js

@@ -134,7 +134,6 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro
       var tempClasses = options.tempClasses;
       if (tempClasses) {
         classes += ' ' + tempClasses;
-        options.tempClasses = null;
       }
 
       var prepareClassName;
@@ -185,9 +184,8 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro
           toBeSortedAnimations.push({
             domNode: getDomNode(animationEntry.from ? animationEntry.from.element : animationEntry.element),
             fn: 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.
+              // It is important that we apply the `ng-animate` CSS class before we do any driver
+              // invoking since these CSS classes may be required for proper CSS detection.
               animationEntry.beforeStart();
 
               var startAnimationFn, closeFn = animationEntry.close;
@@ -360,9 +358,6 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro
 
       function beforeStart() {
         element.addClass(NG_ANIMATE_CLASSNAME);
-        if (tempClasses) {
-          $$jqLite.addClass(element, tempClasses);
-        }
         if (prepareClassName) {
           $$jqLite.removeClass(element, prepareClassName);
           prepareClassName = null;

test/ngAnimate/animateCssSpec.js

@@ -1641,6 +1641,26 @@ describe('ngAnimate $animateCss', function() {
       expect(element).toHaveClass('super-active');
     }));
 
+    it('should apply `tempClasses` when starting the animation',
+      inject(function($animateCss, $document, $rootElement) {
+
+      var element = angular.element('<div></div>');
+      $rootElement.append(element);
+      $document.find('body').append($rootElement);
+
+      $animateCss(element, {
+        event: 'super',
+        duration: 1000,
+        to: fakeStyle,
+        tempClasses: 'foo'
+      }).start();
+
+      expect(element).not.toHaveClass('foo');
+
+      triggerAnimationStartFrame();
+      expect(element).toHaveClass('foo');
+    }));
+
     describe('structural animations', function() {
       they('should decorate the element with the ng-$prop CSS class',
         ['enter', 'leave', 'move'], function(event) {

test/ngAnimate/animateJsSpec.js

@@ -193,7 +193,7 @@ describe('ngAnimate $$animateJs', function() {
     });
   });
 
-  it('should always run the provided animation in atleast one RAF frame if defined', function() {
+  it('should always run the provided animation in at least one RAF frame if defined', function() {
     var before, after, endCalled;
     module(function($animateProvider) {
       $animateProvider.register('.the-end', function() {
@@ -325,6 +325,23 @@ describe('ngAnimate $$animateJs', function() {
     });
   });
 
+  it('should apply `tempClasses` when starting the animation', function() {
+    module(function($animateProvider) {
+      $animateProvider.register('.foo-element', function() {
+        return {foo: noop};
+      });
+    });
+    inject(function($$animateJs, $animate, $rootScope) {
+      var element = jqLite('<div class="foo-element"></div>');
+      var animator = $$animateJs(element, 'foo', 'foo-element', {tempClasses: 'temp'});
+
+      expect(element).not.toHaveClass('temp');
+
+      animator.start();
+      expect(element).toHaveClass('temp');
+    });
+  });
+
   describe('events', function() {
     var animations, runAnimation, element, log;
     beforeEach(module(function($animateProvider) {

test/ngAnimate/animationSpec.js

@@ -859,13 +859,14 @@ describe('$$animation', function() {
       element = jqLite('<div></div>');
       parent = jqLite('<div></div>');
 
-      return function($$AnimateRunner, $rootElement, $document) {
+      return function($$AnimateRunner, $$jqLite, $rootElement, $document) {
         jqLite($document[0].body).append($rootElement);
         $rootElement.append(parent);
 
-        mockedDriverFn = function(element, method, options, domOperation) {
+        mockedDriverFn = function(animationEntry) {
           return {
             start: function() {
+              $$jqLite.addClass(animationEntry.element, animationEntry.options.tempClasses);
               runner = new $$AnimateRunner();
               return runner;
             }
@@ -874,7 +875,7 @@ describe('$$animation', function() {
       };
     }));
 
-    it('should temporarily assign the provided CSS class for the duration of the animation',
+    it('should temporarily assign the provided CSS classes for the duration of the animation',
       inject(function($rootScope, $$animation) {
 
       parent.append(element);
@@ -894,6 +895,26 @@ describe('$$animation', function() {
       expect(element).not.toHaveClass('fudge');
     }));
 
+    it('should remove the temporary CSS classes if the animation gets cancelled',
+      inject(function($rootScope, $$animation) {
+
+      parent.append(element);
+
+      $$animation(element, 'enter', {
+        tempClasses: 'temporary fudge'
+      });
+      $rootScope.$digest();
+
+      expect(element).toHaveClass('temporary');
+      expect(element).toHaveClass('fudge');
+
+      runner.cancel();
+      $rootScope.$digest();
+
+      expect(element).not.toHaveClass('temporary');
+      expect(element).not.toHaveClass('fudge');
+    }));
+
     it('should add and remove the ng-animate CSS class when the animation is active',
       inject(function($$animation, $rootScope) {
 
@@ -910,11 +931,9 @@ describe('$$animation', function() {
     }));
 
 
-    it('should apply the `ng-animate` and temporary CSS classes before the driver is invoked', function() {
+    it('should apply the `ng-animate` CSS class before the driver is invoked', function() {
       var capturedElementClasses;
 
-      parent.append(element);
-
       module(function($provide) {
         $provide.factory('mockedTestDriver', function() {
           return function(details) {
@@ -932,7 +951,29 @@ describe('$$animation', function() {
         $rootScope.$digest();
 
         expect(capturedElementClasses).toMatch(/\bng-animate\b/);
-        expect(capturedElementClasses).toMatch(/\btemp-class-name\b/);
+      });
+    });
+
+    it('should not apply `tempClasses` before the driver is invoked', function() {
+      var capturedElementClasses;
+
+      module(function($provide) {
+        $provide.factory('mockedTestDriver', function($$jqLite) {
+          return function(details) {
+            capturedElementClasses = details.element.attr('class');
+          };
+        });
+      });
+
+      inject(function($$animation, $animate, $rootScope) {
+        parent.append(element);
+
+        $$animation(element, 'enter', {
+          tempClasses: 'temp-class-name'
+        });
+        $rootScope.$digest();
+
+        expect(capturedElementClasses).not.toMatch(/\btemp-class-name\b/);
       });
     });
 

test/ngAnimate/integrationSpec.js

@@ -544,6 +544,71 @@ describe('ngAnimate integration tests', function() {
         expect(child).not.toHaveClass('blue');
       });
     });
+
+    it('should apply a temporary class only for the actual duration of the animation', function() {
+      var elementClassList = [];
+      var hasTempClass = {
+        beforeCssAnimationStart: null,
+        afterCssAnimationStart: null,
+        afterCssAnimationTriggered: null,
+        afterCssAnimationFinished: null
+      };
+
+      module(function($provide) {
+        $provide.decorator('$animateCss', function($delegate) {
+          var decorated = function(element) {
+            var animator = $delegate.apply(null, arguments);
+            var startFn = animator.start;
+
+            animator.start = function() {
+              hasTempClass.beforeCssAnimationStart = element.hasClass('temp-class');
+              var runner = startFn.apply(animator, arguments);
+              hasTempClass.afterCssAnimationStart = element.hasClass('temp-class');
+              return runner;
+            };
+
+            return animator;
+          };
+
+          return decorated;
+        });
+      });
+
+      inject(function($animate, $compile, $rootScope, $rootElement, $$rAF) {
+        element = jqLite('<div class="animate-me"></div>');
+        $compile(element)($rootScope);
+
+        var className = 'klass';
+        var parent = jqLite('<div></div>');
+        html(parent);
+
+        parent.append(element);
+
+        ss.addRule('.animate-me', 'transition: all 2s;');
+
+        var runner = $animate.addClass(element, className, {
+          tempClasses: 'temp-class'
+        });
+
+        $rootScope.$digest();
+        $animate.flush();
+
+        hasTempClass.afterCssAnimationTriggered = element.hasClass('temp-class');
+
+        browserTrigger(element, 'transitionend', {timeStamp: Date.now(), elapsedTime: 2});
+        $rootScope.$digest();
+        $animate.flush();
+
+        hasTempClass.afterCssAnimationFinished = element.hasClass('temp-class');
+
+        expect(hasTempClass).toEqual({
+          beforeCssAnimationStart: false,
+          afterCssAnimationStart: false,
+          afterCssAnimationTriggered: true,
+          afterCssAnimationFinished: false
+        });
+      });
+    });
   });
 
   describe('JS animations', function() {