From cac4ca8f27f7d47dafaf68056143eecc5b27c2de Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Wed, 24 Sep 2014 22:04:06 -0400 Subject: [PATCH] fix(ngModel): minimize jank when toggling CSS classes during validation Previously, class toggling would always occur immediately. This causes problems in cases where class changes happen super frequently, and can result in flickering in some browsers which do not handle this jank well. Closes #8234 --- src/ng/directive/input.js | 49 +++++++++++++++++++++++++++++++++- test/ng/directive/inputSpec.js | 45 +++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index b8ceea7e546a..bd502076f561 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -2056,6 +2056,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ var viewValue = ctrl.$$lastCommittedViewValue; var modelValue = viewValue; var parserValid = isUndefined(modelValue) ? undefined : true; + var flushPendingClassChanges = schedulePendingClassChanges($scope, ctrl, $element, $animate); if (parserValid) { for(var i = 0; i < ctrl.$parsers.length; i++) { @@ -2092,6 +2093,8 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ ctrl.$$writeModelToScope(); } } + + flushPendingClassChanges(); }; this.$$writeModelToScope = function() { @@ -2197,7 +2200,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ // TODO(perf): why not move this to the action fn? if (modelValue !== ctrl.$modelValue) { ctrl.$modelValue = modelValue; - + var flushPendingClassChanges = schedulePendingClassChanges($scope, ctrl, $element, $animate); var formatters = ctrl.$formatters, idx = formatters.length; @@ -2211,6 +2214,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ ctrl.$$runValidators(undefined, modelValue, viewValue, noop); } + flushPendingClassChanges(); } return modelValue; @@ -2218,6 +2222,37 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ }]; +function schedulePendingClassChanges(scope, ctrl, element, animate) { + if (!ctrl.$$pendingClassChanges) { + ctrl.$$pendingClassChanges = {}; + + if (scope.$$phase || scope.$root.$$phase) { + scope.$$postDigest(flushPendingClassChangesImmediately); + } else { + return flushPendingClassChangesImmediately; + } + } + return noop; + + function flushPendingClassChangesImmediately() { + flushPendingClassChanges(animate, element, ctrl.$$pendingClassChanges); + ctrl.$$pendingClassChanges = null; + } +} + + +function flushPendingClassChanges($animate, element, pendingChanges) { + var keys = Object.keys(pendingChanges); + + for (var i=0, ii = keys.length; i < ii; ++i) { + var key = keys[i]; + var value = pendingChanges[key]; + if (value < 0) $animate.removeClass(element, key); + else if (value > 0) $animate.addClass(element, key); + } +} + + /** * @ngdoc directive * @name ngModel @@ -3037,6 +3072,18 @@ function addSetValidityMethod(context) { } function cachedToggleClass(className, switchValue) { + var pendingChanges = ctrl.$$pendingClassChanges; + if (pendingChanges) { + if (switchValue) { + pendingChanges[className] = 1; + classCache[className] = true; + } else { + pendingChanges[className] = -1; + classCache[className] = false; + } + return; + } + if (switchValue && !classCache[className]) { $animate.addClass($element, className); classCache[className] = true; diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index 88708b6a6296..3677a898431f 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -892,6 +892,51 @@ 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) { + // Don't worry about classes that the input already has. + if (input && element[0] === input[0] && (' ' + element.attr('class') + ' ').indexOf(' ' + className + ' ') < 0) { + ++addClassCallCount; + } + return addClass.call($animate, element, className); + }; + + $animate.removeClass = function(element, className) { + // Don't worry about classes that the input doesn't have. + if (input && element[0] === input[0] && (' ' + element.attr('class') + ' ').indexOf(' ' + className + ' ') !== -1) { + ++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(2); + expect(removeClassCallCount).toBe(0); + + dealoc(element); + })); }); });