From e0eb8ce8cee5de4b67dfc30d18081307515d99b6 Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Wed, 24 Sep 2014 22:04:06 -0400 Subject: [PATCH] fix(ngAnimate): defer DOM operations for changing classes to postDigest When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. Closes #8234 Closes #9263 --- src/ng/animate.js | 106 +++++++++++++++++++++++++++++++-- src/ngAnimate/animate.js | 7 ++- test/ng/directive/inputSpec.js | 39 ++++++++++++ 3 files changed, 146 insertions(+), 6 deletions(-) diff --git a/src/ng/animate.js b/src/ng/animate.js index ababe39f96e9..a2120c36547f 100644 --- a/src/ng/animate.js +++ b/src/ng/animate.js @@ -81,9 +81,69 @@ var $AnimateProvider = ['$provide', function($provide) { return this.$$classNameFilter; }; - this.$get = ['$$q', '$$asyncCallback', function($$q, $$asyncCallback) { + this.$get = ['$$q', '$$asyncCallback', '$rootScope', function($$q, $$asyncCallback, $rootScope) { var currentDefer; + var ELEMENT_NODE = 1; + + function extractElementNodes(element) { + var elements = new Array(element.length); + var count = 0; + for(var i = 0; i < element.length; i++) { + var elm = element[i]; + if (elm.nodeType == ELEMENT_NODE) { + elements[count++] = elm; + } + } + elements.length = count; + return jqLite(elements); + } + + function runAnimationPostDigest(fn) { + if (!$rootScope.$$phase) { + fn(noop); + return asyncPromise(); + } + var cancelFn, defer = $$q.defer(); + defer.promise.$$cancelFn = function ngAnimateMaybeCancel() { + cancelFn && cancelFn(); + }; + $rootScope.$$postDigest(function ngAnimatePostDigest() { + cancelFn = fn(function ngAnimateNotifyComplete() { + defer.resolve(); + }); + }); + return defer.promise; + } + + function resolveElementClasses(element, cache) { + var map = {}; + + forEach(cache.add, function(className) { + if (className && className.length) { + map[className] = map[className] || 0; + map[className]++; + } + }); + + forEach(cache.remove, function(className) { + if (className && className.length) { + map[className] = map[className] || 0; + map[className]--; + } + }); + + var toAdd = [], toRemove = []; + forEach(map, function(status, className) { + var hasClass = jqLiteHasClass(element[0], className); + + if (status < 0 && hasClass) toRemove.push(className); + else if (status > 0 && !hasClass) toAdd.push(className); + }); + + return (toAdd.length + toRemove.length) > 0 && [toAdd.join(' '), toRemove.join(' ')]; + } + function asyncPromise() { // only serve one instance of a promise in order to save CPU cycles if (!currentDefer) { @@ -187,6 +247,10 @@ var $AnimateProvider = ['$provide', function($provide) { * @return {Promise} the animation callback promise */ addClass : function(element, className) { + return this.setClass(element, className, []); + }, + + $$addClass : function(element, className) { className = !isString(className) ? (isArray(className) ? className.join(' ') : '') : className; @@ -209,6 +273,10 @@ var $AnimateProvider = ['$provide', function($provide) { * @return {Promise} the animation callback promise */ removeClass : function(element, className) { + return this.setClass(element, [], className); + }, + + $$removeClass : function(element, className) { className = !isString(className) ? (isArray(className) ? className.join(' ') : '') : className; @@ -232,9 +300,39 @@ var $AnimateProvider = ['$provide', function($provide) { * @return {Promise} the animation callback promise */ setClass : function(element, add, remove) { - this.addClass(element, add); - this.removeClass(element, remove); - return asyncPromise(); + var self = this; + var STORAGE_KEY = '$$animateClasses'; + element = extractElementNodes(jqLite(element)); + + add = isArray(add) ? add : add.split(' '); + remove = isArray(remove) ? remove : remove.split(' '); + + var cache = element.data(STORAGE_KEY); + if (cache) { + cache.add = cache.add.concat(add); + cache.remove = cache.remove.concat(remove); + //the digest cycle will combine all the animations into one function + return cache.promise; + } else { + element.data(STORAGE_KEY, cache = { + add : add, + remove : remove + }); + } + + return cache.promise = runAnimationPostDigest(function(done) { + var cache = element.data(STORAGE_KEY); + element.removeData(STORAGE_KEY); + + var classes = cache && resolveElementClasses(element, cache); + + if (classes) { + self.$$addClass(element, classes[0]); + self.$$removeClass(element, classes[1]); + } + + done(); + }); }, enabled : noop, diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index e66fcd6e1cfc..99628ada05c1 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -994,7 +994,9 @@ angular.module('ngAnimate', ['ng']) element = stripCommentsFromElement(element); if (classBasedAnimationsBlocked(element)) { - return $delegate.setClass(element, add, remove); + $delegate.$$addClass(element, add); + $delegate.$$removeClass(element, remove); + return; } add = isArray(add) ? add : add.split(' '); @@ -1023,7 +1025,8 @@ angular.module('ngAnimate', ['ng']) return !classes ? done() : performAnimation('setClass', classes, element, null, null, function() { - $delegate.setClass(element, classes[0], classes[1]); + $delegate.$$addClass(element, classes[0]); + $delegate.$$removeClass(element, classes[1]); }, done); }); }, diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index a10676732a14..62df189153d5 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -892,6 +892,45 @@ describe('NgModelController', function() { dealoc(element); })); + + it('should minimize janky setting of classes during $validate() and ngModelWatch', inject(function($animate, $compile, $rootScope) { + var addClass = $animate.$$addClass; + var removeClass = $animate.$$removeClass; + var addClassCallCount = 0; + var removeClassCallCount = 0; + var input; + $animate.$$addClass = function(element, className) { + if (input && element[0] === input[0]) ++addClassCallCount; + return addClass.call($animate, element, className); + }; + + $animate.$$removeClass = function(element, className) { + if (input && element[0] === input[0]) ++removeClassCallCount; + return removeClass.call($animate, element, className); + }; + + dealoc(element); + + $rootScope.value = "123456789"; + element = $compile( + '
' + + '' + + '
' + )($rootScope); + + var form = $rootScope.form; + input = element.children().eq(0); + + $rootScope.$digest(); + + expect(input).toBeValid(); + expect(input).not.toHaveClass('ng-invalid-maxlength'); + expect(input).toHaveClass('ng-valid-maxlength'); + expect(addClassCallCount).toBe(1); + expect(removeClassCallCount).toBe(1); + + dealoc(element); + })); }); });