From d1c983e033519d7efd111ef25646d01155fa0ab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Thu, 3 Oct 2013 10:52:38 -0400 Subject: [PATCH 1/7] refactor($animate): avoid checking for transition/animation support during each animation --- src/ngAnimate/animate.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 7fcdef73f856..5aed6832e619 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -215,8 +215,11 @@ angular.module('ngAnimate', ['ng']) //the empty string value is the default animation //operation which performs CSS transition and keyframe //animations sniffing. This is always included for each - //element animation procedure - classes.push(''); + //element animation procedure if the browser supports + //transitions and/or keyframe animations + if ($sniffer.transitions || $sniffer.animations) { + classes.push(''); + } for(var i=0; i < classes.length; i++) { var klass = classes[i], @@ -477,7 +480,7 @@ angular.module('ngAnimate', ['ng']) //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) { + if ((parent.inheritedData(NG_ANIMATE_STATE) || disabledAnimation).running || animations.length == 0) { //avoid calling done() since there is no need to remove any //data or className values since this happens earlier than that //and also use a timeout so that it won't be asynchronous @@ -579,11 +582,7 @@ angular.module('ngAnimate', ['ng']) ELEMENT_NODE = 1; function animate(element, className, done) { - if (!($sniffer.transitions || $sniffer.animations)) { - done(); - return; - } - else if(['ng-enter','ng-leave','ng-move'].indexOf(className) == -1) { + if(['ng-enter','ng-leave','ng-move'].indexOf(className) == -1) { var existingDuration = 0; forEach(element, function(element) { if (element.nodeType == ELEMENT_NODE) { From 055fd4cc3293850017d88b35a76fcb439496abc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Thu, 3 Oct 2013 15:06:42 -0400 Subject: [PATCH 2/7] fix($animate): ensure elapsedTime always considers delay values --- src/ngAnimate/animate.js | 46 +++++++++++-------- test/ngAnimate/animateSpec.js | 84 +++++++++++++++++------------------ 2 files changed, 69 insertions(+), 61 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 5aed6832e619..ee637b3ba4a1 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -600,40 +600,43 @@ angular.module('ngAnimate', ['ng']) element.addClass(className); //we want all the styles defined before and after - var transitionTime = 0, - animationTime = 0; + var transitionDuration = 0, + animationDuration = 0, + transitionDelay = 0, + animationDelay = 0; forEach(element, function(element) { if (element.nodeType == ELEMENT_NODE) { var elementStyles = $window.getComputedStyle(element) || {}; - var transitionDelay = parseMaxTime(elementStyles[transitionProp + delayKey]); + transitionDelay = Math.max(parseMaxTime(elementStyles[transitionProp + delayKey]), transitionDelay); - var animationDelay = parseMaxTime(elementStyles[animationProp + delayKey]); + animationDelay = Math.max(parseMaxTime(elementStyles[animationProp + delayKey]), animationDelay); - var transitionDuration = parseMaxTime(elementStyles[transitionProp + durationKey]); + transitionDuration = Math.max(parseMaxTime(elementStyles[transitionProp + durationKey]), transitionDuration); - var animationDuration = parseMaxTime(elementStyles[animationProp + durationKey]); + var aDuration = parseMaxTime(elementStyles[animationProp + durationKey]); - if(animationDuration > 0) { - animationDuration *= parseInt(elementStyles[animationProp + animationIterationCountKey]) || 1; + if(aDuration > 0) { + aDuration *= parseInt(elementStyles[animationProp + animationIterationCountKey]) || 1; } - transitionTime = Math.max(transitionDelay + transitionDuration, transitionTime); - animationTime = Math.max(animationDelay + animationDuration, animationTime); + animationDuration = Math.max(aDuration, animationDuration); } }); /* there is no point in performing a reflow if the animation timeout is empty (this would cause a flicker bug normally - in the page */ - var maxTime = Math.max(transitionTime,animationTime) * 1000; - if(maxTime > 0) { - var node = element[0], - startTime = Date.now(); + in the page. There is also no point in performing an animation + that only has a delay and no duration */ + var maxDuration = Math.max(transitionDuration, animationDuration); + if(maxDuration > 0) { + var maxDelayTime = Math.max(transitionDelay, animationDelay) * 1000, + startTime = Date.now(), + node = element[0]; //temporarily disable the transition so that the enter styles //don't animate twice (this is here to avoid a bug in Chrome/FF). - if(transitionTime > 0) { + if(transitionDuration > 0) { node.style[transitionProp + propertyKey] = 'none'; } @@ -644,7 +647,7 @@ angular.module('ngAnimate', ['ng']) // This triggers a reflow which allows for the transition animation to kick in. element.prop('clientWidth'); - if(transitionTime > 0) { + if(transitionDuration > 0) { node.style[transitionProp + propertyKey] = ''; } element.addClass(activeClassName); @@ -678,8 +681,13 @@ angular.module('ngAnimate', ['ng']) var ev = event.originalEvent || event; /* $manualTimeStamp is a mocked timeStamp value which is set * within browserTrigger(). This is only here so that tests can - * mock animations properly. Real events fallback to event.timeStamp. */ - if((ev.$manualTimeStamp || ev.timeStamp) - startTime >= maxTime) { + * mock animations properly. Real events fallback to event.timeStamp. + * We're checking to see if the timestamp surpasses the expected delay, + * but we're using elapsedTime instead of the timestamp on the 2nd + * pre-condition since animations sometimes close off early */ + if((ev.$manualTimeStamp || ev.timeStamp) - startTime >= maxDelayTime && + ev.elapsedTime >= maxDuration) { + done(); } } diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index f5faa0241680..90868ef6abc4 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -137,7 +137,7 @@ describe("ngAnimate", function() { if($sniffer.transitions) { expect(child.hasClass('ng-enter')).toBe(true); expect(child.hasClass('ng-enter-active')).toBe(true); - browserTrigger(element, 'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(element, 'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } expect(element.contents().length).toBe(1); @@ -153,7 +153,7 @@ describe("ngAnimate", function() { if($sniffer.transitions) { expect(child.hasClass('ng-leave')).toBe(true); expect(child.hasClass('ng-leave-active')).toBe(true); - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } expect(element.contents().length).toBe(0); @@ -185,7 +185,7 @@ describe("ngAnimate", function() { if($sniffer.transitions) { expect(child.hasClass('ng-hide-remove')).toBe(true); expect(child.hasClass('ng-hide-remove-active')).toBe(true); - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } expect(child.hasClass('ng-hide-remove')).toBe(false); expect(child.hasClass('ng-hide-remove-active')).toBe(false); @@ -201,7 +201,7 @@ describe("ngAnimate", function() { if($sniffer.transitions) { expect(child.hasClass('ng-hide-add')).toBe(true); expect(child.hasClass('ng-hide-add-active')).toBe(true); - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } expect(child).toBeHidden(); })); @@ -220,7 +220,7 @@ describe("ngAnimate", function() { expect(child.attr('class')).toContain('ng-enter'); expect(child.attr('class')).toContain('ng-enter-active'); - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); //move element.append(after); @@ -229,26 +229,26 @@ describe("ngAnimate", function() { expect(child.attr('class')).toContain('ng-move'); expect(child.attr('class')).toContain('ng-move-active'); - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); //hide $animate.addClass(child, 'ng-hide'); expect(child.attr('class')).toContain('ng-hide-add'); expect(child.attr('class')).toContain('ng-hide-add-active'); - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); //show $animate.removeClass(child, 'ng-hide'); expect(child.attr('class')).toContain('ng-hide-remove'); expect(child.attr('class')).toContain('ng-hide-remove-active'); - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); //leave $animate.leave(child); $rootScope.$digest(); expect(child.attr('class')).toContain('ng-leave'); expect(child.attr('class')).toContain('ng-leave-active'); - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); })); it("should not run if animations are disabled", @@ -291,7 +291,7 @@ describe("ngAnimate", function() { if($sniffer.transitions) { expect(element.children().length).toBe(1); //still animating - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } $timeout.flush(2000); $timeout.flush(2000); @@ -308,7 +308,7 @@ describe("ngAnimate", function() { child.addClass('custom-delay ng-hide'); $animate.removeClass(child, 'ng-hide'); if($sniffer.transitions) { - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } $timeout.flush(2000); @@ -372,7 +372,7 @@ describe("ngAnimate", function() { $animate.removeClass(element, 'ng-hide'); if($sniffer.transitions) { - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } $timeout.flush(2000); $timeout.flush(20000); @@ -414,7 +414,7 @@ describe("ngAnimate", function() { $animate.removeClass(element, 'ng-hide'); if ($sniffer.animations) { - browserTrigger(element,'animationend', { timeStamp: Date.now() + 4000 }); + browserTrigger(element,'animationend', { timeStamp: Date.now() + 4000, elapsedTime: 4000 }); } expect(element).toBeShown(); })); @@ -437,7 +437,7 @@ describe("ngAnimate", function() { $animate.removeClass(element, 'ng-hide'); if ($sniffer.animations) { - browserTrigger(element,'animationend', { timeStamp: Date.now() + 6000 }); + browserTrigger(element,'animationend', { timeStamp: Date.now() + 6000, elapsedTime: 6000 }); } expect(element).toBeShown(); })); @@ -460,7 +460,7 @@ describe("ngAnimate", function() { $animate.removeClass(element, 'ng-hide'); if ($sniffer.animations) { - browserTrigger(element,'animationend', { timeStamp: Date.now() + 2000 }); + browserTrigger(element,'animationend', { timeStamp: Date.now() + 2000, elapsedTime: 2000 }); } expect(element).toBeShown(); })); @@ -485,7 +485,7 @@ describe("ngAnimate", function() { $animate.removeClass(element, 'ng-hide'); if ($sniffer.transitions) { - browserTrigger(element,'animationend', { timeStamp : Date.now() + 20000 }); + browserTrigger(element,'animationend', { timeStamp : Date.now() + 20000, elapsedTime: 20000 }); } expect(element).toBeShown(); })); @@ -530,7 +530,7 @@ describe("ngAnimate", function() { if($sniffer.animations) { //cleanup some pending animations expect(element.hasClass('ng-hide-add')).toBe(true); expect(element.hasClass('ng-hide-add-active')).toBe(true); - browserTrigger(element,'animationend', { timeStamp: Date.now() + 2000 }); + browserTrigger(element,'animationend', { timeStamp: Date.now() + 2000, elapsedTime: 2000 }); } expect(element.hasClass('ng-hide-remove-active')).toBe(false); @@ -562,7 +562,7 @@ describe("ngAnimate", function() { $animate.removeClass(element, 'ng-hide'); if ($sniffer.transitions) { - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } expect(element).toBeShown(); })); @@ -584,9 +584,9 @@ describe("ngAnimate", function() { if ($sniffer.transitions) { expect(element).toBeHidden(); var now = Date.now(); - browserTrigger(element,'transitionend', { timeStamp: now + 1000 }); - browserTrigger(element,'transitionend', { timeStamp: now + 1000 }); - browserTrigger(element,'transitionend', { timeStamp: now + 2000 }); + browserTrigger(element,'transitionend', { timeStamp: now + 1000, elapsedTime: 1000 }); + browserTrigger(element,'transitionend', { timeStamp: now + 1000, elapsedTime: 1000 }); + browserTrigger(element,'transitionend', { timeStamp: now + 2000, elapsedTime: 2000 }); } expect(element).toBeShown(); })); @@ -618,9 +618,9 @@ describe("ngAnimate", function() { $animate.removeClass(element, 'ng-hide'); if ($sniffer.transitions) { var now = Date.now(); - browserTrigger(element,'transitionend', { timeStamp: now + 1000 }); - browserTrigger(element,'transitionend', { timeStamp: now + 3000 }); - browserTrigger(element,'transitionend', { timeStamp: now + 3000 }); + browserTrigger(element,'transitionend', { timeStamp: now + 1000, elapsedTime: 1000 }); + browserTrigger(element,'transitionend', { timeStamp: now + 3000, elapsedTime: 3000 }); + browserTrigger(element,'transitionend', { timeStamp: now + 3000, elapsedTime: 3000 }); } expect(element).toBeShown(); })); @@ -642,7 +642,7 @@ describe("ngAnimate", function() { $animate.removeClass(element, 'ng-hide'); if ($sniffer.transitions) { - browserTrigger(element,'animationend', { timeStamp: Date.now() + 11000 }); + browserTrigger(element,'animationend', { timeStamp: Date.now() + 11000, elapsedTime: 11000 }); } expect(element).toBeShown(); })); @@ -662,7 +662,7 @@ describe("ngAnimate", function() { if($sniffer.transitions) { expect(element.hasClass('ng-hide-remove')).toBe(true); expect(element.hasClass('ng-hide-remove-active')).toBe(true); - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } expect(element.hasClass('ng-hide-remove')).toBe(false); expect(element.hasClass('ng-hide-remove-active')).toBe(false); @@ -698,7 +698,7 @@ describe("ngAnimate", function() { if ($sniffer.transitions) { expect(element.hasClass('abc ng-enter')).toBe(true); expect(element.hasClass('abc ng-enter ng-enter-active')).toBe(true); - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 22000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 22000, elapsedTime: 22000 }); } expect(element.hasClass('abc')).toBe(true); @@ -709,7 +709,7 @@ describe("ngAnimate", function() { if ($sniffer.transitions) { expect(element.hasClass('xyz')).toBe(true); expect(element.hasClass('xyz ng-enter ng-enter-active')).toBe(true); - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000, elapsedTime: 11000 }); } expect(element.hasClass('xyz')).toBe(true); })); @@ -736,7 +736,7 @@ describe("ngAnimate", function() { expect(element.hasClass('one two ng-enter ng-enter-active')).toBe(true); expect(element.hasClass('one-active')).toBe(false); expect(element.hasClass('two-active')).toBe(false); - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 3000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 3000, elapsedTime: 3000 }); } expect(element.hasClass('one two')).toBe(true); @@ -880,7 +880,7 @@ describe("ngAnimate", function() { }); if($sniffer.transitions) { - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } $timeout.flush(); expect(flag).toBe(true); @@ -1013,7 +1013,7 @@ describe("ngAnimate", function() { expect(element.hasClass('klass')).toBe(false); expect(element.hasClass('klass-add')).toBe(true); expect(element.hasClass('klass-add-active')).toBe(true); - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 3000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 3000, elapsedTime: 3000 }); } //this cancels out the older animation @@ -1027,7 +1027,7 @@ describe("ngAnimate", function() { expect(element.hasClass('klass-add-active')).toBe(false); expect(element.hasClass('klass-remove')).toBe(true); - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 3000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 3000, elapsedTime: 3000 }); } $timeout.flush(); @@ -1085,7 +1085,7 @@ describe("ngAnimate", function() { if($sniffer.transitions) { expect(element.hasClass('klass-add')).toBe(true); expect(element.hasClass('klass-add-active')).toBe(true); - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000, elapsedTime: 11000 }); expect(element.hasClass('klass-add')).toBe(false); expect(element.hasClass('klass-add-active')).toBe(false); } @@ -1099,7 +1099,7 @@ describe("ngAnimate", function() { if($sniffer.transitions) { expect(element.hasClass('klass-remove')).toBe(true); expect(element.hasClass('klass-remove-active')).toBe(true); - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000, elapsedTime: 11000 }); expect(element.hasClass('klass-remove')).toBe(false); expect(element.hasClass('klass-remove-active')).toBe(false); } @@ -1134,7 +1134,7 @@ describe("ngAnimate", function() { expect(element.hasClass('one-add-active')).toBe(true); expect(element.hasClass('two-add-active')).toBe(true); - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 7000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 7000, elapsedTime: 7000 }); expect(element.hasClass('one-add')).toBe(false); expect(element.hasClass('one-add-active')).toBe(false); @@ -1178,7 +1178,7 @@ describe("ngAnimate", function() { expect(element.hasClass('one-remove-active')).toBe(true); expect(element.hasClass('two-remove-active')).toBe(true); - browserTrigger(element,'transitionend', { timeStamp: Date.now() + 9000 }); + browserTrigger(element,'transitionend', { timeStamp: Date.now() + 9000, elapsedTime: 9000 }); expect(element.hasClass('one-remove')).toBe(false); expect(element.hasClass('one-remove-active')).toBe(false); @@ -1227,7 +1227,7 @@ describe("ngAnimate", function() { if($sniffer.transitions) { expect(child.hasClass('ng-enter')).toBe(true); expect(child.hasClass('ng-enter-active')).toBe(true); - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } expect(child.hasClass('ng-enter')).toBe(false); @@ -1249,7 +1249,7 @@ describe("ngAnimate", function() { if($sniffer.transitions) { expect(child.hasClass('ng-enter')).toBe(true); expect(child.hasClass('ng-enter-active')).toBe(true); - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 9000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 9000, elapsedTime: 9000 }); } expect(child.hasClass('ng-enter')).toBe(false); expect(child.hasClass('ng-enter-active')).toBe(false); @@ -1275,7 +1275,7 @@ describe("ngAnimate", function() { $animate.enter(child, element); $rootScope.$digest(); - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 2000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 2000, elapsedTime: 2000 }); expect(child.css(propertyKey)).toBe('background-color'); child.remove(); @@ -1333,7 +1333,7 @@ describe("ngAnimate", function() { $timeout.flush(10); if($sniffer.transitions) { - browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); } expect(child.hasClass('i-was-animated')).toBe(true); @@ -1561,12 +1561,12 @@ describe("ngAnimate", function() { expect(child.hasClass('ng-enter')).toBe(true); expect(child.hasClass('ng-enter-active')).toBe(true); - browserTrigger(child, 'transitionend', { timeStamp: Date.now() + 1000 }); + browserTrigger(child, 'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 0 }); expect(child.hasClass('ng-enter')).toBe(true); expect(child.hasClass('ng-enter-active')).toBe(true); - browserTrigger(child, 'transitionend', { timeStamp: Date.now() + 2000 }); + browserTrigger(child, 'transitionend', { timeStamp: Date.now() + 2000, elapsedTime: 2000 }); expect(child.hasClass('ng-enter')).toBe(false); expect(child.hasClass('ng-enter-active')).toBe(false); From c12c02fe9df1c7115f2c88c30d15fac93527130c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Thu, 10 Oct 2013 09:16:19 -0400 Subject: [PATCH 3/7] fix($animate): cancel any ongoing child animations during move and leave animations --- src/ngAnimate/animate.js | 38 +++++++++++++--- test/ngAnimate/animateSpec.js | 84 ++++++++++++++++++++++++++++++++--- 2 files changed, 110 insertions(+), 12 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index ee637b3ba4a1..9b09cad23d12 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -200,6 +200,7 @@ angular.module('ngAnimate', ['ng']) var selectors = $animateProvider.$$selectors; var NG_ANIMATE_STATE = '$$ngAnimateState'; + var NG_ANIMATE_CLASS_NAME = 'ng-animate'; var rootAnimateState = {running:true}; $provide.decorator('$animate', ['$delegate', '$injector', '$sniffer', '$rootElement', '$timeout', '$rootScope', function($delegate, $injector, $sniffer, $rootElement, $timeout, $rootScope) { @@ -320,6 +321,7 @@ angular.module('ngAnimate', ['ng']) * @param {function()=} done callback function that will be called once the animation is complete */ leave : function(element, done) { + cancelChildAnimations(element); $rootScope.$$postDigest(function() { performAnimation('leave', 'ng-leave', element, null, null, function() { $delegate.leave(element, done); @@ -358,6 +360,7 @@ angular.module('ngAnimate', ['ng']) * @param {function()=} done callback function that will be called once the animation is complete */ move : function(element, parent, after, done) { + cancelChildAnimations(element); $delegate.move(element, parent, after); $rootScope.$$postDigest(function() { performAnimation('move', 'ng-move', element, null, null, function() { @@ -503,6 +506,10 @@ angular.module('ngAnimate', ['ng']) done:done }); + //the ng-animate class does nothing, but it's here to allow for + //parent animations to find and cancel child animations when needed + element.addClass(NG_ANIMATE_CLASS_NAME); + forEach(animations, function(animation, index) { var fn = function() { progress(index); @@ -519,12 +526,6 @@ angular.module('ngAnimate', ['ng']) } }); - function cancelAnimations(animations) { - var isCancelledFlag = true; - forEach(animations, function(animation) { - (animation.endFn || noop)(isCancelledFlag); - }); - } function progress(index) { animations[index].done = true; @@ -538,11 +539,34 @@ angular.module('ngAnimate', ['ng']) function done() { if(!done.hasBeenRun) { done.hasBeenRun = true; - element.removeData(NG_ANIMATE_STATE); + cleanup(element); (onComplete || noop)(); } } } + + function cancelChildAnimations(element) { + angular.forEach(element[0].querySelectorAll('.' + NG_ANIMATE_CLASS_NAME), function(element) { + element = angular.element(element); + var data = element.data(NG_ANIMATE_STATE); + if(data) { + cancelAnimations(data.animations); + cleanup(element); + } + }); + } + + function cancelAnimations(animations) { + var isCancelledFlag = true; + forEach(animations, function(animation) { + (animation.endFn || noop)(isCancelledFlag); + }); + } + + function cleanup(element) { + element.removeClass(NG_ANIMATE_CLASS_NAME); + element.removeData(NG_ANIMATE_STATE); + } }]); $animateProvider.register('', ['$window', '$sniffer', function($window, $sniffer) { diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 90868ef6abc4..5e53c5850f03 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -696,8 +696,9 @@ describe("ngAnimate", function() { $rootScope.$digest(); if ($sniffer.transitions) { - expect(element.hasClass('abc ng-enter')).toBe(true); - expect(element.hasClass('abc ng-enter ng-enter-active')).toBe(true); + expect(element.hasClass('abc')).toBe(true); + expect(element.hasClass('ng-enter')).toBe(true); + expect(element.hasClass('ng-enter-active')).toBe(true); browserTrigger(element,'transitionend', { timeStamp: Date.now() + 22000, elapsedTime: 22000 }); } expect(element.hasClass('abc')).toBe(true); @@ -708,7 +709,8 @@ describe("ngAnimate", function() { if ($sniffer.transitions) { expect(element.hasClass('xyz')).toBe(true); - expect(element.hasClass('xyz ng-enter ng-enter-active')).toBe(true); + expect(element.hasClass('ng-enter')).toBe(true); + expect(element.hasClass('ng-enter-active')).toBe(true); browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000, elapsedTime: 11000 }); } expect(element.hasClass('xyz')).toBe(true); @@ -732,8 +734,10 @@ describe("ngAnimate", function() { $animate.enter(element, parent); $rootScope.$digest(); if($sniffer.transitions) { - expect(element.hasClass('one two ng-enter')).toBe(true); - expect(element.hasClass('one two ng-enter ng-enter-active')).toBe(true); + expect(element.hasClass('one')).toBe(true); + expect(element.hasClass('two')).toBe(true); + expect(element.hasClass('ng-enter')).toBe(true); + expect(element.hasClass('ng-enter-active')).toBe(true); expect(element.hasClass('one-active')).toBe(false); expect(element.hasClass('two-active')).toBe(false); browserTrigger(element,'transitionend', { timeStamp: Date.now() + 3000, elapsedTime: 3000 }); @@ -1574,4 +1578,74 @@ describe("ngAnimate", function() { expect(element.contents().length).toBe(1); })); + it("should cancel all child animations when a leave or move animation is triggered on a parent element", function() { + + var animationState; + module(function($animateProvider) { + $animateProvider.register('.animan', function($timeout) { + return { + enter : function(element, done) { + animationState = 'enter'; + $timeout(done, 0, false); + return function() { + animationState = 'enter-cancel'; + } + }, + addClass : function(element, className, done) { + animationState = 'addClass'; + $timeout(done, 0, false); + return function() { + animationState = 'addClass-cancel'; + } + } + }; + }); + }); + + inject(function($animate, $compile, $rootScope, $timeout, $sniffer) { + var element = html($compile('
')($rootScope)); + var container = html($compile('
')($rootScope)); + var child = html($compile('
')($rootScope)); + + ss.addRule('.animan.ng-enter, .animan.something-add', '-webkit-transition: width 1s, background 1s 1s;' + + 'transition: width 1s, background 1s 1s;'); + + $rootElement.append(element); + jqLite(document.body).append($rootElement); + + $animate.enter(child, element); + $rootScope.$digest(); + + expect(animationState).toBe('enter'); + if($sniffer.transitions) { + expect(child.hasClass('ng-enter')).toBe(true); + expect(child.hasClass('ng-enter-active')).toBe(true); + } + + $animate.move(element, container); + if($sniffer.transitions) { + expect(child.hasClass('ng-enter')).toBe(false); + expect(child.hasClass('ng-enter-active')).toBe(false); + } + + expect(animationState).toBe('enter-cancel'); + $rootScope.$digest(); + $timeout.flush(); + + $animate.addClass(child, 'something'); + expect(animationState).toBe('addClass'); + if($sniffer.transitions) { + expect(child.hasClass('something-add')).toBe(true); + expect(child.hasClass('something-add-active')).toBe(true); + } + + $animate.leave(container); + expect(animationState).toBe('addClass-cancel'); + if($sniffer.transitions) { + expect(child.hasClass('something-add')).toBe(false); + expect(child.hasClass('something-add-active')).toBe(false); + } + }); + }); + }); From e8bd341dc44ba4568dc800064eadf55dfb64563d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Mon, 7 Oct 2013 14:35:24 -0400 Subject: [PATCH 4/7] refactor($animate): queue all successive animations to use only one reflow --- src/ngAnimate/animate.js | 29 +++++++++--- test/ngAnimate/animateSpec.js | 89 ++++++++++++++++++++++++++++++----- 2 files changed, 98 insertions(+), 20 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 9b09cad23d12..fc865a650c5a 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -569,7 +569,7 @@ angular.module('ngAnimate', ['ng']) } }]); - $animateProvider.register('', ['$window', '$sniffer', function($window, $sniffer) { + $animateProvider.register('', ['$window', '$sniffer', '$timeout', function($window, $sniffer, $timeout) { var forEach = angular.forEach; // Detect proper transitionend/animationend event names. @@ -605,6 +605,19 @@ angular.module('ngAnimate', ['ng']) animationIterationCountKey = 'IterationCount', ELEMENT_NODE = 1; + var animationReflowQueue = [], animationTimer, timeOut = false; + function afterReflow(callback) { + animationReflowQueue.push(callback); + $timeout.cancel(animationTimer); + animationTimer = $timeout(function() { + angular.forEach(animationReflowQueue, function(fn) { + fn(); + }); + animationReflowQueue = []; + animationTimer = null; + }, 10, false); + } + function animate(element, className, done) { if(['ng-enter','ng-leave','ng-move'].indexOf(className) == -1) { var existingDuration = 0; @@ -670,13 +683,15 @@ angular.module('ngAnimate', ['ng']) }); // This triggers a reflow which allows for the transition animation to kick in. - element.prop('clientWidth'); - if(transitionDuration > 0) { - node.style[transitionProp + propertyKey] = ''; - } - element.addClass(activeClassName); - var css3AnimationEvents = animationendEvent + ' ' + transitionendEvent; + + afterReflow(function() { + if(transitionDuration > 0) { + node.style[transitionProp + propertyKey] = ''; + } + element.addClass(activeClassName); + }); + element.on(css3AnimationEvents, onAnimationProgress); // This will automatically be called by $animate so diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 5e53c5850f03..b51a358412f7 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -127,7 +127,7 @@ describe("ngAnimate", function() { }) it("should animate the enter animation event", - inject(function($animate, $rootScope, $sniffer) { + inject(function($animate, $rootScope, $sniffer, $timeout) { element[0].removeChild(child[0]); expect(element.contents().length).toBe(0); @@ -135,6 +135,7 @@ describe("ngAnimate", function() { $rootScope.$digest(); if($sniffer.transitions) { + $timeout.flush(); expect(child.hasClass('ng-enter')).toBe(true); expect(child.hasClass('ng-enter-active')).toBe(true); browserTrigger(element, 'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); @@ -144,13 +145,14 @@ describe("ngAnimate", function() { })); it("should animate the leave animation event", - inject(function($animate, $rootScope, $sniffer) { + inject(function($animate, $rootScope, $sniffer, $timeout) { expect(element.contents().length).toBe(1); $animate.leave(child); $rootScope.$digest(); if($sniffer.transitions) { + $timeout.flush(); expect(child.hasClass('ng-leave')).toBe(true); expect(child.hasClass('ng-leave-active')).toBe(true); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); @@ -160,7 +162,7 @@ describe("ngAnimate", function() { })); it("should animate the move animation event", - inject(function($animate, $compile, $rootScope) { + inject(function($animate, $compile, $rootScope, $timeout, $sniffer) { $rootScope.$digest(); element.html(''); @@ -172,17 +174,21 @@ describe("ngAnimate", function() { expect(element.text()).toBe('12'); $animate.move(child1, element, child2); $rootScope.$digest(); + if($sniffer.transitions) { + $timeout.flush(); + } expect(element.text()).toBe('21'); })); it("should animate the show animation event", - inject(function($animate, $rootScope, $sniffer) { + inject(function($animate, $rootScope, $sniffer, $timeout) { $rootScope.$digest(); child.addClass('ng-hide'); expect(child).toBeHidden(); $animate.removeClass(child, 'ng-hide'); if($sniffer.transitions) { + $timeout.flush(); expect(child.hasClass('ng-hide-remove')).toBe(true); expect(child.hasClass('ng-hide-remove-active')).toBe(true); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); @@ -193,12 +199,13 @@ describe("ngAnimate", function() { })); it("should animate the hide animation event", - inject(function($animate, $rootScope, $sniffer) { + inject(function($animate, $rootScope, $sniffer, $timeout) { $rootScope.$digest(); expect(child).toBeShown(); $animate.addClass(child, 'ng-hide'); if($sniffer.transitions) { + $timeout.flush(); expect(child.hasClass('ng-hide-add')).toBe(true); expect(child.hasClass('ng-hide-add-active')).toBe(true); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); @@ -207,7 +214,7 @@ describe("ngAnimate", function() { })); it("should assign the ng-event className to all animation events when transitions/keyframes are used", - inject(function($animate, $sniffer, $rootScope) { + inject(function($animate, $sniffer, $rootScope, $timeout) { if (!$sniffer.transitions) return; @@ -217,6 +224,7 @@ describe("ngAnimate", function() { //enter $animate.enter(child, element); $rootScope.$digest(); + $timeout.flush(); expect(child.attr('class')).toContain('ng-enter'); expect(child.attr('class')).toContain('ng-enter-active'); @@ -226,6 +234,7 @@ describe("ngAnimate", function() { element.append(after); $animate.move(child, element, after); $rootScope.$digest(); + $timeout.flush(); expect(child.attr('class')).toContain('ng-move'); expect(child.attr('class')).toContain('ng-move-active'); @@ -233,12 +242,14 @@ describe("ngAnimate", function() { //hide $animate.addClass(child, 'ng-hide'); + $timeout.flush(); expect(child.attr('class')).toContain('ng-hide-add'); expect(child.attr('class')).toContain('ng-hide-add-active'); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); //show $animate.removeClass(child, 'ng-hide'); + $timeout.flush(); expect(child.attr('class')).toContain('ng-hide-remove'); expect(child.attr('class')).toContain('ng-hide-remove-active'); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); @@ -246,6 +257,7 @@ describe("ngAnimate", function() { //leave $animate.leave(child); $rootScope.$digest(); + $timeout.flush(); expect(child.attr('class')).toContain('ng-leave'); expect(child.attr('class')).toContain('ng-leave-active'); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); @@ -283,6 +295,7 @@ describe("ngAnimate", function() { $animate.leave(child); $rootScope.$digest(); + $timeout.flush(); expect(child).toBeHidden(); //hides instantly //lets change this to prove that done doesn't fire anymore for the previous hide() operation @@ -507,7 +520,7 @@ describe("ngAnimate", function() { })); it("should finish the previous animation when a new animation is started", - inject(function($animate, $rootScope, $compile, $sniffer) { + inject(function($animate, $rootScope, $compile, $sniffer, $timeout) { var style = '-webkit-animation: some_animation 2s linear 0s 1 alternate;' + 'animation: some_animation 2s linear 0s 1 alternate;'; @@ -518,7 +531,9 @@ describe("ngAnimate", function() { element.addClass('custom'); $animate.removeClass(element, 'ng-hide'); + if($sniffer.animations) { + $timeout.flush(); expect(element.hasClass('ng-hide-remove')).toBe(true); expect(element.hasClass('ng-hide-remove-active')).toBe(true); } @@ -528,6 +543,7 @@ describe("ngAnimate", function() { if($sniffer.animations) { //cleanup some pending animations + $timeout.flush(); expect(element.hasClass('ng-hide-add')).toBe(true); expect(element.hasClass('ng-hide-add-active')).toBe(true); browserTrigger(element,'animationend', { timeStamp: Date.now() + 2000, elapsedTime: 2000 }); @@ -648,7 +664,7 @@ describe("ngAnimate", function() { })); it("should finish the previous transition when a new animation is started", - inject(function($animate, $rootScope, $compile, $sniffer) { + inject(function($animate, $rootScope, $compile, $sniffer, $timeout) { var style = '-webkit-transition: 1s linear all;' + 'transition: 1s linear all;'; @@ -659,7 +675,9 @@ describe("ngAnimate", function() { element.addClass('ng-hide'); $animate.removeClass(element, 'ng-hide'); + if($sniffer.transitions) { + $timeout.flush(); expect(element.hasClass('ng-hide-remove')).toBe(true); expect(element.hasClass('ng-hide-remove-active')).toBe(true); browserTrigger(element,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); @@ -669,7 +687,9 @@ describe("ngAnimate", function() { expect(element).toBeShown(); $animate.addClass(element, 'ng-hide'); + if($sniffer.transitions) { + $timeout.flush(); expect(element.hasClass('ng-hide-add')).toBe(true); expect(element.hasClass('ng-hide-add-active')).toBe(true); } @@ -696,6 +716,7 @@ describe("ngAnimate", function() { $rootScope.$digest(); if ($sniffer.transitions) { + $timeout.flush(); expect(element.hasClass('abc')).toBe(true); expect(element.hasClass('ng-enter')).toBe(true); expect(element.hasClass('ng-enter-active')).toBe(true); @@ -708,6 +729,7 @@ describe("ngAnimate", function() { $rootScope.$digest(); if ($sniffer.transitions) { + $timeout.flush(); expect(element.hasClass('xyz')).toBe(true); expect(element.hasClass('ng-enter')).toBe(true); expect(element.hasClass('ng-enter-active')).toBe(true); @@ -717,7 +739,7 @@ describe("ngAnimate", function() { })); it('should only append active to the newly append CSS className values', - inject(function($animate, $rootScope, $sniffer, $rootElement) { + inject(function($animate, $rootScope, $sniffer, $rootElement, $timeout) { ss.addRule('.ng-enter', '-webkit-transition:9s linear all;' + 'transition:9s linear all;'); @@ -733,7 +755,9 @@ describe("ngAnimate", function() { $animate.enter(element, parent); $rootScope.$digest(); + if($sniffer.transitions) { + $timeout.flush(); expect(element.hasClass('one')).toBe(true); expect(element.hasClass('two')).toBe(true); expect(element.hasClass('ng-enter')).toBe(true); @@ -1013,7 +1037,9 @@ describe("ngAnimate", function() { $animate.addClass(element,'klass', function() { signature += '1'; }); + if($sniffer.transitions) { + $timeout.flush(); expect(element.hasClass('klass')).toBe(false); expect(element.hasClass('klass-add')).toBe(true); expect(element.hasClass('klass-add-active')).toBe(true); @@ -1026,6 +1052,7 @@ describe("ngAnimate", function() { }); if($sniffer.transitions) { + $timeout.flush(); expect(element.hasClass('klass')).toBe(true); expect(element.hasClass('klass-add')).toBe(false); expect(element.hasClass('klass-add-active')).toBe(false); @@ -1086,7 +1113,9 @@ describe("ngAnimate", function() { $animate.addClass(element,'klass', function() { signature += 'd'; }); + if($sniffer.transitions) { + $timeout.flush(); expect(element.hasClass('klass-add')).toBe(true); expect(element.hasClass('klass-add-active')).toBe(true); browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000, elapsedTime: 11000 }); @@ -1100,7 +1129,9 @@ describe("ngAnimate", function() { $animate.removeClass(element,'klass', function() { signature += 'b'; }); + if($sniffer.transitions) { + $timeout.flush(); expect(element.hasClass('klass-remove')).toBe(true); expect(element.hasClass('klass-remove-active')).toBe(true); browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000, elapsedTime: 11000 }); @@ -1133,6 +1164,7 @@ describe("ngAnimate", function() { }); if($sniffer.transitions) { + $timeout.flush(); expect(element.hasClass('one-add')).toBe(true); expect(element.hasClass('two-add')).toBe(true); @@ -1177,6 +1209,7 @@ describe("ngAnimate", function() { }); if($sniffer.transitions) { + $timeout.flush(); expect(element.hasClass('one-remove')).toBe(true); expect(element.hasClass('two-remove')).toBe(true); @@ -1217,7 +1250,7 @@ describe("ngAnimate", function() { } it("should properly animate and parse CSS3 transitions", - inject(function($compile, $rootScope, $animate, $sniffer) { + inject(function($compile, $rootScope, $animate, $sniffer, $timeout) { ss.addRule('.ng-enter', '-webkit-transition:1s linear all;' + 'transition:1s linear all;'); @@ -1229,6 +1262,7 @@ describe("ngAnimate", function() { $rootScope.$digest(); if($sniffer.transitions) { + $timeout.flush(); expect(child.hasClass('ng-enter')).toBe(true); expect(child.hasClass('ng-enter-active')).toBe(true); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); @@ -1239,7 +1273,7 @@ describe("ngAnimate", function() { })); it("should properly animate and parse CSS3 animations", - inject(function($compile, $rootScope, $animate, $sniffer) { + inject(function($compile, $rootScope, $animate, $sniffer, $timeout) { ss.addRule('.ng-enter', '-webkit-animation: some_animation 4s linear 1s 2 alternate;' + 'animation: some_animation 4s linear 1s 2 alternate;'); @@ -1251,6 +1285,7 @@ describe("ngAnimate", function() { $rootScope.$digest(); if($sniffer.transitions) { + $timeout.flush(); expect(child.hasClass('ng-enter')).toBe(true); expect(child.hasClass('ng-enter-active')).toBe(true); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 9000, elapsedTime: 9000 }); @@ -1260,7 +1295,7 @@ describe("ngAnimate", function() { })); it("should not set the transition property flag if only CSS animations are used", - inject(function($compile, $rootScope, $animate, $sniffer) { + inject(function($compile, $rootScope, $animate, $sniffer, $timeout) { if (!$sniffer.animations) return; @@ -1278,6 +1313,7 @@ describe("ngAnimate", function() { $animate.enter(child, element); $rootScope.$digest(); + $timeout.flush(); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 2000, elapsedTime: 2000 }); @@ -1369,6 +1405,7 @@ describe("ngAnimate", function() { //this is added/removed right away otherwise if($sniffer.transitions) { + $timeout.flush(); expect(child.hasClass('ng-enter')).toBe(true); expect(child.hasClass('ng-enter-active')).toBe(true); } @@ -1377,6 +1414,7 @@ describe("ngAnimate", function() { child.addClass('usurper'); $animate.leave(child); $rootScope.$digest(); + $timeout.flush(); expect(child.hasClass('ng-enter')).toBe(false); expect(child.hasClass('ng-enter-active')).toBe(false); @@ -1561,6 +1599,7 @@ describe("ngAnimate", function() { $animate.enter(child, element); $rootScope.$digest(); + $timeout.flush(); expect(child.hasClass('ng-enter')).toBe(true); expect(child.hasClass('ng-enter-active')).toBe(true); @@ -1619,6 +1658,7 @@ describe("ngAnimate", function() { expect(animationState).toBe('enter'); if($sniffer.transitions) { expect(child.hasClass('ng-enter')).toBe(true); + $timeout.flush(); expect(child.hasClass('ng-enter-active')).toBe(true); } @@ -1630,12 +1670,12 @@ describe("ngAnimate", function() { expect(animationState).toBe('enter-cancel'); $rootScope.$digest(); - $timeout.flush(); $animate.addClass(child, 'something'); expect(animationState).toBe('addClass'); if($sniffer.transitions) { expect(child.hasClass('something-add')).toBe(true); + $timeout.flush(); expect(child.hasClass('something-add-active')).toBe(true); } @@ -1648,4 +1688,27 @@ describe("ngAnimate", function() { }); }); + it("should wait until a queue of animations are complete before performing a reflow", + inject(function($rootScope, $compile, $timeout,$sniffer) { + + if(!$sniffer.transitions) return; + + $rootScope.items = [1,2,3,4,5]; + var element = html($compile('
')($rootScope)); + + ss.addRule('.animated.ng-enter', '-webkit-transition: width 1s, background 1s 1s;' + + 'transition: width 1s, background 1s 1s;'); + + $rootScope.$digest(); + expect(element[0].querySelectorAll('.ng-enter-active').length).toBe(0); + $timeout.flush(); + expect(element[0].querySelectorAll('.ng-enter-active').length).toBe(5); + + angular.forEach(element.children(), function(kid) { + browserTrigger(kid, 'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); + }); + + expect(element[0].querySelectorAll('.ng-enter-active').length).toBe(0); + })); + }); From 21e1a66cfa46d6e7c66488fa0e09ae1c93272089 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Tue, 8 Oct 2013 23:40:46 -0400 Subject: [PATCH 5/7] fix($animate): ensure structural animations skip all child animations even if no animation is present during compile Closes #3215 --- src/ngAnimate/animate.js | 75 +++++++++++---- test/ngAnimate/animateSpec.js | 170 +++++++++++++++++++++++++++++++++- 2 files changed, 223 insertions(+), 22 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index fc865a650c5a..4bd7b886d817 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -285,6 +285,7 @@ angular.module('ngAnimate', ['ng']) * @param {function()=} done callback function that will be called once the animation is complete */ enter : function(element, parent, after, done) { + this.enabled(false, element); $delegate.enter(element, parent, after); $rootScope.$$postDigest(function() { performAnimation('enter', 'ng-enter', element, parent, after, function() { @@ -322,6 +323,7 @@ angular.module('ngAnimate', ['ng']) */ leave : function(element, done) { cancelChildAnimations(element); + this.enabled(false, element); $rootScope.$$postDigest(function() { performAnimation('leave', 'ng-leave', element, null, null, function() { $delegate.leave(element, done); @@ -361,6 +363,7 @@ angular.module('ngAnimate', ['ng']) */ move : function(element, parent, after, done) { cancelChildAnimations(element); + this.enabled(false, element); $delegate.move(element, parent, after); $rootScope.$$postDigest(function() { performAnimation('move', 'ng-move', element, null, null, function() { @@ -451,12 +454,30 @@ angular.module('ngAnimate', ['ng']) * Globally enables/disables animations. * */ - enabled : function(value) { - if (arguments.length) { - rootAnimateState.running = !value; + enabled : function(value, element) { + switch(arguments.length) { + case 2: + if(value) { + cleanup(element); + } + else { + var data = element.data(NG_ANIMATE_STATE) || {}; + data.structural = true; + data.running = true; + element.data(NG_ANIMATE_STATE, data); + } + break; + + case 1: + rootAnimateState.running = !value; + break; + + default: + value = !rootAnimateState.running + break; } - return !rootAnimateState.running; - } + return !!value; + } }; /* @@ -484,24 +505,29 @@ angular.module('ngAnimate', ['ng']) //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) { - //avoid calling done() since there is no need to remove any - //data or className values since this happens earlier than that - //and also use a timeout so that it won't be asynchronous - onComplete && onComplete(); + done(); return; } var ngAnimateState = element.data(NG_ANIMATE_STATE) || {}; - //if an animation is currently running on the element then lets take the steps - //to cancel that animation and fire any required callbacks + var isClassBased = event == 'addClass' || event == 'removeClass'; if(ngAnimateState.running) { + if(isClassBased && ngAnimateState.structural) { + onComplete && onComplete(); + return; + } + + //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); cancelAnimations(ngAnimateState.animations); - ngAnimateState.done(); + (ngAnimateState.done || noop)(); } element.data(NG_ANIMATE_STATE, { running:true, + structural:!isClassBased, animations:animations, done:done }); @@ -516,17 +542,14 @@ angular.module('ngAnimate', ['ng']) }; if(animation.start) { - if(event == 'addClass' || event == 'removeClass') { - animation.endFn = animation.start(element, className, fn); - } else { - animation.endFn = animation.start(element, fn); - } + animation.endFn = isClassBased ? + animation.start(element, className, fn) : + animation.start(element, fn); } else { fn(); } }); - function progress(index) { animations[index].done = true; (animations[index].endFn || noop)(); @@ -539,7 +562,21 @@ angular.module('ngAnimate', ['ng']) function done() { if(!done.hasBeenRun) { done.hasBeenRun = true; - cleanup(element); + var data = element.data(NG_ANIMATE_STATE); + if(data) { + /* only structural animations wait for reflow before removing an + animation, but class-based animations don't. An example of this + failing would be when a parent HTML tag has a ng-class attribute + causing ALL directives below to skip animations during the digest */ + if(isClassBased) { + cleanup(element); + } else { + data.flagTimer = $timeout(function() { + cleanup(element); + }, 0, false); + element.data(NG_ANIMATE_STATE, data); + } + } (onComplete || noop)(); } } diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index b51a358412f7..3652e450f832 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -36,7 +36,7 @@ describe("ngAnimate", function() { describe("enable / disable", function() { - it("should disable and enable the animations", function() { + it("should work for all animations", function() { var $animate, initialState = null; angular.bootstrap(body, ['ngAnimate',function() { @@ -56,7 +56,6 @@ describe("ngAnimate", function() { expect($animate.enabled(1)).toBe(true); expect($animate.enabled()).toBe(true); }); - }); describe("with polyfill", function() { @@ -229,6 +228,7 @@ describe("ngAnimate", function() { expect(child.attr('class')).toContain('ng-enter'); expect(child.attr('class')).toContain('ng-enter-active'); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); + $timeout.flush(); //move element.append(after); @@ -239,6 +239,7 @@ describe("ngAnimate", function() { expect(child.attr('class')).toContain('ng-move'); expect(child.attr('class')).toContain('ng-move-active'); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); + $timeout.flush(); //hide $animate.addClass(child, 'ng-hide'); @@ -261,6 +262,7 @@ describe("ngAnimate", function() { expect(child.attr('class')).toContain('ng-leave'); expect(child.attr('class')).toContain('ng-leave-active'); browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); + $timeout.flush(); })); it("should not run if animations are disabled", @@ -330,6 +332,29 @@ describe("ngAnimate", function() { expect(child.hasClass('animation-cancelled')).toBe(true); })); + it("should skip a class-based animation if the same element already has an ongoing structural animation", + inject(function($animate, $rootScope, $sniffer, $timeout) { + + var completed = false; + $animate.enter(child, element, null, function() { + completed = true; + }); + $rootScope.$digest(); + + expect(completed).toBe(false); + + $animate.addClass(child, 'green'); + expect(element.hasClass('green')); + + expect(completed).toBe(false); + if($sniffer.transitions) { + $timeout.flush(); + browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 }); + } + $timeout.flush(); + + expect(completed).toBe(true); + })); it("should fire the cancel/end function with the correct flag in the parameters", inject(function($animate, $rootScope, $sniffer, $timeout) { @@ -722,6 +747,7 @@ describe("ngAnimate", function() { expect(element.hasClass('ng-enter-active')).toBe(true); browserTrigger(element,'transitionend', { timeStamp: Date.now() + 22000, elapsedTime: 22000 }); } + $timeout.flush(); expect(element.hasClass('abc')).toBe(true); $rootScope.klass = 'xyz'; @@ -735,6 +761,7 @@ describe("ngAnimate", function() { expect(element.hasClass('ng-enter-active')).toBe(true); browserTrigger(element,'transitionend', { timeStamp: Date.now() + 11000, elapsedTime: 11000 }); } + $timeout.flush(); expect(element.hasClass('xyz')).toBe(true); })); @@ -767,7 +794,8 @@ describe("ngAnimate", function() { browserTrigger(element,'transitionend', { timeStamp: Date.now() + 3000, elapsedTime: 3000 }); } - expect(element.hasClass('one two')).toBe(true); + expect(element.hasClass('one')).toBe(true); + expect(element.hasClass('two')).toBe(true); })); }); @@ -1670,6 +1698,7 @@ describe("ngAnimate", function() { expect(animationState).toBe('enter-cancel'); $rootScope.$digest(); + $timeout.flush(); $animate.addClass(child, 'something'); expect(animationState).toBe('addClass'); @@ -1711,4 +1740,139 @@ describe("ngAnimate", function() { expect(element[0].querySelectorAll('.ng-enter-active').length).toBe(0); })); + + it("should work to disable all child animations for an element", function() { + var childAnimated = false, + containerAnimated = false; + module(function($animateProvider) { + $animateProvider.register('.child', function() { + return { + addClass : function(element, className, done) { + childAnimated = true; + done(); + } + } + }); + $animateProvider.register('.container', function() { + return { + leave : function(element, done) { + containerAnimated = true; + done(); + } + } + }); + }); + + inject(function($compile, $rootScope, $animate, $timeout, $rootElement) { + $animate.enabled(true); + + var element = $compile('
')($rootScope); + jqLite($document[0].body).append($rootElement); + $rootElement.append(element); + + var child = $compile('
')($rootScope); + element.append(child); + + $animate.enabled(true, element); + + $animate.addClass(child, 'awesome'); + expect(childAnimated).toBe(true); + + childAnimated = false; + $animate.enabled(false, element); + + $animate.addClass(child, 'super'); + expect(childAnimated).toBe(false); + + $animate.leave(element); + $rootScope.$digest(); + expect(containerAnimated).toBe(true); + }); + }); + + it("should disable all child animations on structural animations until the first reflow has passed", function() { + var intercepted; + module(function($animateProvider) { + $animateProvider.register('.animated', function() { + return { + enter : ani('enter'), + leave : ani('leave'), + move : ani('move'), + addClass : ani('addClass'), + removeClass : ani('removeClass') + }; + + function ani(type) { + return function(element, className, done) { + intercepted = type; + (done || className)(); + } + } + }); + }); + + inject(function($animate, $rootScope, $sniffer, $timeout, $compile, _$rootElement_) { + $rootElement = _$rootElement_; + + $animate.enabled(true); + $rootScope.$digest(); + + var element = $compile('
...
')($rootScope); + var child1 = $compile('
...
')($rootScope); + var child2 = $compile('
...
')($rootScope); + var container = $compile('
...
')($rootScope); + + jqLite($document[0].body).append($rootElement); + $rootElement.append(container); + element.append(child1); + element.append(child2); + + $animate.move(element, null, container); + $rootScope.$digest(); + + expect(intercepted).toBe('move'); + + $animate.addClass(child1, 'test'); + expect(child1.hasClass('test')).toBe(true); + + expect(intercepted).toBe('move'); + $animate.leave(child1); + $rootScope.$digest(); + + expect(intercepted).toBe('move'); + + //reflow has passed + $timeout.flush(); + + $animate.leave(child2); + $rootScope.$digest(); + expect(intercepted).toBe('leave'); + }); + }); + + it("should not disable any child animations when any parent class-based animations are run", function() { + var intercepted; + module(function($animateProvider) { + $animateProvider.register('.animated', function() { + return { + enter : function(element, done) { + intercepted = true; + done(); + } + } + }); + }); + + inject(function($animate, $rootScope, $sniffer, $timeout, $compile, $document, $rootElement) { + $animate.enabled(true); + + var element = $compile('
value
')($rootScope); + $rootElement.append(element); + jqLite($document[0].body).append($rootElement); + + $rootScope.bool = true; + $rootScope.$digest(); + expect(intercepted).toBe(true); + }); + }); }); From a465f8f488cd86c45f9edc0ceb065e03e2e3e2e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Tue, 8 Oct 2013 13:13:10 -0400 Subject: [PATCH 6/7] chore(ngdocs): improve the side search animation effects --- docs/src/templates/css/animations.css | 34 ++++++++++++++++++--------- docs/src/templates/index.html | 10 ++++---- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/docs/src/templates/css/animations.css b/docs/src/templates/css/animations.css index 132ee3523cac..7c4049809c67 100644 --- a/docs/src/templates/css/animations.css +++ b/docs/src/templates/css/animations.css @@ -38,18 +38,21 @@ } .expand.ng-enter, +.expand.ng-move, .expand.ng-leave { -webkit-transition:0.3s cubic-bezier(0.250, 0.460, 0.450, 0.940) all; -moz-transition:0.3s cubic-bezier(0.250, 0.460, 0.450, 0.940) all; -o-transition:0.3s cubic-bezier(0.250, 0.460, 0.450, 0.940) all; transition:0.3s cubic-bezier(0.250, 0.460, 0.450, 0.940) all; } +.expand.ng-move, .expand.ng-enter { opacity:0; line-height:0; height:0!important; } -.expand.ng-enter.expand.ng-enter-active { +.expand.ng-move.ng-move-active, +.expand.ng-enter.ng-enter-active { opacity:1; line-height:20px; height:20px!important; @@ -59,12 +62,17 @@ opacity:1; height:20px; } -.expand.ng-leave.expand.ng-leave-active { +.expand.ng-leave.ng-leave-active { opacity:0; height:0; } +.fade-in.ng-enter, +.fade-in.ng-move, +.fade-in.ng-hide-add, +.fade-in.ng-hide-remove, .foldout.ng-enter, +.foldout.ng-move, .foldout.ng-hide-add, .foldout.ng-hide-remove { -webkit-transition:0.3s cubic-bezier(0.250, 0.460, 0.450, 0.940) all; @@ -73,20 +81,24 @@ transition:0.3s cubic-bezier(0.250, 0.460, 0.450, 0.940) all; } +.fade-in.ng-enter, +.fade-in.ng-move, +.fade-in.ng-hide-remove, +.fade-in.ng-hide-add.ng-hide-active, .foldout.ng-hide-remove, -.foldout.ng-enter { +.foldout.ng-hide-add.ng-hide-active, +.foldout.ng-enter, +.foldout.ng-move { opacity:0; } +.fade-in.ng-enter.ng-enter-active, +.fade-in.ng-move.ng-move-active, +.fade-in.ng-hide-remove.ng-hide-remove-active, +.fade-in.ng-hide-add, +.foldout.ng-move.ng-move-active, .foldout.ng-hide-remove.ng-hide-remove-active, +.foldout.ng-hide-add, .foldout.ng-enter.ng-enter-active { opacity:1; } - -.foldout.ng-hide-add { - opacity:1; -} - -.foldout.ng-hide-add.ng-hide-active { - opacity:0; -} diff --git a/docs/src/templates/index.html b/docs/src/templates/index.html index 872ca58650d8..e36780546851 100644 --- a/docs/src/templates/index.html +++ b/docs/src/templates/index.html @@ -272,22 +272,22 @@

{{ key }}

Filtered results:
-
    + -
      +
      • module {{module.name}} @@ -326,7 +326,7 @@

        {{ key }}

        global APIs  
        • -
      • +
      • {{page.id}}
        • From ddcbf7604af44e264aeeed75f1afb4a477a637fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Wed, 9 Oct 2013 01:12:03 -0400 Subject: [PATCH 7/7] fix($animate): perform internal caching on getComputedStyle to boost the performance of CSS3 transitions/animations Closes #4011 Closes #4124 --- src/ngAnimate/animate.js | 105 +++++++++++++++++++++------------- test/ngAnimate/animateSpec.js | 53 +++++++++++++++-- 2 files changed, 114 insertions(+), 44 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 4bd7b886d817..f5be6b766cb0 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -642,6 +642,10 @@ angular.module('ngAnimate', ['ng']) animationIterationCountKey = 'IterationCount', ELEMENT_NODE = 1; + var NG_ANIMATE_PARENT_KEY = '$ngAnimateKey'; + var lookupCache = {}; + var parentCounter = 0; + var animationReflowQueue = [], animationTimer, timeOut = false; function afterReflow(callback) { animationReflowQueue.push(callback); @@ -652,65 +656,93 @@ angular.module('ngAnimate', ['ng']) }); animationReflowQueue = []; animationTimer = null; + lookupCache = {}; }, 10, false); } - function animate(element, className, done) { - if(['ng-enter','ng-leave','ng-move'].indexOf(className) == -1) { - var existingDuration = 0; + function getElementAnimationDetails(element, cacheKey, onlyCheckTransition) { + var data = lookupCache[cacheKey]; + if(!data) { + var transitionDuration = 0, transitionDelay = 0, + animationDuration = 0, animationDelay = 0; + + //we want all the styles defined before and after forEach(element, function(element) { if (element.nodeType == ELEMENT_NODE) { var elementStyles = $window.getComputedStyle(element) || {}; - existingDuration = Math.max(parseMaxTime(elementStyles[transitionProp + durationKey]), - existingDuration); + + transitionDuration = Math.max(parseMaxTime(elementStyles[transitionProp + durationKey]), transitionDuration); + + if(!onlyCheckTransition) { + transitionDelay = Math.max(parseMaxTime(elementStyles[transitionProp + delayKey]), transitionDelay); + + animationDelay = Math.max(parseMaxTime(elementStyles[animationProp + delayKey]), animationDelay); + + var aDuration = parseMaxTime(elementStyles[animationProp + durationKey]); + + if(aDuration > 0) { + aDuration *= parseInt(elementStyles[animationProp + animationIterationCountKey]) || 1; + } + + animationDuration = Math.max(aDuration, animationDuration); + } } }); - if(existingDuration > 0) { - done(); - return; - } + data = { + transitionDelay : transitionDelay, + animationDelay : animationDelay, + transitionDuration : transitionDuration, + animationDuration : animationDuration + }; + lookupCache[cacheKey] = data; } + return data; + } - element.addClass(className); - - //we want all the styles defined before and after - var transitionDuration = 0, - animationDuration = 0, - transitionDelay = 0, - animationDelay = 0; - forEach(element, function(element) { - if (element.nodeType == ELEMENT_NODE) { - var elementStyles = $window.getComputedStyle(element) || {}; + function parseMaxTime(str) { + var total = 0, values = angular.isString(str) ? str.split(/\s*,\s*/) : []; + forEach(values, function(value) { + total = Math.max(parseFloat(value) || 0, total); + }); + return total; + } - transitionDelay = Math.max(parseMaxTime(elementStyles[transitionProp + delayKey]), transitionDelay); + function getCacheKey(element) { + var parent = element.parent(); + var parentID = parent.data(NG_ANIMATE_PARENT_KEY); + if(!parentID) { + parent.data(NG_ANIMATE_PARENT_KEY, ++parentCounter); + parentID = parentCounter; + } + return parentID + '-' + element[0].className; + } - animationDelay = Math.max(parseMaxTime(elementStyles[animationProp + delayKey]), animationDelay); + function animate(element, className, done) { - transitionDuration = Math.max(parseMaxTime(elementStyles[transitionProp + durationKey]), transitionDuration); + var cacheKey = getCacheKey(element); + if(getElementAnimationDetails(element, cacheKey, true).transitionDuration > 0) { - var aDuration = parseMaxTime(elementStyles[animationProp + durationKey]); + done(); + return; + } - if(aDuration > 0) { - aDuration *= parseInt(elementStyles[animationProp + animationIterationCountKey]) || 1; - } + element.addClass(className); - animationDuration = Math.max(aDuration, animationDuration); - } - }); + var timings = getElementAnimationDetails(element, cacheKey + ' ' + className); /* there is no point in performing a reflow if the animation timeout is empty (this would cause a flicker bug normally in the page. There is also no point in performing an animation that only has a delay and no duration */ - var maxDuration = Math.max(transitionDuration, animationDuration); + var maxDuration = Math.max(timings.transitionDuration, timings.animationDuration); if(maxDuration > 0) { - var maxDelayTime = Math.max(transitionDelay, animationDelay) * 1000, + var maxDelayTime = Math.max(timings.transitionDelay, timings.animationDelay) * 1000, startTime = Date.now(), node = element[0]; //temporarily disable the transition so that the enter styles //don't animate twice (this is here to avoid a bug in Chrome/FF). - if(transitionDuration > 0) { + if(timings.transitionDuration > 0) { node.style[transitionProp + propertyKey] = 'none'; } @@ -723,7 +755,7 @@ angular.module('ngAnimate', ['ng']) var css3AnimationEvents = animationendEvent + ' ' + transitionendEvent; afterReflow(function() { - if(transitionDuration > 0) { + if(timings.transitionDuration > 0) { node.style[transitionProp + propertyKey] = ''; } element.addClass(activeClassName); @@ -768,13 +800,6 @@ angular.module('ngAnimate', ['ng']) } } - function parseMaxTime(str) { - var total = 0, values = angular.isString(str) ? str.split(/\s*,\s*/) : []; - forEach(values, function(value) { - total = Math.max(parseFloat(value) || 0, total); - }); - return total; - } } return { diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 3652e450f832..cae252665a0e 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -726,10 +726,10 @@ describe("ngAnimate", function() { it('should re-evaluate the CSS classes for an animation each time', inject(function($animate, $rootScope, $sniffer, $rootElement, $timeout, $compile) { - ss.addRule('.abc', '-webkit-transition:22s linear all;' + - 'transition:22s linear all;'); - ss.addRule('.xyz', '-webkit-transition:11s linear all;' + - 'transition:11s linear all;'); + ss.addRule('.abc.ng-enter', '-webkit-transition:22s linear all;' + + 'transition:22s linear all;'); + ss.addRule('.xyz.ng-enter', '-webkit-transition:11s linear all;' + + 'transition:11s linear all;'); var parent = $compile('
        ')($rootScope); var element = parent.find('span'); @@ -1875,4 +1875,49 @@ describe("ngAnimate", function() { expect(intercepted).toBe(true); }); }); + + it("should cache the response from getComputedStyle if each successive element has the same className value and parent until the first reflow hits", function() { + var count = 0; + module(function($provide) { + $provide.value('$window', { + document : jqLite(window.document), + getComputedStyle: function(element) { + count++; + return window.getComputedStyle(element); + } + }); + }); + + inject(function($animate, $rootScope, $compile, $rootElement, $timeout, $document, $sniffer) { + if(!$sniffer.transitions) return; + + $animate.enabled(true); + + var element = $compile('
        ')($rootScope); + $rootElement.append(element); + jqLite($document[0].body).append($rootElement); + + for(var i=0;i<20;i++) { + var kid = $compile('
        ')($rootScope); + $animate.enter(kid, element); + } + $rootScope.$digest(); + $timeout.flush(); + + expect(count).toBe(2); + + dealoc(element); + count = 0; + + for(var i=0;i<20;i++) { + var kid = $compile('
        ')($rootScope); + $animate.enter(kid, element); + } + + $rootScope.$digest(); + $timeout.flush(); + + expect(count).toBe(40); + }); + }); });