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 angular#8234 Closes angular#9263
caitp · Sep 25, 2014 · e0eb8ce · e0eb8ce
1 parent a8fe2cc
commit e0eb8ce
Show file tree

Hide file tree

Showing 3 changed files with 146 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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(
+        '<form name="form">' +
+            '<input type="text" ng-model="value" name="alias" ng-maxlength="10">' +
+        '</form>'
+      )($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);
+    }));
   });
 });