fix($animate): ensure addClass/removeClass animations do not snap during reflow

Closes angular#4892

Author: matsko

src/ngAnimate/animate.js

@@ -549,7 +549,8 @@ angular.module('ngAnimate', ['ng'])
         and the onComplete callback will be fired once the animation is fully complete.
       */
       function performAnimation(animationEvent, className, element, parentElement, afterElement, domOperation, doneCallback) {
-        var classes = (element.attr('class') || '') + ' ' + className;
+        var currentClassName = element.attr('class') || '';
+        var classes = currentClassName + ' ' + className;
         var animationLookup = (' ' + classes).replace(/\s+/g,'.');
         if (!parentElement) {
           parentElement = afterElement ? afterElement.parent() : element.parent();
@@ -602,21 +603,42 @@ angular.module('ngAnimate', ['ng'])
           return;
         }
 
+        //this value will be searched for class-based CSS className lookup. Therefore,
+        //we prefix and suffix the current className value with spaces to avoid substring
+        //lookups of className tokens
+        var futureClassName = ' ' + currentClassName + ' ';
         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.closeAnimationTimeout);
           cleanup(element);
           cancelAnimations(ngAnimateState.animations);
-          (ngAnimateState.done || noop)(true);
+
+          //if the class is removed during the reflow then it will revert the styles temporarily
+          //back to the base class CSS styling causing a jump-like effect to occur. This check
+          //here ensures that the domOperation is only performed after the reflow has commenced
+          if(ngAnimateState.beforeComplete) {
+            (ngAnimateState.done || noop)(true);
+          } else if(isClassBased && !ngAnimateState.structural) {
+            //class-based animations will compare element className values after cancelling the
+            //previous animation to see if the element properties already contain the final CSS
+            //class and if so then the animation will be skipped. Since the domOperation will
+            //be performed only after the reflow is complete then our element's className value
+            //will be invalid. Therefore the same string manipulation that would occur within the
+            //DOM operation will be performed below so that the class comparison is valid...
+            futureClassName = ngAnimateState.event == 'removeClass' ?
+              futureClassName.replace(ngAnimateState.className, '') :
+              futureClassName + ngAnimateState.className + ' ';
+          }
         }
 
         //There is no point in perform a class-based animation if the element already contains
         //(on addClass) or doesn't contain (on removeClass) the className being animated.
         //The reason why this is being called after the previous animations are cancelled
         //is so that the CSS classes present on the element can be properly examined.
-        if((animationEvent == 'addClass'    && element.hasClass(className)) ||
-           (animationEvent == 'removeClass' && !element.hasClass(className))) {
+        var classNameToken = ' ' + className + ' ';
+        if((animationEvent == 'addClass'    && futureClassName.indexOf(classNameToken) >= 0) ||
+           (animationEvent == 'removeClass' && futureClassName.indexOf(classNameToken) == -1)) {
           fireDOMOperation();
           fireDoneCallbackAsync();
           return;
@@ -628,6 +650,7 @@ angular.module('ngAnimate', ['ng'])
 
         element.data(NG_ANIMATE_STATE, {
           running:true,
+          event:animationEvent,
           className:className,
           structural:!isClassBased,
           animations:animations,

test/ngAnimate/animateSpec.js

@@ -2515,6 +2515,36 @@ describe("ngAnimate", function() {
     expect(element.hasClass('yellow-add')).toBe(true);
   }));
 
+  it("should cancel and perform the dom operation only after the reflow has run",
+    inject(function($compile, $rootScope, $animate, $sniffer, $timeout) {
+
+    if (!$sniffer.transitions) return;
+
+    ss.addRule('.green-add', '-webkit-transition:1s linear all;' +
+                                     'transition:1s linear all;');
+
+    ss.addRule('.red-add', '-webkit-transition:1s linear all;' +
+                                   'transition:1s linear all;');
+
+    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, 'red');
+    expect(element.hasClass('red-add')).toBe(true);
+
+    expect(element.hasClass('green')).toBe(false);
+    expect(element.hasClass('red')).toBe(false);
+
+    $timeout.flush();
+
+    expect(element.hasClass('green')).toBe(true);
+    expect(element.hasClass('red')).toBe(true);
+  }));
+
   it('should enable and disable animations properly on the root element', function() {
     var count = 0;
     module(function($animateProvider) {