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

Commit 36ad40b

Browse files
matskomhevery
matsko
authored and
mhevery
committed
fix(ngAnimate): ensure that ngClass is always compiled before enter, leave and move animations
Closes #3727 Closes #3603
1 parent 4e15c4f commit 36ad40b

File tree

4 files changed

+147
-37
lines changed

4 files changed

+147
-37
lines changed

‎src/ngAnimate/animate.js

+25-9
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,9 @@ angular.module('ngAnimate', ['ng'])
201201

202202
var NG_ANIMATE_STATE = '$$ngAnimateState';
203203
var rootAnimateState = {running:true};
204-
$provide.decorator('$animate', ['$delegate', '$injector', '$sniffer', '$rootElement', '$timeout',
205-
function($delegate, $injector, $sniffer, $rootElement, $timeout) {
206-
204+
$provide.decorator('$animate', ['$delegate', '$injector', '$sniffer', '$rootElement', '$timeout', '$rootScope',
205+
function($delegate, $injector, $sniffer, $rootElement, $timeout, $rootScope) {
206+
207207
$rootElement.data(NG_ANIMATE_STATE, rootAnimateState);
208208

209209
function lookup(name) {
@@ -282,8 +282,10 @@ angular.module('ngAnimate', ['ng'])
282282
*/
283283
enter : function(element, parent, after, done) {
284284
$delegate.enter(element, parent, after);
285-
performAnimation('enter', 'ng-enter', element, parent, after, function() {
286-
done && $timeout(done, 0, false);
285+
$rootScope.$$postDigest(function() {
286+
performAnimation('enter', 'ng-enter', element, parent, after, function() {
287+
done && $timeout(done, 0, false);
288+
});
287289
});
288290
},
289291

@@ -315,8 +317,10 @@ angular.module('ngAnimate', ['ng'])
315317
* @param {function()=} done callback function that will be called once the animation is complete
316318
*/
317319
leave : function(element, done) {
318-
performAnimation('leave', 'ng-leave', element, null, null, function() {
319-
$delegate.leave(element, done);
320+
$rootScope.$$postDigest(function() {
321+
performAnimation('leave', 'ng-leave', element, null, null, function() {
322+
$delegate.leave(element, done);
323+
});
320324
});
321325
},
322326

@@ -352,8 +356,10 @@ angular.module('ngAnimate', ['ng'])
352356
*/
353357
move : function(element, parent, after, done) {
354358
$delegate.move(element, parent, after);
355-
performAnimation('move', 'ng-move', element, null, null, function() {
356-
done && $timeout(done, 0, false);
359+
$rootScope.$$postDigest(function() {
360+
performAnimation('move', 'ng-move', element, null, null, function() {
361+
done && $timeout(done, 0, false);
362+
});
357363
});
358364
},
359365

@@ -550,6 +556,7 @@ angular.module('ngAnimate', ['ng'])
550556

551557
var durationKey = 'Duration',
552558
delayKey = 'Delay',
559+
propertyKey = 'Property',
553560
animationIterationCountKey = 'IterationCount',
554561
ELEMENT_NODE = 1;
555562

@@ -610,13 +617,22 @@ angular.module('ngAnimate', ['ng'])
610617
timeout is empty (this would cause a flicker bug normally
611618
in the page */
612619
if(duration > 0) {
620+
var node = element[0];
621+
622+
//temporarily disable the transition so that the enter styles
623+
//don't animate twice (this is here to avoid a bug in Chrome/FF).
624+
node.style[w3cTransitionProp + propertyKey] = 'none';
625+
node.style[vendorTransitionProp + propertyKey] = 'none';
626+
613627
var activeClassName = '';
614628
forEach(className.split(' '), function(klass, i) {
615629
activeClassName += (i > 0 ? ' ' : '') + klass + '-active';
616630
});
617631

618632
//this triggers a reflow which allows for the transition animation to kick in
619633
element.prop('clientWidth');
634+
node.style[w3cTransitionProp + propertyKey] = '';
635+
node.style[vendorTransitionProp + propertyKey] = '';
620636
element.addClass(activeClassName);
621637

622638
$timeout(done, duration * 1000, false);

‎test/ng/directive/ngClassSpec.js

+95-28
Original file line numberDiff line numberDiff line change
@@ -308,40 +308,107 @@ describe('ngClass', function() {
308308
describe('ngClass animations', function() {
309309
var body, element, $rootElement;
310310

311-
beforeEach(module('mock.animate'));
312-
313-
it("should avoid calling addClass accidentally when removeClass is going on",
311+
it("should avoid calling addClass accidentally when removeClass is going on", function() {
312+
module('mock.animate');
314313
inject(function($compile, $rootScope, $animate, $timeout) {
314+
var element = angular.element('<div ng-class="val"></div>');
315+
var body = jqLite(document.body);
316+
body.append(element);
317+
$compile(element)($rootScope);
315318

316-
var element = angular.element('<div ng-class="val"></div>');
317-
var body = jqLite(document.body);
318-
body.append(element);
319-
$compile(element)($rootScope);
319+
expect($animate.queue.length).toBe(0);
320320

321-
expect($animate.queue.length).toBe(0);
321+
$rootScope.val = 'one';
322+
$rootScope.$digest();
323+
$animate.flushNext('addClass');
324+
$animate.flushNext('addClass');
325+
expect($animate.queue.length).toBe(0);
322326

323-
$rootScope.val = 'one';
324-
$timeout.flush();
327+
$rootScope.val = '';
328+
$rootScope.$digest();
329+
$animate.flushNext('removeClass'); //only removeClass is called
330+
expect($animate.queue.length).toBe(0);
325331

326-
$rootScope.$digest();
327-
$animate.flushNext('addClass');
328-
$animate.flushNext('addClass');
329-
expect($animate.queue.length).toBe(0);
332+
$rootScope.val = 'one';
333+
$rootScope.$digest();
334+
$animate.flushNext('addClass');
335+
expect($animate.queue.length).toBe(0);
330336

331-
$rootScope.val = '';
332-
$rootScope.$digest();
333-
$animate.flushNext('removeClass'); //only removeClass is called
334-
expect($animate.queue.length).toBe(0);
337+
$rootScope.val = 'two';
338+
$rootScope.$digest();
339+
$animate.flushNext('removeClass');
340+
$animate.flushNext('addClass');
341+
expect($animate.queue.length).toBe(0);
342+
});
343+
});
335344

336-
$rootScope.val = 'one';
337-
$rootScope.$digest();
338-
$animate.flushNext('addClass');
339-
expect($animate.queue.length).toBe(0);
345+
it("should consider the ngClass expression evaluation before performing an animation", function() {
346+
347+
//mocks are not used since the enter delegation method is called before addClass and
348+
//it makes it impossible to test to see that addClass is called first
349+
module('ngAnimate');
350+
351+
var digestQueue = [];
352+
module(function($animateProvider) {
353+
$animateProvider.register('.crazy', function() {
354+
return {
355+
enter : function(element, done) {
356+
element.data('state', 'crazy-enter');
357+
done();
358+
}
359+
};
360+
});
361+
362+
return function($rootScope) {
363+
var before = $rootScope.$$postDigest;
364+
$rootScope.$$postDigest = function() {
365+
var args = arguments;
366+
digestQueue.push(function() {
367+
before.apply($rootScope, args);
368+
});
369+
};
370+
};
371+
});
372+
inject(function($compile, $rootScope, $rootElement, $animate, $timeout, $document) {
340373

341-
$rootScope.val = 'two';
342-
$rootScope.$digest();
343-
$animate.flushNext('removeClass');
344-
$animate.flushNext('addClass');
345-
expect($animate.queue.length).toBe(0);
346-
}));
374+
//since we skip animations upon first digest, this needs to be set to true
375+
$animate.enabled(true);
376+
377+
$rootScope.val = 'crazy';
378+
var element = angular.element('<div ng-class="val"></div>');
379+
jqLite($document[0].body).append($rootElement);
380+
381+
$compile(element)($rootScope);
382+
383+
var enterComplete = false;
384+
$animate.enter(element, $rootElement, null, function() {
385+
enterComplete = true;
386+
});
387+
388+
//jquery doesn't compare both elements properly so let's use the nodes
389+
expect(element.parent()[0]).toEqual($rootElement[0]);
390+
expect(element.hasClass('crazy')).toBe(false);
391+
expect(enterComplete).toBe(false);
392+
393+
expect(digestQueue.length).toBe(1);
394+
$rootScope.$digest();
395+
396+
$timeout.flush();
397+
398+
expect(element.hasClass('crazy')).toBe(true);
399+
expect(enterComplete).toBe(false);
400+
401+
digestQueue.shift()(); //enter
402+
expect(digestQueue.length).toBe(0);
403+
404+
//we don't normally need this, but since the timing between digests
405+
//is spaced-out then it is required so that the original digestion
406+
//is kicked into gear
407+
$rootScope.$digest();
408+
$timeout.flush();
409+
410+
expect(element.data('state')).toBe('crazy-enter');
411+
expect(enterComplete).toBe(true);
412+
});
413+
});
347414
});

‎test/ngAnimate/animateSpec.js

+22
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ describe("ngAnimate", function() {
133133

134134
expect(element.contents().length).toBe(0);
135135
$animate.enter(child, element);
136+
$rootScope.$digest();
136137

137138
if($sniffer.transitions) {
138139
expect(child.hasClass('ng-enter')).toBe(true);
@@ -148,6 +149,8 @@ describe("ngAnimate", function() {
148149

149150
expect(element.contents().length).toBe(1);
150151
$animate.leave(child);
152+
$rootScope.$digest();
153+
151154
if($sniffer.transitions) {
152155
expect(child.hasClass('ng-leave')).toBe(true);
153156
expect(child.hasClass('ng-leave-active')).toBe(true);
@@ -169,6 +172,7 @@ describe("ngAnimate", function() {
169172
element.append(child2);
170173
expect(element.text()).toBe('12');
171174
$animate.move(child1, element, child2);
175+
$rootScope.$digest();
172176
expect(element.text()).toBe('21');
173177
}));
174178

@@ -213,13 +217,17 @@ describe("ngAnimate", function() {
213217

214218
//enter
215219
$animate.enter(child, element);
220+
$rootScope.$digest();
221+
216222
expect(child.attr('class')).toContain('ng-enter');
217223
expect(child.attr('class')).toContain('ng-enter-active');
218224
$timeout.flushNext(1000);
219225

220226
//move
221227
element.append(after);
222228
$animate.move(child, element, after);
229+
$rootScope.$digest();
230+
223231
expect(child.attr('class')).toContain('ng-move');
224232
expect(child.attr('class')).toContain('ng-move-active');
225233
$timeout.flushNext(1000);
@@ -238,6 +246,7 @@ describe("ngAnimate", function() {
238246

239247
//leave
240248
$animate.leave(child);
249+
$rootScope.$digest();
241250
expect(child.attr('class')).toContain('ng-leave');
242251
expect(child.attr('class')).toContain('ng-leave-active');
243252
$timeout.flushNext(1000);
@@ -274,6 +283,7 @@ describe("ngAnimate", function() {
274283
expect(child).toBeShown();
275284

276285
$animate.leave(child);
286+
$rootScope.$digest();
277287
expect(child).toBeHidden(); //hides instantly
278288

279289
//lets change this to prove that done doesn't fire anymore for the previous hide() operation
@@ -682,6 +692,7 @@ describe("ngAnimate", function() {
682692

683693
element[0].className = 'abc';
684694
$animate.enter(element, parent);
695+
$rootScope.$digest();
685696

686697
if ($sniffer.transitions) {
687698
expect(element.hasClass('abc ng-enter')).toBe(true);
@@ -692,6 +703,7 @@ describe("ngAnimate", function() {
692703

693704
element[0].className = 'xyz';
694705
$animate.enter(element, parent);
706+
$rootScope.$digest();
695707

696708
if ($sniffer.transitions) {
697709
expect(element.hasClass('xyz')).toBe(true);
@@ -717,6 +729,7 @@ describe("ngAnimate", function() {
717729
element.attr('class','one two');
718730

719731
$animate.enter(element, parent);
732+
$rootScope.$digest();
720733
if($sniffer.transitions) {
721734
expect(element.hasClass('one two ng-enter')).toBe(true);
722735
expect(element.hasClass('one two ng-enter ng-enter-active')).toBe(true);
@@ -766,6 +779,7 @@ describe("ngAnimate", function() {
766779
$animate.enter(element, parent, null, function() {
767780
flag = true;
768781
});
782+
$rootScope.$digest();
769783

770784
$timeout.flush();
771785

@@ -784,6 +798,7 @@ describe("ngAnimate", function() {
784798
$animate.leave(element, function() {
785799
flag = true;
786800
});
801+
$rootScope.$digest();
787802

788803
$timeout.flush();
789804

@@ -803,6 +818,7 @@ describe("ngAnimate", function() {
803818
$animate.move(element, parent, parent2, function() {
804819
flag = true;
805820
});
821+
$rootScope.$digest();
806822

807823
$timeout.flush();
808824

@@ -1212,6 +1228,7 @@ describe("ngAnimate", function() {
12121228
var child = $compile('<div>...</div>')($rootScope);
12131229

12141230
$animate.enter(child, element);
1231+
$rootScope.$digest();
12151232

12161233
if($sniffer.transitions) {
12171234
expect(child.hasClass('ng-enter')).toBe(true);
@@ -1233,6 +1250,7 @@ describe("ngAnimate", function() {
12331250
var child = $compile('<div>...</div>')($rootScope);
12341251

12351252
$animate.enter(child, element);
1253+
$rootScope.$digest();
12361254

12371255
if($sniffer.transitions) {
12381256
expect(child.hasClass('ng-enter')).toBe(true);
@@ -1257,6 +1275,7 @@ describe("ngAnimate", function() {
12571275

12581276
expect(child.hasClass('ng-enter')).toBe(false);
12591277
$animate.enter(child, element);
1278+
$rootScope.$digest();
12601279
expect(child.hasClass('ng-enter')).toBe(false);
12611280
}));
12621281

@@ -1283,6 +1302,7 @@ describe("ngAnimate", function() {
12831302

12841303
child.addClass('custom');
12851304
$animate.enter(child, element);
1305+
$rootScope.$digest();
12861306

12871307
$timeout.flushNext(10);
12881308

@@ -1315,6 +1335,7 @@ describe("ngAnimate", function() {
13151335
var child = $compile('<div>...</div>')($rootScope);
13161336

13171337
$animate.enter(child, element);
1338+
$rootScope.$digest();
13181339

13191340
//this is added/removed right away otherwise
13201341
if($sniffer.transitions) {
@@ -1325,6 +1346,7 @@ describe("ngAnimate", function() {
13251346
expect(child.hasClass('this-is-mine-now')).toBe(false);
13261347
child.addClass('usurper');
13271348
$animate.leave(child);
1349+
$rootScope.$digest();
13281350

13291351
expect(child.hasClass('ng-enter')).toBe(false);
13301352
expect(child.hasClass('ng-enter-active')).toBe(false);

‎test/ngRoute/directive/ngViewSpec.js

+5
Original file line numberDiff line numberDiff line change
@@ -637,13 +637,18 @@ describe('ngView animations', function() {
637637
$rootScope.$digest();
638638

639639
$animate.flushNext('leave'); //ngView old
640+
641+
$rootScope.$digest();
642+
640643
$animate.flushNext('enter'); //ngView new
641644

642645
expect(n(element.text())).toEqual(''); //this is midway during the animation
643646

644647
$animate.flushNext('enter'); //ngRepeat 3
645648
$animate.flushNext('enter'); //ngRepeat 4
646649

650+
$rootScope.$digest();
651+
647652
expect(element.text()).toEqual('34');
648653

649654
function n(text) {

0 commit comments

Comments
 (0)