Skip to content
This repository has been archived by the owner on Apr 13, 2022. It is now read-only.

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(ngAnimate): always apply a preparation reflow for CSS-based anima…
…tions

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
  • Loading branch information
matsko authored and lgalfaso committed Aug 12, 2015
1 parent 6838c97 commit d33cedd
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 75 deletions.
5 changes: 3 additions & 2 deletions src/ngAnimate/animateQueue.js
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
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
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

0 comments on commit d33cedd

Please sign in to comment.