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

Commit ce274bd

Browse files
caitpcaitp
committed
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 11a1f4c commit ce274bd

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
@@ -994,8 +994,8 @@ angular.module('ngAnimate', ['ng'])
994994
element = stripCommentsFromElement(element);
995995

996996
if (classBasedAnimationsBlocked(element)) {
997-
if (add) $delegate.$$addClassImmediately(element, add);
998-
if (remove) $delegate.$$removeClassImmediately(element, remove);
997+
$delegate.$$addClassImmediately(element, add);
998+
$delegate.$$removeClassImmediately(element, remove);
999999
return;
10001000
}
10011001

‎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
@@ -573,24 +573,28 @@ describe('form', function() {
573573
expect(doc).toBeValid();
574574

575575
control.$setValidity('error', false);
576+
scope.$digest();
576577
expect(doc).toBeInvalid();
577578
expect(doc.hasClass('ng-valid-error')).toBe(false);
578579
expect(doc.hasClass('ng-invalid-error')).toBe(true);
579580

580581
control.$setValidity('another', false);
582+
scope.$digest();
581583
expect(doc.hasClass('ng-valid-error')).toBe(false);
582584
expect(doc.hasClass('ng-invalid-error')).toBe(true);
583585
expect(doc.hasClass('ng-valid-another')).toBe(false);
584586
expect(doc.hasClass('ng-invalid-another')).toBe(true);
585587

586588
control.$setValidity('error', true);
589+
scope.$digest();
587590
expect(doc).toBeInvalid();
588591
expect(doc.hasClass('ng-valid-error')).toBe(true);
589592
expect(doc.hasClass('ng-invalid-error')).toBe(false);
590593
expect(doc.hasClass('ng-valid-another')).toBe(false);
591594
expect(doc.hasClass('ng-invalid-another')).toBe(true);
592595

593596
control.$setValidity('another', true);
597+
scope.$digest();
594598
expect(doc).toBeValid();
595599
expect(doc.hasClass('ng-valid-error')).toBe(true);
596600
expect(doc.hasClass('ng-invalid-error')).toBe(false);
@@ -600,6 +604,7 @@ describe('form', function() {
600604
// validators are skipped, e.g. becuase of a parser error
601605
control.$setValidity('error', null);
602606
control.$setValidity('another', null);
607+
scope.$digest();
603608
expect(doc.hasClass('ng-valid-error')).toBe(false);
604609
expect(doc.hasClass('ng-invalid-error')).toBe(false);
605610
expect(doc.hasClass('ng-valid-another')).toBe(false);
@@ -671,7 +676,9 @@ describe('form', function() {
671676
expect(input1).toBeDirty();
672677
expect(input2).toBeDirty();
673678

679+
674680
formCtrl.$setPristine();
681+
scope.$digest();
675682
expect(form).toBePristine();
676683
expect(formCtrl.$pristine).toBe(true);
677684
expect(formCtrl.$dirty).toBe(false);
@@ -704,6 +711,7 @@ describe('form', function() {
704711
expect(input).toBeDirty();
705712

706713
formCtrl.$setPristine();
714+
scope.$digest();
707715
expect(form).toBePristine();
708716
expect(formCtrl.$pristine).toBe(true);
709717
expect(formCtrl.$dirty).toBe(false);
@@ -738,6 +746,7 @@ describe('form', function() {
738746
expect(nestedInput).toBeDirty();
739747

740748
formCtrl.$setPristine();
749+
scope.$digest();
741750
expect(form).toBePristine();
742751
expect(formCtrl.$pristine).toBe(true);
743752
expect(formCtrl.$dirty).toBe(false);

0 commit comments

Comments
 (0)