Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 8932480

Browse files
caitpcaitp
committed
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
1 parent dbae5f9 commit 8932480

File tree

4 files changed

+260
-6
lines changed

4 files changed

+260
-6
lines changed

‎src/ng/animate.js

+102-4
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,69 @@ var $AnimateProvider = ['$provide', function($provide) {
8181
return this.$$classNameFilter;
8282
};
8383

84-
this.$get = ['$$q', '$$asyncCallback', function($$q, $$asyncCallback) {
84+
this.$get = ['$$q', '$$asyncCallback', '$rootScope', function($$q, $$asyncCallback, $rootScope) {
8585

8686
var currentDefer;
87+
var ELEMENT_NODE = 1;
88+
89+
function extractElementNodes(element) {
90+
var elements = new Array(element.length);
91+
var count = 0;
92+
for(var i = 0; i < element.length; i++) {
93+
var elm = element[i];
94+
if (elm.nodeType == ELEMENT_NODE) {
95+
elements[count++] = elm;
96+
}
97+
}
98+
elements.length = count;
99+
return jqLite(elements);
100+
}
101+
102+
function runAnimationPostDigest(fn) {
103+
if (!$rootScope.$$phase) {
104+
fn(noop);
105+
return asyncPromise();
106+
}
107+
var cancelFn, defer = $$q.defer();
108+
defer.promise.$$cancelFn = function ngAnimateMaybeCancel() {
109+
cancelFn && cancelFn();
110+
};
111+
$rootScope.$$postDigest(function ngAnimatePostDigest() {
112+
cancelFn = fn(function ngAnimateNotifyComplete() {
113+
defer.resolve();
114+
});
115+
});
116+
return defer.promise;
117+
}
118+
119+
function resolveElementClasses(element, cache) {
120+
var map = {};
121+
122+
forEach(cache.add, function(className) {
123+
if (className && className.length) {
124+
map[className] = map[className] || 0;
125+
map[className]++;
126+
}
127+
});
128+
129+
forEach(cache.remove, function(className) {
130+
if (className && className.length) {
131+
map[className] = map[className] || 0;
132+
map[className]--;
133+
}
134+
});
135+
136+
var toAdd = [], toRemove = [];
137+
forEach(map, function(status, className) {
138+
var hasClass = jqLiteHasClass(element[0], className);
139+
140+
if (status < 0 && hasClass) toRemove.push(className);
141+
else if (status > 0 && !hasClass) toAdd.push(className);
142+
});
143+
144+
return (toAdd.length + toRemove.length) > 0 && [toAdd.join(' '), toRemove.join(' ')];
145+
}
146+
87147
function asyncPromise() {
88148
// only serve one instance of a promise in order to save CPU cycles
89149
if (!currentDefer) {
@@ -187,6 +247,10 @@ var $AnimateProvider = ['$provide', function($provide) {
187247
* @return {Promise} the animation callback promise
188248
*/
189249
addClass : function(element, className) {
250+
return this.setClass(element, className, []);
251+
},
252+
253+
$$addClass : function(element, className) {
190254
className = !isString(className)
191255
? (isArray(className) ? className.join(' ') : '')
192256
: className;
@@ -209,6 +273,10 @@ var $AnimateProvider = ['$provide', function($provide) {
209273
* @return {Promise} the animation callback promise
210274
*/
211275
removeClass : function(element, className) {
276+
return this.setClass(element, [], className);
277+
},
278+
279+
$$removeClass : function(element, className) {
212280
className = !isString(className)
213281
? (isArray(className) ? className.join(' ') : '')
214282
: className;
@@ -232,9 +300,39 @@ var $AnimateProvider = ['$provide', function($provide) {
232300
* @return {Promise} the animation callback promise
233301
*/
234302
setClass : function(element, add, remove) {
235-
this.addClass(element, add);
236-
this.removeClass(element, remove);
237-
return asyncPromise();
303+
var self = this;
304+
var STORAGE_KEY = '$$animateClasses';
305+
element = extractElementNodes(jqLite(element));
306+
307+
add = isArray(add) ? add : add.split(' ');
308+
remove = isArray(remove) ? remove : remove.split(' ');
309+
310+
var cache = element.data(STORAGE_KEY);
311+
if (cache) {
312+
cache.add = cache.add.concat(add);
313+
cache.remove = cache.remove.concat(remove);
314+
//the digest cycle will combine all the animations into one function
315+
return cache.promise;
316+
} else {
317+
element.data(STORAGE_KEY, cache = {
318+
add : add,
319+
remove : remove
320+
});
321+
}
322+
323+
return cache.promise = runAnimationPostDigest(function(done) {
324+
var cache = element.data(STORAGE_KEY);
325+
element.removeData(STORAGE_KEY);
326+
327+
var classes = cache && resolveElementClasses(element, cache);
328+
329+
if (classes) {
330+
if (classes[0].length) self.$$addClass(element, classes[0]);
331+
if (classes[1].length) self.$$removeClass(element, classes[1]);
332+
}
333+
334+
done();
335+
});
238336
},
239337

240338
enabled : noop,

‎src/ngAnimate/animate.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -994,7 +994,9 @@ angular.module('ngAnimate', ['ng'])
994994
element = stripCommentsFromElement(element);
995995

996996
if (classBasedAnimationsBlocked(element)) {
997-
return $delegate.setClass(element, add, remove);
997+
$delegate.$$addClass(element, add);
998+
$delegate.$$removeClass(element, remove);
999+
return;
9981000
}
9991001

10001002
add = isArray(add) ? add : add.split(' ');
@@ -1023,7 +1025,8 @@ angular.module('ngAnimate', ['ng'])
10231025
return !classes
10241026
? done()
10251027
: performAnimation('setClass', classes, element, null, null, function() {
1026-
$delegate.setClass(element, classes[0], classes[1]);
1028+
$delegate.$$addClass(element, classes[0]);
1029+
$delegate.$$removeClass(element, classes[1]);
10271030
}, done);
10281031
});
10291032
},

‎test/ng/animateSpec.js

+114
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,118 @@ describe("$animate", function() {
100100
inject();
101101
});
102102
});
103+
104+
describe('class API', function() {
105+
var element;
106+
var addClass;
107+
var removeClass;
108+
109+
beforeEach(inject(function($animate) {
110+
addClass = spyOn($animate, '$$addClass').andCallThrough();
111+
removeClass = spyOn($animate, '$$removeClass').andCallThrough();
112+
}));
113+
114+
afterEach(function() {
115+
dealoc(element);
116+
});
117+
118+
it('should defer class manipulation until end of digest', inject(function($rootScope, $animate) {
119+
element = jqLite('<p>test</p>');
120+
121+
$rootScope.$apply(function() {
122+
$animate.addClass(element, 'test-class1');
123+
expect(element).not.toHaveClass('test-class1');
124+
125+
$animate.removeClass(element, 'test-class1');
126+
127+
$animate.addClass(element, 'test-class2');
128+
expect(element).not.toHaveClass('test-class2');
129+
130+
$animate.setClass(element, 'test-class3', 'test-class4');
131+
expect(element).not.toHaveClass('test-class3');
132+
expect(element).not.toHaveClass('test-class4');
133+
});
134+
135+
expect(element).not.toHaveClass('test-class1');
136+
expect(element).not.toHaveClass('test-class4');
137+
expect(element).toHaveClass('test-class2');
138+
expect(element).toHaveClass('test-class3');
139+
expect(addClass.callCount).toBe(1);
140+
expect(removeClass.callCount).toBe(0);
141+
}));
142+
143+
144+
it('should perform class manipulation immediately outside of digest', inject(function($rootScope, $animate) {
145+
element = jqLite('<p>test</p>');
146+
147+
$animate.addClass(element, 'test-class1');
148+
expect(element).toHaveClass('test-class1');
149+
150+
$animate.removeClass(element, 'test-class1');
151+
expect(element).not.toHaveClass('test-class1');
152+
153+
$animate.addClass(element, 'test-class2');
154+
expect(element).toHaveClass('test-class2');
155+
156+
$animate.setClass(element, 'test-class3', 'test-class4');
157+
expect(element).toHaveClass('test-class3');
158+
expect(element).not.toHaveClass('test-class4');
159+
160+
expect(element).not.toHaveClass('test-class1');
161+
expect(element).toHaveClass('test-class2');
162+
expect(addClass.callCount).toBe(3);
163+
expect(removeClass.callCount).toBe(1);
164+
}));
165+
166+
167+
it('should defer class manipulation until end of digest for SVG', inject(function($rootScope, $animate) {
168+
if (!window.SVGElement) return;
169+
element = jqLite('<svg><g></g></svg>');
170+
var target = element.children().eq(0);
171+
172+
$rootScope.$apply(function() {
173+
$animate.addClass(target, 'test-class1');
174+
expect(target).not.toHaveClass('test-class1');
175+
176+
$animate.removeClass(target, 'test-class1');
177+
178+
$animate.addClass(target, 'test-class2');
179+
expect(target).not.toHaveClass('test-class2');
180+
181+
$animate.setClass(target, 'test-class3', 'test-class4');
182+
expect(target).not.toHaveClass('test-class3');
183+
expect(target).not.toHaveClass('test-class4');
184+
});
185+
186+
expect(target).not.toHaveClass('test-class1');
187+
expect(target).toHaveClass('test-class2');
188+
expect(addClass.callCount).toBe(1);
189+
expect(removeClass.callCount).toBe(0);
190+
}));
191+
192+
193+
it('should perform class manipulation immediately outside of digest for SVG', inject(function($rootScope, $animate) {
194+
if (!window.SVGElement) return;
195+
element = jqLite('<svg><g></g></svg>');
196+
var target = element.children().eq(0);
197+
198+
$animate.addClass(target, 'test-class1');
199+
expect(target).toHaveClass('test-class1');
200+
201+
$animate.removeClass(target, 'test-class1');
202+
expect(target).not.toHaveClass('test-class1');
203+
204+
$animate.addClass(target, 'test-class2');
205+
expect(target).toHaveClass('test-class2');
206+
207+
$animate.setClass(target, 'test-class3', 'test-class4');
208+
expect(target).toHaveClass('test-class3');
209+
expect(target).not.toHaveClass('test-class4');
210+
211+
expect(target).not.toHaveClass('test-class1');
212+
expect(target).toHaveClass('test-class2');
213+
expect(addClass.callCount).toBe(3);
214+
expect(removeClass.callCount).toBe(1);
215+
}));
216+
});
103217
});

‎test/ng/directive/inputSpec.js

+39
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,45 @@ describe('NgModelController', function() {
892892
dealoc(element);
893893
}));
894894

895+
896+
it('should minimize janky setting of classes during $validate() and ngModelWatch', inject(function($animate, $compile, $rootScope) {
897+
var addClass = $animate.$$addClass;
898+
var removeClass = $animate.$$removeClass;
899+
var addClassCallCount = 0;
900+
var removeClassCallCount = 0;
901+
var input;
902+
$animate.$$addClass = function(element, className) {
903+
if (input && element[0] === input[0]) ++addClassCallCount;
904+
return addClass.call($animate, element, className);
905+
};
906+
907+
$animate.$$removeClass = function(element, className) {
908+
if (input && element[0] === input[0]) ++removeClassCallCount;
909+
return removeClass.call($animate, element, className);
910+
};
911+
912+
dealoc(element);
913+
914+
$rootScope.value = "123456789";
915+
element = $compile(
916+
'<form name="form">' +
917+
'<input type="text" ng-model="value" name="alias" ng-maxlength="10">' +
918+
'</form>'
919+
)($rootScope);
920+
921+
var form = $rootScope.form;
922+
input = element.children().eq(0);
923+
924+
$rootScope.$digest();
925+
926+
expect(input).toBeValid();
927+
expect(input).not.toHaveClass('ng-invalid-maxlength');
928+
expect(input).toHaveClass('ng-valid-maxlength');
929+
expect(addClassCallCount).toBe(1);
930+
expect(removeClassCallCount).toBe(0);
931+
932+
dealoc(element);
933+
}));
895934
});
896935
});
897936

0 commit comments

Comments
 (0)