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

fix($animate): abort class-based animations if the element is removed during digest #8958

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
16 changes: 15 additions & 1 deletion src/ngAnimate/animate.js
Original file line number Diff line number Diff line change
Expand Up @@ -1015,14 +1015,28 @@ angular.module('ngAnimate', ['ng'])
}

return cache.promise = runAnimationPostDigest(function(done) {
var parentElement;

//any ancestor elements past $rootElement's are not apart of the
//app therefore there is no need to examine the parent element
if (!isMatchingElement(element, $rootElement)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite get this check. it's not like we are traversing up until the $rootElement (even though the comment above makes it sounds like it).

parentElement = element.parent();
var elementNode = extractElementNode(element);
var parentNode = extractElementNode(parentElement);
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need to extract? the parent node must be an element, so just do element[0].parentNode or elementNode.parentNode

if (!parentNode || parentNode['$$NG_REMOVED'] || elementNode['$$NG_REMOVED']) {
done();
return;
}
}

var cache = element.data(STORAGE_KEY);
element.removeData(STORAGE_KEY);

var state = element.data(NG_ANIMATE_STATE) || {};
var classes = resolveElementClasses(element, cache, state.active);
return !classes
? done()
: performAnimation('setClass', classes, element, null, null, function() {
: performAnimation('setClass', classes, element, parentElement, null, function() {
$delegate.setClass(element, classes[0], classes[1]);
}, done);
});
Expand Down
95 changes: 95 additions & 0 deletions test/ngAnimate/animateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3413,6 +3413,101 @@ describe("ngAnimate", function() {
});
});

it('should skip class-based animations if the element is removed before the digest occurs', function() {
var spy = jasmine.createSpy();
module(function($animateProvider) {
$animateProvider.register('.animated', function() {
return {
beforeAddClass : spy,
beforeRemoveClass : spy,
beforeSetClass : spy
};
});
});
inject(function($rootScope, $animate, $compile, $rootElement, $document) {
$animate.enabled(true);

var one = $compile('<div class="animated"></div>')($rootScope);
var two = $compile('<div class="animated"></div>')($rootScope);
var three = $compile('<div class="animated three"></div>')($rootScope);

$rootElement.append(one);
$rootElement.append(two);
angular.element($document[0].body).append($rootElement);

$animate.addClass(one, 'active-class');
one.remove();

$rootScope.$digest();
expect(spy).not.toHaveBeenCalled();

$animate.addClass(two, 'active-class');

$rootScope.$digest();
expect(spy).toHaveBeenCalled();

spy.reset();
$animate.removeClass(two, 'active-class');
two.remove();

$rootScope.$digest();
expect(spy).not.toHaveBeenCalled();

$animate.setClass(three, 'active-class', 'three');
three.remove();

$rootScope.$digest();
expect(spy).not.toHaveBeenCalled();
});
});

it('should skip class-based animations if ngRepeat has marked the element or its parent for removal', function() {
var spy = jasmine.createSpy();
module(function($animateProvider) {
$animateProvider.register('.animated', function() {
return {
beforeAddClass : spy,
beforeRemoveClass : spy,
beforeSetClass : spy
};
});
});
inject(function($rootScope, $animate, $compile, $rootElement, $document) {
$animate.enabled(true);

var element = $compile(
'<div>' +
' <div ng-repeat="item in items" class="animated">' +
' <span>{{ $index }}</span>' +
' </div>' +
'</div>'
)($rootScope);

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

$rootScope.items = [1,2,3];
$rootScope.$digest();

var child = element.find('div');

$animate.addClass(child, 'start-animation');
$rootScope.items = [2,3];
$rootScope.$digest();

expect(spy).not.toHaveBeenCalled();

var innerChild = element.find('span');

$animate.addClass(innerChild, 'start-animation');
$rootScope.items = [3];
$rootScope.$digest();

expect(spy).not.toHaveBeenCalled();
dealoc(element);
});
});

it('should call class-based animation callbacks in the correct order when animations are skipped', function() {
var continueAnimation;
module(function($animateProvider) {
Expand Down