From 7d8d0a48b34740149f8f0c0ecce30eb19571af1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Sat, 16 Nov 2013 00:43:08 -0500 Subject: [PATCH] fix($animate): ensure addClass/removeClass animations do not snap during reflow Closes #4892 --- src/ngAnimate/animate.js | 31 +++++++++++++++++++++++++++---- test/ngAnimate/animateSpec.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 1a287e91df20..ab7c58164bd2 100644 --- a/src/ngAnimate/animate.js +++ b/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, diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 48ea00414f97..b9b7f9ed0eb8 100644 --- a/test/ngAnimate/animateSpec.js +++ b/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('
')($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) {