From 3bb79d23d97496de0285407561b12274a7de6e87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Thu, 17 Oct 2013 22:37:17 -0400 Subject: [PATCH] fix($animate): only cancel class-based animations if the follow-up class contains CSS transition/keyframe animation code Closes #4463 Closes #3784 --- src/ngAnimate/animate.js | 86 +++++++++++++++++++++++++++-------- test/ngAnimate/animateSpec.js | 40 +++++++++++++++- 2 files changed, 104 insertions(+), 22 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 20b9101e0585..f1945d02d224 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -493,35 +493,47 @@ angular.module('ngAnimate', ['ng']) */ function performAnimation(event, className, element, parent, after, onComplete) { var classes = (element.attr('class') || '') + ' ' + className; - var animationLookup = (' ' + classes).replace(/\s+/g,'.'), - animations = []; - forEach(lookup(animationLookup), function(animation, index) { - animations.push({ - start : animation[event] - }); - }); - + var animationLookup = (' ' + classes).replace(/\s+/g,'.'); if (!parent) { parent = after ? after.parent() : element.parent(); } + var disabledAnimation = { running : true }; + var matches = lookup(animationLookup); + var isClassBased = event == 'addClass' || event == 'removeClass'; + var ngAnimateState = element.data(NG_ANIMATE_STATE) || {}; - //skip the animation if animations are disabled, a parent is already being animated - //or the element is not currently attached to the document body. - if ((parent.inheritedData(NG_ANIMATE_STATE) || disabledAnimation).running || animations.length === 0) { + //skip the animation if animations are disabled, a parent is already being animated, + //the element is not currently attached to the document body or then completely close + //the animation if any matching animations are not found at all. + //NOTE: IE8 + IE9 should close properly (run done()) in case a NO animation is not found. + if ((parent.inheritedData(NG_ANIMATE_STATE) || disabledAnimation).running || matches.length == 0) { done(); return; } - var ngAnimateState = element.data(NG_ANIMATE_STATE) || {}; + var animations = []; + //only add animations if the currently running animation is not structural + //or if there is no animation running at all + if(!ngAnimateState.running || !(isClassBased && ngAnimateState.structural)) { + forEach(matches, function(animation) { + //add the animation to the queue to if it is allowed to be cancelled + if(!animation.allowCancel || animation.allowCancel(element, event, className)) { + animations.push({ + start : animation[event] + }); + } + }); + } - var isClassBased = event == 'addClass' || event == 'removeClass'; - if(ngAnimateState.running) { - if(isClassBased && ngAnimateState.structural) { - onComplete && onComplete(); - return; - } + //this would mean that an animation was not allowed so let the existing + //animation do it's thing and close this one early + if(animations.length == 0) { + onComplete && onComplete(); + return; + } + 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.flagTimer); @@ -651,6 +663,7 @@ angular.module('ngAnimate', ['ng']) animationIterationCountKey = 'IterationCount'; var NG_ANIMATE_PARENT_KEY = '$ngAnimateKey'; + var NG_ANIMATE_CLASS_KEY = '$$ngAnimateClasses'; var lookupCache = {}; var parentCounter = 0; @@ -669,7 +682,7 @@ angular.module('ngAnimate', ['ng']) } function getElementAnimationDetails(element, cacheKey, onlyCheckTransition) { - var data = lookupCache[cacheKey]; + var data = cacheKey ? lookupCache[cacheKey] : null; if(!data) { var transitionDuration = 0, transitionDelay = 0, animationDuration = 0, animationDelay = 0; @@ -702,7 +715,9 @@ angular.module('ngAnimate', ['ng']) transitionDuration : transitionDuration, animationDuration : animationDuration }; - lookupCache[cacheKey] = data; + if(cacheKey) { + lookupCache[cacheKey] = data; + } } return data; } @@ -769,6 +784,7 @@ angular.module('ngAnimate', ['ng']) element.addClass(activeClassName); }); + element.data(NG_ANIMATE_CLASS_KEY, className + ' ' + activeClassName); element.on(css3AnimationEvents, onAnimationProgress); // This will automatically be called by $animate so @@ -778,6 +794,7 @@ angular.module('ngAnimate', ['ng']) element.off(css3AnimationEvents, onAnimationProgress); element.removeClass(className); element.removeClass(activeClassName); + element.removeData(NG_ANIMATE_CLASS_KEY); // Only when the animation is cancelled is the done() // function not called for this animation therefore @@ -811,6 +828,35 @@ angular.module('ngAnimate', ['ng']) } return { + allowCancel : function(element, event, className) { + //always cancel the current animation if it is a + //structural animation + var oldClasses = element.data(NG_ANIMATE_CLASS_KEY); + if(!oldClasses || ['enter','leave','move'].indexOf(event) >= 0) { + return true; + } + + var parent = element.parent(); + var clone = angular.element(element[0].cloneNode()); + + //make the element super hidden and override any CSS style values + clone.attr('style','position:absolute; top:-9999px; left:-9999px'); + clone.removeAttr('id'); + clone.html(''); + + angular.forEach(oldClasses.split(' '), function(klass) { + clone.removeClass(klass); + }); + + var suffix = event == 'addClass' ? '-add' : '-remove'; + clone.addClass(suffixClasses(className, suffix)); + parent.append(clone); + + var timings = getElementAnimationDetails(clone); + clone.remove(); + + return Math.max(timings.transitionDuration, timings.animationDuration) > 0; + }, enter : function(element, done) { return animate(element, 'ng-enter', done); }, diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index cae252665a0e..6dfd46e5c0dd 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -746,8 +746,8 @@ describe("ngAnimate", function() { expect(element.hasClass('ng-enter')).toBe(true); expect(element.hasClass('ng-enter-active')).toBe(true); browserTrigger(element,'transitionend', { timeStamp: Date.now() + 22000, elapsedTime: 22000 }); + $timeout.flush(); } - $timeout.flush(); expect(element.hasClass('abc')).toBe(true); $rootScope.klass = 'xyz'; @@ -760,8 +760,8 @@ describe("ngAnimate", function() { expect(element.hasClass('ng-enter')).toBe(true); expect(element.hasClass('ng-enter-active')).toBe(true); browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000, elapsedTime: 11000 }); + $timeout.flush(); } - $timeout.flush(); expect(element.hasClass('xyz')).toBe(true); })); @@ -1920,4 +1920,40 @@ describe("ngAnimate", function() { expect(count).toBe(40); }); }); + + it("should cancel an ongoing class-based animation only if the new class contains transition/animation CSS code", + inject(function($compile, $rootScope, $animate, $sniffer) { + + if (!$sniffer.transitions) return; + + ss.addRule('.green-add', '-webkit-transition:1s linear all;' + + 'transition:1s linear all;'); + + ss.addRule('.blue-add', 'background:blue;'); + + ss.addRule('.red-add', '-webkit-transition:1s linear all;' + + 'transition:1s linear all;'); + + ss.addRule('.yellow-add', '-webkit-animation: some_animation 4s linear 1s 2 alternate;' + + 'animation: some_animation 4s linear 1s 2 alternate;'); + + 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, 'blue'); + expect(element.hasClass('blue')).toBe(true); + expect(element.hasClass('green-add')).toBe(true); //not cancelled + + $animate.addClass(element, 'red'); + expect(element.hasClass('green-add')).toBe(false); + expect(element.hasClass('red-add')).toBe(true); + + $animate.addClass(element, 'yellow'); + expect(element.hasClass('red-add')).toBe(false); + expect(element.hasClass('yellow-add')).toBe(true); + })); });