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

fix(ngAnimate): always apply a preparation reflow for CSS-based animations #12554

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/ngAnimate/animateQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
});

this.$get = ['$$rAF', '$rootScope', '$rootElement', '$document', '$$body', '$$HashMap',
'$$animation', '$$AnimateRunner', '$templateRequest', '$$jqLite',
'$$animation', '$$AnimateRunner', '$templateRequest', '$$jqLite', '$$forceReflow',
function($$rAF, $rootScope, $rootElement, $document, $$body, $$HashMap,
$$animation, $$AnimateRunner, $templateRequest, $$jqLite) {
$$animation, $$AnimateRunner, $templateRequest, $$jqLite, $$forceReflow) {

var activeAnimationsLookup = new $$HashMap();
var disabledElementsLookup = new $$HashMap();
Expand Down Expand Up @@ -443,6 +443,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {

markElementAnimationState(element, RUNNING_STATE);
var realRunner = $$animation(element, event, animationDetails.options, function(e) {
$$forceReflow();
blockTransitions(getDomNode(e), false);
});

Expand Down
47 changes: 8 additions & 39 deletions src/ngAnimate/animation.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
return element.data(RUNNER_STORAGE_KEY);
}

this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$HashMap', '$$forceReflow',
function($$jqLite, $rootScope, $injector, $$AnimateRunner, $$HashMap, $$forceReflow) {
this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$HashMap',
function($$jqLite, $rootScope, $injector, $$AnimateRunner, $$HashMap) {

var animationQueue = [];
var applyAnimationClasses = applyAnimationClassesFactory($$jqLite);
Expand Down Expand Up @@ -91,11 +91,8 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
result = result.concat(row);
row = [];
}
row.push({
fn: entry.fn,
terminal: entry.children.length === 0
});
entry.children.forEach(function(childEntry) {
row.push(entry.fn);
forEach(entry.children, function(childEntry) {
nextLevelEntries++;
queue.push(childEntry);
});
Expand All @@ -105,17 +102,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
if (row.length) {
result = result.concat(row);
}

var terminalAnimations = [];
var parentAnimations = [];
forEach(result, function(result) {
if (result.terminal) {
terminalAnimations.push(result.fn);
} else {
parentAnimations.push(result.fn);
}
});
return [parentAnimations, terminalAnimations];
return result;
}
}

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

// we need to sort each of the animations in order of parent to child
// relationships. This ensures that the child classes are applied at the
// right time.
var anim = sortAnimations(toBeSortedAnimations);
var finalLevel = anim.length - 1;

// sortAnimations will return two lists of animations. The first list
// is all of the parent animations that are likely class-based and the
// second list is a collection of the rest. Before we run the second
// list we must ensure that atleast one reflow has been passed such that
// the preparation classes (ng-enter, class-add, etc...) have been applied
// to their associated element.
if (anim[0].length) {
forEach(anim[0], function(triggerAnimation) {
$$forceReflow();
triggerAnimation();
});
} else {
$$forceReflow();
}

forEach(anim[1], function(triggerAnimation) {
// relationships. This ensures that the parent to child classes are
// applied at the right time.
forEach(sortAnimations(toBeSortedAnimations), function(triggerAnimation) {
triggerAnimation();
});
});
Expand Down
42 changes: 8 additions & 34 deletions test/ngAnimate/integrationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ describe('ngAnimate integration tests', function() {
expect(child).not.toHaveClass('expand-add');
}));

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

$rootScope.$digest();
expect($animate.reflows).toBe(2);

// 2 parents + 10 items = 12
expect($animate.reflows).toBe(12);
});
});

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

$rootScope.exp = true;
$rootScope.$digest();
expect($animate.reflows).toBe(2);
});
});

it('should only issue one reflow for class-based animations if none of them have children with queued animations', function() {
module('ngAnimateMock');
inject(function($animate, $compile, $rootScope, $rootElement, $$rAF, $document) {
element = jqLite(
'<div ng-class="{one:exp}">' +
'<div ng-if="exp2"></div>' +
'</div>' +
'<div ng-class="{two:exp}">' +
'<div ng-if="exp2"></div>' +
'</div>' +
'<div ng-class="{three:exp}"></div>'
);

$rootElement.append(element);
jqLite($document[0].body).append($rootElement);

$compile(element)($rootScope);
$rootScope.$digest();
expect($animate.reflows).toBe(0);

$rootScope.exp = true;
$rootScope.$digest();
expect($animate.reflows).toBe(1);

$rootScope.exp2 = true;
$rootScope.$digest();
expect($animate.reflows).toBe(2);
// there is one element's expression in there that is false
expect($animate.reflows).toBe(6);
});
});

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

expect($animate.reflows).toBe(1);
expect($animate.reflows).toBe(10);
});
});
});
Expand Down