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

fix(ngAnimate): remove prepare classes with multiple structural anima… #16681

Merged
merged 2 commits into from
Sep 6, 2018
Merged
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: 2 additions & 3 deletions src/ngAnimate/animation.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro

// TODO(matsko): document the signature in a better way
return function(element, event, options) {
var node = getDomNode(element);

options = prepareAnimationOptions(options);
var isStructural = ['enter', 'move', 'leave'].indexOf(event) >= 0;

Expand Down Expand Up @@ -186,8 +184,9 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro
forEach(groupedAnimations, function(animationEntry) {
var element = animationEntry.from ? animationEntry.from.element : animationEntry.element;
var extraClasses = options.addClass;

extraClasses = (extraClasses ? (extraClasses + ' ') : '') + NG_ANIMATE_CLASSNAME;
var cacheKey = $$animateCache.cacheKey(node, event, extraClasses, options.removeClass);
var cacheKey = $$animateCache.cacheKey(element[0], animationEntry.event, extraClasses, options.removeClass);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT, element could also be a DOM node (in which case element[0] will be undefined), no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see, at this point it's already normalized to be a DOM node

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, looking at it again, it seems that element is treated as a jqLite/jQuery element. So, it's fine.


toBeSortedAnimations.push({
element: element,
Expand Down
81 changes: 81 additions & 0 deletions test/ngAnimate/integrationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ describe('ngAnimate integration tests', function() {
ss.destroy();
});


it('should cancel a running and started removeClass animation when a follow-up addClass animation adds the same class',
inject(function($animate, $rootScope, $$rAF, $document, $rootElement) {

Expand Down Expand Up @@ -362,6 +363,7 @@ describe('ngAnimate integration tests', function() {
});
});


it('should add the preparation class for an enter animation before a parent class-based animation is applied', function() {
module('ngAnimateMock');
inject(function($animate, $compile, $rootScope, $rootElement, $document) {
Expand Down Expand Up @@ -397,6 +399,7 @@ describe('ngAnimate integration tests', function() {
});
});


it('should avoid adding the ng-enter-prepare method to a parent structural animation that contains child animations', function() {
module('ngAnimateMock');
inject(function($animate, $compile, $rootScope, $rootElement, $document, $$rAF) {
Expand Down Expand Up @@ -468,6 +471,84 @@ describe('ngAnimate integration tests', function() {
});
});


it('should remove the prepare classes when different structural animations happen in the same digest', function() {
module('ngAnimateMock');
inject(function($animate, $compile, $rootScope, $rootElement, $document, $$animateCache) {
element = jqLite(
// Class animation on parent element is neeeded so the child elements get the prepare class
'<div id="outer" ng-class="{blue: cond}" ng-switch="cond">' +
'<div id="default" ng-switch-default></div>' +
'<div id="truthy" ng-switch-when="true"></div>' +
'</div>'
);

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

$compile(element)($rootScope);
$rootScope.cond = false;
$rootScope.$digest();

$rootScope.cond = true;
$rootScope.$digest();

var parent = element;
var truthySwitch = jqLite(parent[0].querySelector('#truthy'));
var defaultSwitch = jqLite(parent[0].querySelector('#default'));

expect(parent).not.toHaveClass('blue');
expect(parent).toHaveClass('blue-add');
expect(truthySwitch).toHaveClass('ng-enter-prepare');
expect(defaultSwitch).toHaveClass('ng-leave-prepare');

$animate.flush();

expect(parent).toHaveClass('blue');
expect(parent).not.toHaveClass('blue-add');
expect(truthySwitch).not.toHaveClass('ng-enter-prepare');
expect(defaultSwitch).not.toHaveClass('ng-leave-prepare');
});
});

it('should respect the element node for caching when animations with the same type happen in the same digest', function() {
module('ngAnimateMock');
inject(function($animate, $compile, $rootScope, $rootElement, $document, $$animateCache) {
ss.addRule('.animate.ng-enter', 'transition:2s linear all;');

element = jqLite(
'<div>' +
'<div>' +
'<div id="noanimate" ng-if="cond"></div>' +
'</div>' +
'<div>' +
'<div id="animate" class="animate" ng-if="cond"></div>' +
'</div>' +
'</div>'
);

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

$compile(element)($rootScope);
$rootScope.cond = true;
$rootScope.$digest();

var parent = element;
var noanimate = jqLite(parent[0].querySelector('#noanimate'));
var animate = jqLite(parent[0].querySelector('#animate'));

expect(noanimate).not.toHaveClass('ng-enter');
expect(animate).toHaveClass('ng-enter');

$animate.closeAndFlush();

expect(noanimate).not.toHaveClass('ng-enter');
expect(animate).not.toHaveClass('ng-enter');
});
});


it('should pack level elements into their own RAF flush', function() {
module('ngAnimateMock');
inject(function($animate, $compile, $rootScope, $rootElement, $document) {
Expand Down