From b08241620987af433260444e9625e107ee053f57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Sun, 5 Jan 2014 23:47:18 -0500 Subject: [PATCH] fix($animate): prevent race conditions for class-based animations when animating on the same CSS class Closes #5588 --- src/ngAnimate/animate.js | 19 +++++++++--- test/ngAnimate/animateSpec.js | 56 ++++++++++++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 26fe982d6b2a..f3a86f8246ed 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -659,12 +659,23 @@ angular.module('ngAnimate', ['ng']) cleanup(element); cancelAnimations(ngAnimateState.animations); + //in the event that the CSS is class is quickly added and removed back + //then we don't want to wait until after the reflow to add/remove the CSS + //class since both class animations may run into a race condition. + //The code below will check to see if that is occurring and will + //immediately remove the former class before the reflow so that the + //animation can snap back to the original animation smoothly + var isFullyClassBasedAnimation = isClassBased && !ngAnimateState.structural; + var isRevertingClassAnimation = isFullyClassBasedAnimation && + ngAnimateState.className == className && + animationEvent != ngAnimateState.event; + //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) { + if(ngAnimateState.beforeComplete || isRevertingClassAnimation) { (ngAnimateState.done || noop)(true); - } else if(isClassBased && !ngAnimateState.structural) { + } else if(isFullyClassBasedAnimation) { //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 @@ -812,10 +823,10 @@ angular.module('ngAnimate', ['ng']) function cancelAnimations(animations) { var isCancelledFlag = true; forEach(animations, function(animation) { - if(!animations.beforeComplete) { + if(!animation.beforeComplete) { (animation.beforeEnd || noop)(isCancelledFlag); } - if(!animations.afterComplete) { + if(!animation.afterComplete) { (animation.afterEnd || noop)(isCancelledFlag); } }); diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 99527cc4aa90..e74049be1049 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -2673,10 +2673,16 @@ describe("ngAnimate", function() { beforeAddClass : function(element, className, done) { currentAnimation = 'addClass'; currentFn = done; + return function(cancelled) { + currentAnimation = cancelled ? null : currentAnimation; + } }, beforeRemoveClass : function(element, className, done) { currentAnimation = 'removeClass'; currentFn = done; + return function(cancelled) { + currentAnimation = cancelled ? null : currentAnimation; + } } }; }); @@ -2690,10 +2696,12 @@ describe("ngAnimate", function() { expect(currentAnimation).toBe('addClass'); currentFn(); + currentAnimation = null; + $animate.removeClass(element, 'on'); $animate.addClass(element, 'on'); - expect(currentAnimation).toBe('addClass'); + expect(currentAnimation).toBe(null); }); }); @@ -3113,5 +3121,51 @@ describe("ngAnimate", function() { $timeout.flush(1); expect(ready).toBe(true); })); + + it('should avoid skip animations if the same CSS class is added / removed synchronously before the reflow kicks in', + inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) { + + if (!$sniffer.transitions) return; + + ss.addRule('.water-class', '-webkit-transition:2s linear all;' + + 'transition:2s linear all;'); + + $animate.enabled(true); + + var element = $compile('
')($rootScope); + $rootElement.append(element); + jqLite($document[0].body).append($rootElement); + + var signature = ''; + $animate.removeClass(element, 'on', function() { + signature += 'A'; + }); + $animate.addClass(element, 'on', function() { + signature += 'B'; + }); + + $timeout.flush(1); + expect(signature).toBe('AB'); + + signature = ''; + $animate.removeClass(element, 'on', function() { + signature += 'A'; + }); + $animate.addClass(element, 'on', function() { + signature += 'B'; + }); + $animate.removeClass(element, 'on', function() { + signature += 'C'; + }); + + $timeout.flush(1); + expect(signature).toBe('AB'); + + $timeout.flush(10); + browserTrigger(element, 'transitionend', { timeStamp: Date.now(), elapsedTime: 2000 }); + $timeout.flush(1); + + expect(signature).toBe('ABC'); + })); }); });