From 0e2619762398a7cd566c6e7f5141d22081d6b465 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Wed, 2 Mar 2016 16:09:25 -0800 Subject: [PATCH] perf(ngAnimate): avoid repeated calls to addClass/removeClass when animation has no duration Background: ngAnimate writes helper classes to DOM elements to see if animations are defined on them. If many elements have the same definition, and the same parent, we can cache the definition and skip the application of the helper classes altogether. This helps particularly with large ngRepeat collections. Closes #14165 Closes #14166 Closes #16613 --- angularFiles.js | 1 + src/ngAnimate/.eslintrc.json | 1 + src/ngAnimate/animateCache.js | 57 +++++++++++++++ src/ngAnimate/animateCss.js | 92 +++++++++--------------- src/ngAnimate/animateQueue.js | 2 +- src/ngAnimate/animation.js | 74 ++++++++++++++----- src/ngAnimate/module.js | 1 + src/ngAnimate/shared.js | 2 +- test/ngAnimate/.eslintrc.json | 1 + test/ngAnimate/animateCacheSpec.js | 99 +++++++++++++++++++++++++ test/ngAnimate/animationSpec.js | 53 +++++++++++--- test/ngAnimate/integrationSpec.js | 111 +++++++++++++++++++++++++++++ 12 files changed, 408 insertions(+), 86 deletions(-) create mode 100644 src/ngAnimate/animateCache.js create mode 100644 test/ngAnimate/animateCacheSpec.js diff --git a/angularFiles.js b/angularFiles.js index 45d9675261df..16edba44f35d 100644 --- a/angularFiles.js +++ b/angularFiles.js @@ -104,6 +104,7 @@ var angularFiles = { 'src/ngAnimate/animateJs.js', 'src/ngAnimate/animateJsDriver.js', 'src/ngAnimate/animateQueue.js', + 'src/ngAnimate/animateCache.js', 'src/ngAnimate/animation.js', 'src/ngAnimate/ngAnimateSwap.js', 'src/ngAnimate/module.js' diff --git a/src/ngAnimate/.eslintrc.json b/src/ngAnimate/.eslintrc.json index b0100218511b..976e640e7c8a 100644 --- a/src/ngAnimate/.eslintrc.json +++ b/src/ngAnimate/.eslintrc.json @@ -73,6 +73,7 @@ /* ngAnimate directives/services */ "ngAnimateSwapDirective": true, "$$rAFSchedulerFactory": true, + "$$AnimateCacheProvider": true, "$$AnimateChildrenDirective": true, "$$AnimateQueueProvider": true, "$$AnimationProvider": true, diff --git a/src/ngAnimate/animateCache.js b/src/ngAnimate/animateCache.js new file mode 100644 index 000000000000..4465f2a9bcaa --- /dev/null +++ b/src/ngAnimate/animateCache.js @@ -0,0 +1,57 @@ +'use strict'; + +/** @this */ +var $$AnimateCacheProvider = function() { + + var KEY = '$$ngAnimateParentKey'; + var parentCounter = 0; + var cache = Object.create(null); + + this.$get = [function() { + return { + cacheKey: function(node, method, addClass, removeClass) { + var parentNode = node.parentNode; + var parentID = parentNode[KEY] || (parentNode[KEY] = ++parentCounter); + var parts = [parentID, method, node.getAttribute('class')]; + if (addClass) { + parts.push(addClass); + } + if (removeClass) { + parts.push(removeClass); + } + return parts.join(' '); + }, + + containsCachedAnimationWithoutDuration: function(key) { + var entry = cache[key]; + + // nothing cached, so go ahead and animate + // otherwise it should be a valid animation + return (entry && !entry.isValid) || false; + }, + + flush: function() { + cache = Object.create(null); + }, + + count: function(key) { + var entry = cache[key]; + return entry ? entry.total : 0; + }, + + get: function(key) { + var entry = cache[key]; + return entry && entry.value; + }, + + put: function(key, value, isValid) { + if (!cache[key]) { + cache[key] = { total: 1, value: value, isValid: isValid }; + } else { + cache[key].total++; + cache[key].value = value; + } + } + }; + }]; +}; diff --git a/src/ngAnimate/animateCss.js b/src/ngAnimate/animateCss.js index 75c9270ab4d0..e18bb9017e03 100644 --- a/src/ngAnimate/animateCss.js +++ b/src/ngAnimate/animateCss.js @@ -304,33 +304,6 @@ function getCssTransitionDurationStyle(duration, applyOnlyDuration) { return [style, value]; } -function createLocalCacheLookup() { - var cache = Object.create(null); - return { - flush: function() { - cache = Object.create(null); - }, - - count: function(key) { - var entry = cache[key]; - return entry ? entry.total : 0; - }, - - get: function(key) { - var entry = cache[key]; - return entry && entry.value; - }, - - put: function(key, value) { - if (!cache[key]) { - cache[key] = { total: 1, value: value }; - } else { - cache[key].total++; - } - } - }; -} - // we do not reassign an already present style value since // if we detect the style property value again we may be // detecting styles that were added via the `from` styles. @@ -349,26 +322,16 @@ function registerRestorableStyles(backup, node, properties) { } var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animateProvider) { - var gcsLookup = createLocalCacheLookup(); - var gcsStaggerLookup = createLocalCacheLookup(); - this.$get = ['$window', '$$jqLite', '$$AnimateRunner', '$timeout', + this.$get = ['$window', '$$jqLite', '$$AnimateRunner', '$timeout', '$$animateCache', '$$forceReflow', '$sniffer', '$$rAFScheduler', '$$animateQueue', - function($window, $$jqLite, $$AnimateRunner, $timeout, + function($window, $$jqLite, $$AnimateRunner, $timeout, $$animateCache, $$forceReflow, $sniffer, $$rAFScheduler, $$animateQueue) { var applyAnimationClasses = applyAnimationClassesFactory($$jqLite); - var parentCounter = 0; - function gcsHashFn(node, extraClasses) { - var KEY = '$$ngAnimateParentKey'; - var parentNode = node.parentNode; - var parentID = parentNode[KEY] || (parentNode[KEY] = ++parentCounter); - return parentID + '-' + node.getAttribute('class') + '-' + extraClasses; - } - - function computeCachedCssStyles(node, className, cacheKey, properties) { - var timings = gcsLookup.get(cacheKey); + function computeCachedCssStyles(node, className, cacheKey, allowNoDuration, properties) { + var timings = $$animateCache.get(cacheKey); if (!timings) { timings = computeCssStyles($window, node, properties); @@ -377,20 +340,26 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro } } + // if a css animation has no duration we + // should mark that so that repeated addClass/removeClass calls are skipped + var hasDuration = allowNoDuration || (timings.transitionDuration > 0 || timings.animationDuration > 0); + // we keep putting this in multiple times even though the value and the cacheKey are the same // because we're keeping an internal tally of how many duplicate animations are detected. - gcsLookup.put(cacheKey, timings); + $$animateCache.put(cacheKey, timings, hasDuration); + return timings; } function computeCachedCssStaggerStyles(node, className, cacheKey, properties) { var stagger; + var staggerCacheKey = 'stagger-' + cacheKey; // if we have one or more existing matches of matching elements // containing the same parent + CSS styles (which is how cacheKey works) // then staggering is possible - if (gcsLookup.count(cacheKey) > 0) { - stagger = gcsStaggerLookup.get(cacheKey); + if ($$animateCache.count(cacheKey) > 0) { + stagger = $$animateCache.get(staggerCacheKey); if (!stagger) { var staggerClassName = pendClasses(className, '-stagger'); @@ -405,7 +374,7 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro $$jqLite.removeClass(node, staggerClassName); - gcsStaggerLookup.put(cacheKey, stagger); + $$animateCache.put(staggerCacheKey, stagger, true); } } @@ -416,8 +385,7 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro function waitUntilQuiet(callback) { rafWaitQueue.push(callback); $$rAFScheduler.waitUntilQuiet(function() { - gcsLookup.flush(); - gcsStaggerLookup.flush(); + $$animateCache.flush(); // DO NOT REMOVE THIS LINE OR REFACTOR OUT THE `pageWidth` variable. // PLEASE EXAMINE THE `$$forceReflow` service to understand why. @@ -432,8 +400,8 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro }); } - function computeTimings(node, className, cacheKey) { - var timings = computeCachedCssStyles(node, className, cacheKey, DETECT_CSS_PROPERTIES); + function computeTimings(node, className, cacheKey, allowNoDuration) { + var timings = computeCachedCssStyles(node, className, cacheKey, allowNoDuration, DETECT_CSS_PROPERTIES); var aD = timings.animationDelay; var tD = timings.transitionDelay; timings.maxDelay = aD && tD @@ -520,7 +488,6 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro var preparationClasses = [structuralClassName, addRemoveClassName].join(' ').trim(); var fullClassName = classes + ' ' + preparationClasses; - var activeClasses = pendClasses(preparationClasses, ACTIVE_CLASS_SUFFIX); var hasToStyles = styles.to && Object.keys(styles.to).length > 0; var containsKeyframeAnimation = (options.keyframeStyle || '').length > 0; @@ -533,7 +500,12 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro return closeAndReturnNoopAnimator(); } - var cacheKey, stagger; + var stagger, cacheKey = $$animateCache.cacheKey(node, method, options.addClass, options.removeClass); + if ($$animateCache.containsCachedAnimationWithoutDuration(cacheKey)) { + preparationClasses = null; + return closeAndReturnNoopAnimator(); + } + if (options.stagger > 0) { var staggerVal = parseFloat(options.stagger); stagger = { @@ -543,7 +515,6 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro animationDuration: 0 }; } else { - cacheKey = gcsHashFn(node, fullClassName); stagger = computeCachedCssStaggerStyles(node, preparationClasses, cacheKey, DETECT_STAGGER_CSS_PROPERTIES); } @@ -577,7 +548,7 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro var itemIndex = stagger ? options.staggerIndex >= 0 ? options.staggerIndex - : gcsLookup.count(cacheKey) + : $$animateCache.count(cacheKey) : 0; var isFirst = itemIndex === 0; @@ -592,7 +563,7 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro blockTransitions(node, SAFE_FAST_FORWARD_DURATION_VALUE); } - var timings = computeTimings(node, fullClassName, cacheKey); + var timings = computeTimings(node, fullClassName, cacheKey, !isStructural); var relativeDelay = timings.maxDelay; maxDelay = Math.max(relativeDelay, 0); maxDuration = timings.maxDuration; @@ -630,6 +601,8 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro return closeAndReturnNoopAnimator(); } + var activeClasses = pendClasses(preparationClasses, ACTIVE_CLASS_SUFFIX); + if (options.delay != null) { var delayStyle; if (typeof options.delay !== 'boolean') { @@ -717,10 +690,13 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro animationClosed = true; animationPaused = false; - if (!options.$$skipPreparationClasses) { + if (preparationClasses && !options.$$skipPreparationClasses) { $$jqLite.removeClass(element, preparationClasses); } - $$jqLite.removeClass(element, activeClasses); + + if (activeClasses) { + $$jqLite.removeClass(element, activeClasses); + } blockKeyframeAnimations(node, false); blockTransitions(node, false); @@ -904,9 +880,9 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro if (flags.recalculateTimingStyles) { fullClassName = node.getAttribute('class') + ' ' + preparationClasses; - cacheKey = gcsHashFn(node, fullClassName); + cacheKey = $$animateCache.cacheKey(node, method, options.addClass, options.removeClass); - timings = computeTimings(node, fullClassName, cacheKey); + timings = computeTimings(node, fullClassName, cacheKey, false); relativeDelay = timings.maxDelay; maxDelay = Math.max(relativeDelay, 0); maxDuration = timings.maxDuration; diff --git a/src/ngAnimate/animateQueue.js b/src/ngAnimate/animateQueue.js index 327cf0ad24b7..e305e242e6fe 100644 --- a/src/ngAnimate/animateQueue.js +++ b/src/ngAnimate/animateQueue.js @@ -438,7 +438,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate if (existingAnimation.state === RUNNING_STATE) { normalizeAnimationDetails(element, newAnimation); } else { - applyGeneratedPreparationClasses(element, isStructural ? event : null, options); + applyGeneratedPreparationClasses($$jqLite, element, isStructural ? event : null, options); event = newAnimation.event = existingAnimation.event; options = mergeAnimationDetails(element, existingAnimation, newAnimation); diff --git a/src/ngAnimate/animation.js b/src/ngAnimate/animation.js index bd7748114ac2..227009af3be2 100644 --- a/src/ngAnimate/animation.js +++ b/src/ngAnimate/animation.js @@ -8,6 +8,7 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro var drivers = this.drivers = []; var RUNNER_STORAGE_KEY = '$$animationRunner'; + var PREPARE_CLASSES_KEY = '$$animatePrepareClasses'; function setRunner(element, runner) { element.data(RUNNER_STORAGE_KEY, runner); @@ -21,8 +22,8 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro return element.data(RUNNER_STORAGE_KEY); } - this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$Map', '$$rAFScheduler', - function($$jqLite, $rootScope, $injector, $$AnimateRunner, $$Map, $$rAFScheduler) { + this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$Map', '$$rAFScheduler', '$$animateCache', + function($$jqLite, $rootScope, $injector, $$AnimateRunner, $$Map, $$rAFScheduler, $$animateCache) { var animationQueue = []; var applyAnimationClasses = applyAnimationClassesFactory($$jqLite); @@ -37,6 +38,7 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro var animation = animations[i]; lookup.set(animation.domNode, animations[i] = { domNode: animation.domNode, + element: animation.element, fn: animation.fn, children: [] }); @@ -93,7 +95,7 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro result.push(row); row = []; } - row.push(entry.fn); + row.push(entry); entry.children.forEach(function(childEntry) { nextLevelEntries++; queue.push(childEntry); @@ -111,6 +113,8 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro // TODO(matsko): document the signature in a better way return function(element, event, options) { + var node = getDomNode(element); + options = prepareAnimationOptions(options); var isStructural = ['enter', 'move', 'leave'].indexOf(event) >= 0; @@ -128,8 +132,6 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro return runner; } - setRunner(element, runner); - var classes = mergeClasses(element.attr('class'), mergeClasses(options.addClass, options.removeClass)); var tempClasses = options.tempClasses; if (tempClasses) { @@ -137,12 +139,12 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro options.tempClasses = null; } - var prepareClassName; if (isStructural) { - prepareClassName = 'ng-' + event + PREPARE_CLASS_SUFFIX; - $$jqLite.addClass(element, prepareClassName); + element.data(PREPARE_CLASSES_KEY, 'ng-' + event + PREPARE_CLASS_SUFFIX); } + setRunner(element, runner); + animationQueue.push({ // this data is used by the postDigest code and passed into // the driver step function @@ -182,16 +184,30 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro var toBeSortedAnimations = []; forEach(groupedAnimations, function(animationEntry) { + var element = animationEntry.from ? animationEntry.from.element : animationEntry.element; + var extraClasses = options.addClass; + extraClasses = (extraClasses ? (extraClasses + ' ') : '') + NG_ANIMATE_CLASSNAME; + var cacheKey = $$animateCache.cacheKey(node, event, extraClasses, options.removeClass); + toBeSortedAnimations.push({ - domNode: getDomNode(animationEntry.from ? animationEntry.from.element : animationEntry.element), + element: element, + domNode: getDomNode(element), fn: function triggerAnimationStart() { + var startAnimationFn, closeFn = animationEntry.close; + + // in the event that we've cached the animation status for this element + // and it's in fact an invalid animation (something that has duration = 0) + // then we should skip all the heavy work from here on + if ($$animateCache.containsCachedAnimationWithoutDuration(cacheKey)) { + closeFn(); + return; + } + // it's important that we apply the `ng-animate` CSS class and the // temporary classes before we do any driver invoking since these // CSS classes may be required for proper CSS detection. animationEntry.beforeStart(); - var startAnimationFn, closeFn = animationEntry.close; - // in the event that the element was removed before the digest runs or // during the RAF sequencing then we should not trigger the animation. var targetElement = animationEntry.anchors @@ -221,7 +237,32 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro // we need to sort each of the animations in order of parent to child // relationships. This ensures that the child classes are applied at the // right time. - $$rAFScheduler(sortAnimations(toBeSortedAnimations)); + var finalAnimations = sortAnimations(toBeSortedAnimations); + for (var i = 0; i < finalAnimations.length; i++) { + var innerArray = finalAnimations[i]; + for (var j = 0; j < innerArray.length; j++) { + var entry = innerArray[j]; + var element = entry.element; + + // the RAFScheduler code only uses functions + finalAnimations[i][j] = entry.fn; + + // the first row of elements shouldn't have a prepare-class added to them + // since the elements are at the top of the animation hierarchy and they + // will be applied without a RAF having to pass... + if (i === 0) { + element.removeData(PREPARE_CLASSES_KEY); + continue; + } + + var prepareClassName = element.data(PREPARE_CLASSES_KEY); + if (prepareClassName) { + $$jqLite.addClass(element, prepareClassName); + } + } + } + + $$rAFScheduler(finalAnimations); }); return runner; @@ -359,10 +400,10 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro } function beforeStart() { - element.addClass(NG_ANIMATE_CLASSNAME); - if (tempClasses) { - $$jqLite.addClass(element, tempClasses); - } + tempClasses = (tempClasses ? (tempClasses + ' ') : '') + NG_ANIMATE_CLASSNAME; + $$jqLite.addClass(element, tempClasses); + + var prepareClassName = element.data(PREPARE_CLASSES_KEY); if (prepareClassName) { $$jqLite.removeClass(element, prepareClassName); prepareClassName = null; @@ -402,7 +443,6 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro $$jqLite.removeClass(element, tempClasses); } - element.removeClass(NG_ANIMATE_CLASSNAME); runner.complete(!rejected); } }; diff --git a/src/ngAnimate/module.js b/src/ngAnimate/module.js index 70c9570904c1..1d99af0be0f4 100644 --- a/src/ngAnimate/module.js +++ b/src/ngAnimate/module.js @@ -786,6 +786,7 @@ angular.module('ngAnimate', [], function initAngularHelpers() { .factory('$$rAFScheduler', $$rAFSchedulerFactory) .provider('$$animateQueue', $$AnimateQueueProvider) + .provider('$$animateCache', $$AnimateCacheProvider) .provider('$$animation', $$AnimationProvider) .provider('$animateCss', $AnimateCssProvider) diff --git a/src/ngAnimate/shared.js b/src/ngAnimate/shared.js index f65ec61b0996..2708bbb2e817 100644 --- a/src/ngAnimate/shared.js +++ b/src/ngAnimate/shared.js @@ -301,7 +301,7 @@ function getDomNode(element) { return (element instanceof jqLite) ? element[0] : element; } -function applyGeneratedPreparationClasses(element, event, options) { +function applyGeneratedPreparationClasses($$jqLite, element, event, options) { var classes = ''; if (event) { classes = pendClasses(event, EVENT_CLASS_PREFIX, true); diff --git a/test/ngAnimate/.eslintrc.json b/test/ngAnimate/.eslintrc.json index 060d812cd85b..f51d109b48b1 100644 --- a/test/ngAnimate/.eslintrc.json +++ b/test/ngAnimate/.eslintrc.json @@ -3,6 +3,7 @@ "new-cap": "off" }, "globals": { + "getDomNode": false, "mergeAnimationDetails": false, "prepareAnimationOptions": false, "applyAnimationStyles": false, diff --git a/test/ngAnimate/animateCacheSpec.js b/test/ngAnimate/animateCacheSpec.js new file mode 100644 index 000000000000..b04bac871a08 --- /dev/null +++ b/test/ngAnimate/animateCacheSpec.js @@ -0,0 +1,99 @@ +'use strict'; + +describe('ngAnimate $$animateCache', function() { + beforeEach(module('ngAnimate')); + + it('should store the details in a lookup', inject(function($$animateCache) { + var data = { 'hello': 'there' }; + $$animateCache.put('key', data, true); + expect($$animateCache.get('key')).toBe(data); + })); + + it('should update existing stored details in a lookup', inject(function($$animateCache) { + var data = { 'hello': 'there' }; + $$animateCache.put('key', data, true); + + var otherData = { 'hi': 'you' }; + $$animateCache.put('key', otherData, true); + expect($$animateCache.get('key')).toBe(otherData); + })); + + it('should create a special cacheKey based on the element/parent and className relationship', inject(function($$animateCache) { + var cacheKey, elm = jqLite('
'); + elm.addClass('one two'); + + var parent1 = jqLite('
'); + parent1.append(elm); + + cacheKey = $$animateCache.cacheKey(getDomNode(elm), 'event'); + expect(cacheKey).toBe('1 event one two'); + + cacheKey = $$animateCache.cacheKey(getDomNode(elm), 'event', 'add'); + expect(cacheKey).toBe('1 event one two add'); + + cacheKey = $$animateCache.cacheKey(getDomNode(elm), 'event', 'add', 'remove'); + expect(cacheKey).toBe('1 event one two add remove'); + + var parent2 = jqLite('
'); + parent2.append(elm); + + cacheKey = $$animateCache.cacheKey(getDomNode(elm), 'event'); + expect(cacheKey).toBe('2 event one two'); + + cacheKey = $$animateCache.cacheKey(getDomNode(elm), 'event', 'three', 'four'); + expect(cacheKey).toBe('2 event one two three four'); + })); + + it('should keep a count of how many times a cache key has been updated', inject(function($$animateCache) { + var data = { 'hello': 'there' }; + var key = 'key'; + expect($$animateCache.count(key)).toBe(0); + + $$animateCache.put(key, data, true); + expect($$animateCache.count(key)).toBe(1); + + var otherData = { 'other': 'data' }; + $$animateCache.put(key, otherData, true); + expect($$animateCache.count(key)).toBe(2); + })); + + it('should flush the cache and the counters', inject(function($$animateCache) { + $$animateCache.put('key1', { data: 'value' }, true); + $$animateCache.put('key2', { data: 'value' }, true); + + expect($$animateCache.count('key1')).toBe(1); + expect($$animateCache.count('key2')).toBe(1); + + $$animateCache.flush(); + + expect($$animateCache.get('key1')).toBeFalsy(); + expect($$animateCache.get('key2')).toBeFalsy(); + + expect($$animateCache.count('key1')).toBe(0); + expect($$animateCache.count('key2')).toBe(0); + })); + + describe('containsCachedAnimationWithoutDuration', function() { + it('should return false if the validity of a key is false', inject(function($$animateCache) { + var validEntry = { someEssentialProperty: true }; + var invalidEntry = { someEssentialProperty: false }; + + $$animateCache.put('key1', validEntry, true); + $$animateCache.put('key2', invalidEntry, false); + + expect($$animateCache.containsCachedAnimationWithoutDuration('key1')).toBe(false); + expect($$animateCache.containsCachedAnimationWithoutDuration('key2')).toBe(true); + })); + + it('should return false if the key does not exist in the cache', inject(function($$animateCache) { + expect($$animateCache.containsCachedAnimationWithoutDuration('key2')).toBe(false); + + $$animateCache.put('key2', {}, false); + expect($$animateCache.containsCachedAnimationWithoutDuration('key2')).toBe(true); + + $$animateCache.flush(); + expect($$animateCache.containsCachedAnimationWithoutDuration('key2')).toBe(false); + })); + }); + +}); diff --git a/test/ngAnimate/animationSpec.js b/test/ngAnimate/animationSpec.js index cab771308fd3..470e1e46a7bd 100644 --- a/test/ngAnimate/animationSpec.js +++ b/test/ngAnimate/animationSpec.js @@ -36,6 +36,9 @@ describe('$$animation', function() { }); inject(function($$animation, $animate, $rootScope) { element = jqLite('
'); + var parent = jqLite('
'); + parent.append(element); + var done = false; $$animation(element, 'someEvent').then(function() { done = true; @@ -197,7 +200,11 @@ describe('$$animation', function() { }); inject(function($$animation, $rootScope, $animate) { - var status, element = jqLite('
'); + var status; + var element = jqLite('
'); + var parent = jqLite('
'); + parent.append(element); + var runner = $$animation(element, 'enter'); runner.then(function() { status = 'resolve'; @@ -519,11 +526,24 @@ describe('$$animation', function() { })); - they('should add the preparation class before the $prop-animation is pushed to the queue', + they('should only apply the ng-$prop-prepare class if there are a child animations', ['enter', 'leave', 'move'], function(animationType) { inject(function($$animation, $rootScope, $animate) { - var runner = $$animation(element, animationType); - expect(element).toHaveClass('ng-' + animationType + '-prepare'); + var expectedClassName = 'ng-' + animationType + '-prepare'; + + $$animation(element, animationType); + $rootScope.$digest(); + expect(element).not.toHaveClass(expectedClassName); + + var child = jqLite('
'); + element.append(child); + + $$animation(element, animationType); + $$animation(child, animationType); + $rootScope.$digest(); + + expect(element).not.toHaveClass(expectedClassName); + expect(child).toHaveClass(expectedClassName); }); }); @@ -531,9 +551,22 @@ describe('$$animation', function() { they('should remove the preparation class before the $prop-animation starts', ['enter', 'leave', 'move'], function(animationType) { inject(function($$animation, $rootScope, $$rAF) { - var runner = $$animation(element, animationType); + var expectedClassName = 'ng-' + animationType + '-prepare'; + + var child = jqLite('
'); + element.append(child); + + $$animation(element, animationType); + $$animation(child, animationType); $rootScope.$digest(); - expect(element).not.toHaveClass('ng-' + animationType + '-prepare'); + + expect(element).not.toHaveClass(expectedClassName); + expect(child).toHaveClass(expectedClassName); + + $$rAF.flush(); + + expect(element).not.toHaveClass(expectedClassName); + expect(child).not.toHaveClass(expectedClassName); }); }); }); @@ -982,11 +1015,12 @@ describe('$$animation', function() { }); inject(function($$animation, $rootScope, $animate) { element.addClass('four'); + parent.append(element); var completed = false; $$animation(element, 'event', { - from: { background: 'red' }, - to: { background: 'blue', 'font-size': '50px' } + from: { height: '100px' }, + to: { height: '200px', 'font-size': '50px' } }).then(function() { completed = true; }); @@ -997,7 +1031,7 @@ describe('$$animation', function() { $rootScope.$digest(); //the runner promise expect(completed).toBe(true); - expect(element.css('background')).toContain('blue'); + expect(element.css('height')).toContain('200px'); expect(element.css('font-size')).toBe('50px'); }); }); @@ -1033,6 +1067,7 @@ describe('$$animation', function() { }); }); inject(function($$animation, $rootScope, $animate) { + parent.append(element); element.addClass('four'); var completed = false; diff --git a/test/ngAnimate/integrationSpec.js b/test/ngAnimate/integrationSpec.js index c17b737c6e7f..cc6359f715d7 100644 --- a/test/ngAnimate/integrationSpec.js +++ b/test/ngAnimate/integrationSpec.js @@ -316,6 +316,8 @@ describe('ngAnimate integration tests', function() { it('should issue a RAF for each element animation on all DOM levels', function() { module('ngAnimateMock'); inject(function($animate, $compile, $rootScope, $rootElement, $document, $$rAF) { + ss.addRule('.ng-enter', 'transition:2s linear all;'); + element = jqLite( '
' + '
' + @@ -395,6 +397,76 @@ describe('ngAnimate integration tests', function() { }); }); + it('should avoid adding the ng-enter-prepare method to a parent structural animation that contains child animations', function() { + module('ngAnimateMock'); + inject(function($animate, $compile, $rootScope, $rootElement, $document, $$rAF) { + element = jqLite( + '
' + + '
' + + '
' + + '
' + + '
' + + '
' + + '
' + ); + + ss.addRule('.ng-enter', 'transition:2s linear all;'); + + $rootElement.append(element); + jqLite($document[0].body).append($rootElement); + + $compile(element)($rootScope); + $rootScope.parent = true; + $rootScope.child = true; + $rootScope.$digest(); + + var parent = jqLite(element[0].querySelector('.parent')); + var child = jqLite(element[0].querySelector('.child')); + + expect(parent).not.toHaveClass('ng-enter-prepare'); + expect(child).toHaveClass('ng-enter-prepare'); + + $$rAF.flush(); + + expect(parent).not.toHaveClass('ng-enter-prepare'); + expect(child).not.toHaveClass('ng-enter-prepare'); + }); + }); + + it('should add the preparation class for an enter animation before a parent class-based animation is applied', function() { + module('ngAnimateMock'); + inject(function($animate, $compile, $rootScope, $rootElement, $document) { + element = jqLite( + '
' + + '
' + + '
' + + '
' + ); + + ss.addRule('.ng-enter', 'transition:2s linear all;'); + ss.addRule('.parent-add', 'transition:5s linear all;'); + + $rootElement.append(element); + jqLite($document[0].body).append($rootElement); + + $compile(element)($rootScope); + $rootScope.exp = true; + $rootScope.$digest(); + + var parent = element; + var child = element.find('div'); + + expect(parent).not.toHaveClass('parent'); + expect(parent).toHaveClass('parent-add'); + expect(child).not.toHaveClass('ng-enter'); + expect(child).toHaveClass('ng-enter-prepare'); + + $animate.flush(); + expect(parent).toHaveClass('parent parent-add parent-add-active'); + expect(child).toHaveClass('ng-enter ng-enter-active'); + expect(child).not.toHaveClass('ng-enter-prepare'); + }); + }); it('should pack level elements into their own RAF flush', function() { module('ngAnimateMock'); @@ -544,6 +616,45 @@ describe('ngAnimate integration tests', function() { expect(child).not.toHaveClass('blue'); }); }); + + it('should not apply ngAnimate CSS preparation classes when a css animation definition has duration = 0', function() { + function fill(max) { + var arr = []; + for (var i = 0; i < max; i++) { + arr.push(i); + } + return arr; + } + + inject(function($animate, $rootScope, $compile, $timeout, $$rAF, $$jqLite) { + ss.addRule('.animate-me', 'transition: all 0.5s;'); + + var classAddSpy = spyOn($$jqLite, 'addClass').and.callThrough(); + var classRemoveSpy = spyOn($$jqLite, 'removeClass').and.callThrough(); + + element = jqLite( + '
' + + '
' + + '
' + ); + + html(element); + $compile(element)($rootScope); + + $rootScope.items = fill(100); + $rootScope.$digest(); + + expect(classAddSpy.calls.count()).toBe(2); + expect(classRemoveSpy.calls.count()).toBe(2); + + expect(classAddSpy.calls.argsFor(0)[1]).toBe('ng-animate'); + expect(classAddSpy.calls.argsFor(1)[1]).toBe('ng-enter'); + expect(classRemoveSpy.calls.argsFor(0)[1]).toBe('ng-enter'); + expect(classRemoveSpy.calls.argsFor(1)[1]).toBe('ng-animate'); + + expect(element.children().length).toBe(100); + }); + }); }); describe('JS animations', function() {