Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 7c73898

Browse files
caitpcaitp
committedOct 8, 2014
WIP: Fixuppin
Always defer class manipulation DOM writes until the end of digest on the root scope, even when animations are disabled. BREAKING CHANGE: The $animate class API will always defer changes until the end of the next digest. This allows ngAnimate to coalesce class changes which occur over a short period of time into 1 or 2 DOM writes, rather than many. This prevents jank in browsers such as IE, and is generally a good thing. If you're finding that your classes are not being immediately applied, be sure to invoke $digest().
1 parent c5a060f commit 7c73898

File tree

5 files changed

+198
-135
lines changed

5 files changed

+198
-135
lines changed
 

‎src/ng/animate.js

+5-21
Original file line numberDiff line numberDiff line change
@@ -86,33 +86,18 @@ var $AnimateProvider = ['$provide', function($provide) {
8686
var currentDefer;
8787
var ELEMENT_NODE = 1;
8888

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-
10289
function runAnimationPostDigest(fn) {
10390
var cancelFn, defer = $$q.defer();
104-
if (!$rootScope.$$phase) {
105-
fn(noop);
106-
defer.resolve();
107-
}
10891
defer.promise.$$cancelFn = function ngAnimateMaybeCancel() {
10992
cancelFn && cancelFn();
11093
};
94+
11195
$rootScope.$$postDigest(function ngAnimatePostDigest() {
11296
cancelFn = fn(function ngAnimateNotifyComplete() {
11397
defer.resolve();
11498
});
11599
});
100+
116101
return defer.promise;
117102
}
118103

@@ -258,7 +243,6 @@ var $AnimateProvider = ['$provide', function($provide) {
258243
forEach(element, function (element) {
259244
jqLiteAddClass(element, className);
260245
});
261-
return asyncPromise();
262246
},
263247

264248
/**
@@ -304,7 +288,7 @@ var $AnimateProvider = ['$provide', function($provide) {
304288
setClass : function(element, add, remove) {
305289
var self = this;
306290
var STORAGE_KEY = '$$animateClasses';
307-
element = extractElementNodes(jqLite(element));
291+
element = jqLite(element);
308292

309293
add = isArray(add) ? add : add.split(' ');
310294
remove = isArray(remove) ? remove : remove.split(' ');
@@ -329,8 +313,8 @@ var $AnimateProvider = ['$provide', function($provide) {
329313
var classes = cache && resolveElementClasses(element, cache);
330314

331315
if (classes) {
332-
if (classes[0].length) self.$$addClassImmediately(element, classes[0]);
333-
if (classes[1].length) self.$$removeClassImmediately(element, classes[1]);
316+
if (classes[0]) self.$$addClassImmediately(element, classes[0]);
317+
if (classes[1]) self.$$removeClassImmediately(element, classes[1]);
334318
}
335319

336320
done();

‎src/ngAnimate/animate.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -979,8 +979,8 @@ angular.module('ngAnimate', ['ng'])
979979
element = stripCommentsFromElement(element);
980980

981981
if (classBasedAnimationsBlocked(element)) {
982-
if (add) $delegate.$$addClassImmediately(element, add);
983-
if (remove) $delegate.$$removeClassImmediately(element, remove);
982+
$delegate.$$addClassImmediately(element, add);
983+
$delegate.$$removeClassImmediately(element, remove);
984984
return;
985985
}
986986

‎test/ng/animateSpec.js

+23-111
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,11 @@ describe("$animate", function() {
5050
expect(element.text()).toBe('21');
5151
}));
5252

53-
it("should still perform DOM operations even if animations are disabled", inject(function($animate) {
53+
it("should still perform DOM operations even if animations are disabled (post-digest)", inject(function($animate, $rootScope) {
5454
$animate.enabled(false);
5555
expect(element).toBeShown();
5656
$animate.addClass(element, 'ng-hide');
57+
$rootScope.$digest();
5758
expect(element).toBeHidden();
5859
}));
5960

@@ -79,15 +80,17 @@ describe("$animate", function() {
7980
expect($animate.cancel()).toBeUndefined();
8081
}));
8182

82-
it("should add and remove classes on SVG elements", inject(function($animate) {
83+
it("should add and remove classes on SVG elements", inject(function($animate, $rootScope) {
8384
if (!window.SVGElement) return;
8485
var svg = jqLite('<svg><rect></rect></svg>');
8586
var rect = svg.children();
8687
$animate.enabled(false);
8788
expect(rect).toBeShown();
8889
$animate.addClass(rect, 'ng-hide');
90+
$rootScope.$digest();
8991
expect(rect).toBeHidden();
9092
$animate.removeClass(rect, 'ng-hide');
93+
$rootScope.$digest();
9194
expect(rect).not.toBeHidden();
9295
}));
9396

@@ -101,7 +104,7 @@ describe("$animate", function() {
101104
});
102105
});
103106

104-
describe('class API', function() {
107+
describe('CSS class DOM manipulation', function() {
105108
var element;
106109
var addClass;
107110
var removeClass;
@@ -138,8 +141,9 @@ describe("$animate", function() {
138141
});
139142
}
140143

141-
it('should defer class manipulation until end of digest', inject(function($rootScope, $animate) {
142-
setupClassManipulationSpies();
144+
145+
it('should defer class manipulation until end of digest', inject(function($rootScope, $animate, log) {
146+
setupClassManipulationLogger(log);
143147
element = jqLite('<p>test</p>');
144148

145149
$rootScope.$apply(function() {
@@ -154,96 +158,28 @@ describe("$animate", function() {
154158
$animate.setClass(element, 'test-class3', 'test-class4');
155159
expect(element).not.toHaveClass('test-class3');
156160
expect(element).not.toHaveClass('test-class4');
161+
expect(log).toEqual([]);
157162
});
158163

159164
expect(element).not.toHaveClass('test-class1');
160165
expect(element).not.toHaveClass('test-class4');
161166
expect(element).toHaveClass('test-class2');
162167
expect(element).toHaveClass('test-class3');
168+
expect(log).toEqual(['addClass(test-class2 test-class3)']);
163169
expect(addClass.callCount).toBe(1);
164170
expect(removeClass.callCount).toBe(0);
165171
}));
166172

167173

168-
it('should perform class manipulation immediately outside of digest', inject(function($rootScope, $animate) {
169-
setupClassManipulationSpies();
170-
element = jqLite('<p>test</p>');
171-
172-
$animate.addClass(element, 'test-class1');
173-
expect(element).toHaveClass('test-class1');
174-
175-
$animate.removeClass(element, 'test-class1');
176-
expect(element).not.toHaveClass('test-class1');
177-
178-
$animate.addClass(element, 'test-class2');
179-
expect(element).toHaveClass('test-class2');
180-
181-
$animate.setClass(element, 'test-class3', 'test-class4');
182-
expect(element).toHaveClass('test-class3');
183-
expect(element).not.toHaveClass('test-class4');
184-
185-
expect(element).not.toHaveClass('test-class1');
186-
expect(element).toHaveClass('test-class2');
187-
expect(addClass.callCount).toBe(3);
188-
expect(removeClass.callCount).toBe(1);
189-
}));
190-
191-
192-
it('should perform class manipulation in expected order at end of digest', inject(function($rootScope, $animate, log) {
193-
element = jqLite('<p class="test-class3">test</p>');
194-
195-
setupClassManipulationLogger(log);
196-
197-
$rootScope.$apply(function() {
198-
$animate.addClass(element, 'test-class1');
199-
$animate.addClass(element, 'test-class2');
200-
$animate.removeClass(element, 'test-class1');
201-
$animate.removeClass(element, 'test-class3');
202-
$animate.addClass(element, 'test-class3');
203-
});
204-
expect(log).toEqual(['addClass(test-class2)']);
205-
}));
206-
207-
208-
it('should perform class manipulation in expected order outside of digest', inject(function($rootScope, $animate, log) {
209-
element = jqLite('<p class="test-class3">test</p>');
210-
211-
setupClassManipulationLogger(log);
212-
213-
$animate.addClass(element, 'test-class1');
214-
$animate.addClass(element, 'test-class2');
215-
$animate.removeClass(element, 'test-class1');
216-
$animate.removeClass(element, 'test-class3');
217-
$animate.addClass(element, 'test-class3');
218-
219-
expect(log).toEqual([
220-
'addClass(test-class1)',
221-
'addClass(test-class2)',
222-
'removeClass(test-class1)',
223-
'removeClass(test-class3)',
224-
'addClass(test-class3)']);
225-
}));
226-
227-
228-
it('should return a promise which is resolved on a different turn', inject(function(log, $animate, $browser, $rootScope) {
174+
it('should return a promise which is resolved on a different turn digest', inject(function(log, $animate, $browser, $rootScope) {
229175
element = jqLite('<p class="test2">test</p>');
230176

231177
$animate.addClass(element, 'test1').then(log.fn('addClass(test1)'));
232178
$animate.removeClass(element, 'test2').then(log.fn('removeClass(test2)'));
233179

180+
$rootScope.$digest();
234181
$browser.defer.flush();
235182
expect(log).toEqual(['addClass(test1)', 'removeClass(test2)']);
236-
237-
log.reset();
238-
element = jqLite('<p class="test4">test</p>');
239-
240-
$rootScope.$apply(function() {
241-
$animate.addClass(element, 'test3').then(log.fn('addClass(test3)'));
242-
$animate.removeClass(element, 'test4').then(log.fn('removeClass(test4)'));
243-
});
244-
245-
$browser.defer.flush();
246-
expect(log).toEqual(['addClass(test3)', 'removeClass(test4)']);
247183
}));
248184

249185

@@ -274,29 +210,27 @@ describe("$animate", function() {
274210
}));
275211

276212

277-
it('should perform class manipulation immediately outside of digest for SVG', inject(function($rootScope, $animate) {
213+
it('should defer class manipulation until digest outside of digest for SVG', inject(function($rootScope, $animate, log) {
278214
if (!window.SVGElement) return;
279-
setupClassManipulationSpies();
215+
setupClassManipulationLogger(log);
280216
element = jqLite('<svg><g></g></svg>');
281217
var target = element.children().eq(0);
282218

283219
$animate.addClass(target, 'test-class1');
284-
expect(target).toHaveClass('test-class1');
285-
286220
$animate.removeClass(target, 'test-class1');
287-
expect(target).not.toHaveClass('test-class1');
288-
289221
$animate.addClass(target, 'test-class2');
290-
expect(target).toHaveClass('test-class2');
291-
292222
$animate.setClass(target, 'test-class3', 'test-class4');
293-
expect(target).toHaveClass('test-class3');
294-
expect(target).not.toHaveClass('test-class4');
295223

224+
expect(log).toEqual([]);
225+
226+
$rootScope.$digest();
227+
228+
expect(log).toEqual(['addClass(test-class2 test-class3)']);
296229
expect(target).not.toHaveClass('test-class1');
297230
expect(target).toHaveClass('test-class2');
298-
expect(addClass.callCount).toBe(3);
299-
expect(removeClass.callCount).toBe(1);
231+
expect(target).toHaveClass('test-class3');
232+
expect(addClass.callCount).toBe(1);
233+
expect(removeClass.callCount).toBe(0);
300234
}));
301235

302236

@@ -316,27 +250,5 @@ describe("$animate", function() {
316250
});
317251
expect(log).toEqual(['addClass(test-class2)']);
318252
}));
319-
320-
321-
it('should perform class manipulation in expected order outside of digest for SVG', inject(function($rootScope, $animate, log) {
322-
if (!window.SVGElement) return;
323-
element = jqLite('<svg><g class="test-class3"></g></svg>');
324-
var target = element.children().eq(0);
325-
326-
setupClassManipulationLogger(log);
327-
328-
$animate.addClass(target, 'test-class1');
329-
$animate.addClass(target, 'test-class2');
330-
$animate.removeClass(target, 'test-class1');
331-
$animate.removeClass(target, 'test-class3');
332-
$animate.addClass(target, 'test-class3');
333-
334-
expect(log).toEqual([
335-
'addClass(test-class1)',
336-
'addClass(test-class2)',
337-
'removeClass(test-class1)',
338-
'removeClass(test-class3)',
339-
'addClass(test-class3)']);
340-
}));
341253
});
342254
});

‎test/ng/directive/formSpec.js

+9
Original file line numberDiff line numberDiff line change
@@ -554,24 +554,28 @@ describe('form', function() {
554554
expect(doc).toBeValid();
555555

556556
control.$setValidity('error', false);
557+
scope.$digest();
557558
expect(doc).toBeInvalid();
558559
expect(doc.hasClass('ng-valid-error')).toBe(false);
559560
expect(doc.hasClass('ng-invalid-error')).toBe(true);
560561

561562
control.$setValidity('another', false);
563+
scope.$digest();
562564
expect(doc.hasClass('ng-valid-error')).toBe(false);
563565
expect(doc.hasClass('ng-invalid-error')).toBe(true);
564566
expect(doc.hasClass('ng-valid-another')).toBe(false);
565567
expect(doc.hasClass('ng-invalid-another')).toBe(true);
566568

567569
control.$setValidity('error', true);
570+
scope.$digest();
568571
expect(doc).toBeInvalid();
569572
expect(doc.hasClass('ng-valid-error')).toBe(true);
570573
expect(doc.hasClass('ng-invalid-error')).toBe(false);
571574
expect(doc.hasClass('ng-valid-another')).toBe(false);
572575
expect(doc.hasClass('ng-invalid-another')).toBe(true);
573576

574577
control.$setValidity('another', true);
578+
scope.$digest();
575579
expect(doc).toBeValid();
576580
expect(doc.hasClass('ng-valid-error')).toBe(true);
577581
expect(doc.hasClass('ng-invalid-error')).toBe(false);
@@ -581,6 +585,7 @@ describe('form', function() {
581585
// validators are skipped, e.g. becuase of a parser error
582586
control.$setValidity('error', null);
583587
control.$setValidity('another', null);
588+
scope.$digest();
584589
expect(doc.hasClass('ng-valid-error')).toBe(false);
585590
expect(doc.hasClass('ng-invalid-error')).toBe(false);
586591
expect(doc.hasClass('ng-valid-another')).toBe(false);
@@ -652,7 +657,9 @@ describe('form', function() {
652657
expect(input1).toBeDirty();
653658
expect(input2).toBeDirty();
654659

660+
655661
formCtrl.$setPristine();
662+
scope.$digest();
656663
expect(form).toBePristine();
657664
expect(formCtrl.$pristine).toBe(true);
658665
expect(formCtrl.$dirty).toBe(false);
@@ -685,6 +692,7 @@ describe('form', function() {
685692
expect(input).toBeDirty();
686693

687694
formCtrl.$setPristine();
695+
scope.$digest();
688696
expect(form).toBePristine();
689697
expect(formCtrl.$pristine).toBe(true);
690698
expect(formCtrl.$dirty).toBe(false);
@@ -719,6 +727,7 @@ describe('form', function() {
719727
expect(nestedInput).toBeDirty();
720728

721729
formCtrl.$setPristine();
730+
scope.$digest();
722731
expect(form).toBePristine();
723732
expect(formCtrl.$pristine).toBe(true);
724733
expect(formCtrl.$dirty).toBe(false);

‎test/ngAnimate/animateSpec.js

+159-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
'use strict';
22

33
describe("ngAnimate", function() {
4-
4+
var $animateCore;
5+
beforeEach(module(function($provide) {
6+
$provide.decorator('$animate', function($delegate) {
7+
$animateCore = $delegate;
8+
return $delegate;
9+
});
10+
}));
511
beforeEach(module('ngAnimate'));
612
beforeEach(module('ngAnimateMock'));
713

@@ -4871,4 +4877,156 @@ describe("ngAnimate", function() {
48714877
}));
48724878
});
48734879
});
4880+
4881+
4882+
describe('CSS class DOM manipulation', function() {
4883+
var element;
4884+
var addClass;
4885+
var removeClass;
4886+
4887+
beforeEach(module(provideLog));
4888+
4889+
afterEach(function() {
4890+
dealoc(element);
4891+
});
4892+
4893+
function setupClassManipulationSpies() {
4894+
inject(function($animate) {
4895+
addClass = spyOn($animateCore, '$$addClassImmediately').andCallThrough();
4896+
removeClass = spyOn($animateCore, '$$removeClassImmediately').andCallThrough();
4897+
});
4898+
}
4899+
4900+
function setupClassManipulationLogger(log) {
4901+
inject(function($animate) {
4902+
var addClassImmediately = $animateCore.$$addClassImmediately;
4903+
var removeClassImmediately = $animateCore.$$removeClassImmediately;
4904+
addClass = spyOn($animateCore, '$$addClassImmediately').andCallFake(function(element, classes) {
4905+
var names = classes;
4906+
if (Object.prototype.toString.call(classes) === '[object Array]') names = classes.join( ' ');
4907+
log('addClass(' + names + ')');
4908+
return addClassImmediately.call($animateCore, element, classes);
4909+
});
4910+
removeClass = spyOn($animateCore, '$$removeClassImmediately').andCallFake(function(element, classes) {
4911+
var names = classes;
4912+
if (Object.prototype.toString.call(classes) === '[object Array]') names = classes.join( ' ');
4913+
log('removeClass(' + names + ')');
4914+
return removeClassImmediately.call($animateCore, element, classes);
4915+
});
4916+
});
4917+
}
4918+
4919+
it('should defer class manipulation until end of digest', inject(function($rootScope, $animate, log) {
4920+
setupClassManipulationLogger(log);
4921+
element = jqLite('<p>test</p>');
4922+
4923+
$rootScope.$apply(function() {
4924+
$animate.addClass(element, 'test-class1');
4925+
expect(element).not.toHaveClass('test-class1');
4926+
4927+
$animate.removeClass(element, 'test-class1');
4928+
4929+
$animate.addClass(element, 'test-class2');
4930+
expect(element).not.toHaveClass('test-class2');
4931+
4932+
$animate.setClass(element, 'test-class3', 'test-class4');
4933+
expect(element).not.toHaveClass('test-class3');
4934+
expect(element).not.toHaveClass('test-class4');
4935+
expect(log).toEqual([]);
4936+
});
4937+
4938+
expect(element).not.toHaveClass('test-class1');
4939+
expect(element).not.toHaveClass('test-class4');
4940+
expect(element).toHaveClass('test-class2');
4941+
expect(element).toHaveClass('test-class3');
4942+
expect(log).toEqual(['addClass(test-class2 test-class3)']);
4943+
expect(addClass.callCount).toBe(1);
4944+
expect(removeClass.callCount).toBe(0);
4945+
}));
4946+
4947+
4948+
it('should defer class manipulation until digest outside of digest', inject(function($rootScope, $animate, log) {
4949+
setupClassManipulationLogger(log);
4950+
element = jqLite('<p>test</p>');
4951+
4952+
$animate.addClass(element, 'test-class1');
4953+
$animate.removeClass(element, 'test-class1');
4954+
$animate.addClass(element, 'test-class2');
4955+
$animate.setClass(element, 'test-class3', 'test-class4');
4956+
4957+
expect(log).toEqual([]);
4958+
4959+
$rootScope.$digest();
4960+
4961+
expect(log).toEqual(['addClass(test-class2 test-class3)']);
4962+
expect(element).not.toHaveClass('test-class1');
4963+
expect(element).toHaveClass('test-class2');
4964+
expect(element).toHaveClass('test-class3');
4965+
expect(addClass.callCount).toBe(1);
4966+
expect(removeClass.callCount).toBe(0);
4967+
}));
4968+
4969+
4970+
it('should return a promise which is resolved on a different turn digest', inject(function(log, $animate, $browser, $rootScope) {
4971+
element = jqLite('<p class="test2">test</p>');
4972+
4973+
$animate.addClass(element, 'test1').then(log.fn('addClass(test1)'));
4974+
$animate.removeClass(element, 'test2').then(log.fn('removeClass(test2)'));
4975+
4976+
$rootScope.$digest();
4977+
$browser.defer.flush();
4978+
expect(log).toEqual(['addClass(test1)', 'removeClass(test2)']);
4979+
}));
4980+
4981+
4982+
it('should defer class manipulation until end of digest for SVG', inject(function($rootScope, $animate) {
4983+
if (!window.SVGElement) return;
4984+
setupClassManipulationSpies();
4985+
element = jqLite('<svg><g></g></svg>');
4986+
var target = element.children().eq(0);
4987+
4988+
$rootScope.$apply(function() {
4989+
$animate.addClass(target, 'test-class1');
4990+
expect(target).not.toHaveClass('test-class1');
4991+
4992+
$animate.removeClass(target, 'test-class1');
4993+
4994+
$animate.addClass(target, 'test-class2');
4995+
expect(target).not.toHaveClass('test-class2');
4996+
4997+
$animate.setClass(target, 'test-class3', 'test-class4');
4998+
expect(target).not.toHaveClass('test-class3');
4999+
expect(target).not.toHaveClass('test-class4');
5000+
});
5001+
5002+
expect(target).not.toHaveClass('test-class1');
5003+
expect(target).toHaveClass('test-class2');
5004+
expect(addClass.callCount).toBe(1);
5005+
expect(removeClass.callCount).toBe(0);
5006+
}));
5007+
5008+
5009+
it('should defer class manipulation until digest outside of digest for SVG', inject(function($rootScope, $animate, log) {
5010+
if (!window.SVGElement) return;
5011+
setupClassManipulationLogger(log);
5012+
element = jqLite('<svg><g></g></svg>');
5013+
var target = element.children().eq(0);
5014+
5015+
$animate.addClass(target, 'test-class1');
5016+
$animate.removeClass(target, 'test-class1');
5017+
$animate.addClass(target, 'test-class2');
5018+
$animate.setClass(target, 'test-class3', 'test-class4');
5019+
5020+
expect(log).toEqual([]);
5021+
5022+
$rootScope.$digest();
5023+
5024+
expect(log).toEqual(['addClass(test-class2 test-class3)']);
5025+
expect(target).not.toHaveClass('test-class1');
5026+
expect(target).toHaveClass('test-class2');
5027+
expect(target).toHaveClass('test-class3');
5028+
expect(addClass.callCount).toBe(1);
5029+
expect(removeClass.callCount).toBe(0);
5030+
}));
5031+
});
48745032
});

0 commit comments

Comments
 (0)
This repository has been archived.