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

Commit d33cedd

Browse files
matskolgalfaso
authored andcommitted
fix(ngAnimate): always apply a preparation reflow for CSS-based animations
It's unpredictable sometimes to ensure that a browser triggers a reflow for an animation. Prior to this patch, reflows would be applied carefully in between parent/child DOM structure, but that doesn't seem to be enough for animations that contain complex CSS styling rules. Closes #12553 Closes #12554 Closes #12267 Closes #12554
1 parent 6838c97 commit d33cedd

File tree

3 files changed

+19
-75
lines changed

3 files changed

+19
-75
lines changed

src/ngAnimate/animateQueue.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
6767
});
6868

6969
this.$get = ['$$rAF', '$rootScope', '$rootElement', '$document', '$$body', '$$HashMap',
70-
'$$animation', '$$AnimateRunner', '$templateRequest', '$$jqLite',
70+
'$$animation', '$$AnimateRunner', '$templateRequest', '$$jqLite', '$$forceReflow',
7171
function($$rAF, $rootScope, $rootElement, $document, $$body, $$HashMap,
72-
$$animation, $$AnimateRunner, $templateRequest, $$jqLite) {
72+
$$animation, $$AnimateRunner, $templateRequest, $$jqLite, $$forceReflow) {
7373

7474
var activeAnimationsLookup = new $$HashMap();
7575
var disabledElementsLookup = new $$HashMap();
@@ -443,6 +443,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
443443

444444
markElementAnimationState(element, RUNNING_STATE);
445445
var realRunner = $$animation(element, event, animationDetails.options, function(e) {
446+
$$forceReflow();
446447
blockTransitions(getDomNode(e), false);
447448
});
448449

src/ngAnimate/animation.js

+8-39
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
1919
return element.data(RUNNER_STORAGE_KEY);
2020
}
2121

22-
this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$HashMap', '$$forceReflow',
23-
function($$jqLite, $rootScope, $injector, $$AnimateRunner, $$HashMap, $$forceReflow) {
22+
this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$HashMap',
23+
function($$jqLite, $rootScope, $injector, $$AnimateRunner, $$HashMap) {
2424

2525
var animationQueue = [];
2626
var applyAnimationClasses = applyAnimationClassesFactory($$jqLite);
@@ -91,11 +91,8 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
9191
result = result.concat(row);
9292
row = [];
9393
}
94-
row.push({
95-
fn: entry.fn,
96-
terminal: entry.children.length === 0
97-
});
98-
entry.children.forEach(function(childEntry) {
94+
row.push(entry.fn);
95+
forEach(entry.children, function(childEntry) {
9996
nextLevelEntries++;
10097
queue.push(childEntry);
10198
});
@@ -105,17 +102,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
105102
if (row.length) {
106103
result = result.concat(row);
107104
}
108-
109-
var terminalAnimations = [];
110-
var parentAnimations = [];
111-
forEach(result, function(result) {
112-
if (result.terminal) {
113-
terminalAnimations.push(result.fn);
114-
} else {
115-
parentAnimations.push(result.fn);
116-
}
117-
});
118-
return [parentAnimations, terminalAnimations];
105+
return result;
119106
}
120107
}
121108

@@ -224,27 +211,9 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
224211
});
225212

226213
// we need to sort each of the animations in order of parent to child
227-
// relationships. This ensures that the child classes are applied at the
228-
// right time.
229-
var anim = sortAnimations(toBeSortedAnimations);
230-
var finalLevel = anim.length - 1;
231-
232-
// sortAnimations will return two lists of animations. The first list
233-
// is all of the parent animations that are likely class-based and the
234-
// second list is a collection of the rest. Before we run the second
235-
// list we must ensure that atleast one reflow has been passed such that
236-
// the preparation classes (ng-enter, class-add, etc...) have been applied
237-
// to their associated element.
238-
if (anim[0].length) {
239-
forEach(anim[0], function(triggerAnimation) {
240-
$$forceReflow();
241-
triggerAnimation();
242-
});
243-
} else {
244-
$$forceReflow();
245-
}
246-
247-
forEach(anim[1], function(triggerAnimation) {
214+
// relationships. This ensures that the parent to child classes are
215+
// applied at the right time.
216+
forEach(sortAnimations(toBeSortedAnimations), function(triggerAnimation) {
248217
triggerAnimation();
249218
});
250219
});

test/ngAnimate/integrationSpec.js

+8-34
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ describe('ngAnimate integration tests', function() {
249249
expect(child).not.toHaveClass('expand-add');
250250
}));
251251

252-
it('should only issue a reflow for each parent CSS class change that contains ready-to-fire child animations', function() {
252+
it('should issue a reflow for each element animation on all DOM levels', function() {
253253
module('ngAnimateMock');
254254
inject(function($animate, $compile, $rootScope, $rootElement, $$rAF, $document) {
255255
element = jqLite(
@@ -273,11 +273,13 @@ describe('ngAnimate integration tests', function() {
273273
$rootScope.items = [1,2,3,4,5,6,7,8,9,10];
274274

275275
$rootScope.$digest();
276-
expect($animate.reflows).toBe(2);
276+
277+
// 2 parents + 10 items = 12
278+
expect($animate.reflows).toBe(12);
277279
});
278280
});
279281

280-
it('should issue a reflow for each parent class-based animation that contains active child animations', function() {
282+
it('should issue a reflow for each element and also its children', function() {
281283
module('ngAnimateMock');
282284
inject(function($animate, $compile, $rootScope, $rootElement, $$rAF, $document) {
283285
element = jqLite(
@@ -302,37 +304,9 @@ describe('ngAnimate integration tests', function() {
302304

303305
$rootScope.exp = true;
304306
$rootScope.$digest();
305-
expect($animate.reflows).toBe(2);
306-
});
307-
});
308-
309-
it('should only issue one reflow for class-based animations if none of them have children with queued animations', function() {
310-
module('ngAnimateMock');
311-
inject(function($animate, $compile, $rootScope, $rootElement, $$rAF, $document) {
312-
element = jqLite(
313-
'<div ng-class="{one:exp}">' +
314-
'<div ng-if="exp2"></div>' +
315-
'</div>' +
316-
'<div ng-class="{two:exp}">' +
317-
'<div ng-if="exp2"></div>' +
318-
'</div>' +
319-
'<div ng-class="{three:exp}"></div>'
320-
);
321307

322-
$rootElement.append(element);
323-
jqLite($document[0].body).append($rootElement);
324-
325-
$compile(element)($rootScope);
326-
$rootScope.$digest();
327-
expect($animate.reflows).toBe(0);
328-
329-
$rootScope.exp = true;
330-
$rootScope.$digest();
331-
expect($animate.reflows).toBe(1);
332-
333-
$rootScope.exp2 = true;
334-
$rootScope.$digest();
335-
expect($animate.reflows).toBe(2);
308+
// there is one element's expression in there that is false
309+
expect($animate.reflows).toBe(6);
336310
});
337311
});
338312

@@ -356,7 +330,7 @@ describe('ngAnimate integration tests', function() {
356330
$rootScope.items = [1,2,3,4,5,6,7,8,9,10];
357331
$rootScope.$digest();
358332

359-
expect($animate.reflows).toBe(1);
333+
expect($animate.reflows).toBe(10);
360334
});
361335
});
362336
});

0 commit comments

Comments
 (0)