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

fix($animate): ensure that animateable directives cancel expired leave animations #6380

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
17 changes: 11 additions & 6 deletions src/ng/directive/ngIf.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ var ngIfDirective = ['$animate', function($animate) {
restrict: 'A',
$$tlb: true,
link: function ($scope, $element, $attr, ctrl, $transclude) {
var block, childScope;
var block, childScope, previousElements;
$scope.$watch($attr.ngIf, function ngIfWatchAction(value) {

if (toBoolean(value)) {
Expand All @@ -102,14 +102,19 @@ var ngIfDirective = ['$animate', function($animate) {
});
}
} else {

if (childScope) {
if(previousElements) {
previousElements.remove();
previousElements = null;
}
if(childScope) {
childScope.$destroy();
childScope = null;
}

if (block) {
$animate.leave(getBlockElements(block.clone));
if(block) {
previousElements = getBlockElements(block.clone);
$animate.leave(previousElements, function() {
previousElements = null;
});
block = null;
}
}
Expand Down
12 changes: 10 additions & 2 deletions src/ng/directive/ngInclude.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,15 +177,23 @@ var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$animate'
return function(scope, $element, $attr, ctrl, $transclude) {
var changeCounter = 0,
currentScope,
previousElement,
currentElement;

var cleanupLastIncludeContent = function() {
if (currentScope) {
if(previousElement) {
previousElement.remove();
previousElement = null;
}
if(currentScope) {
currentScope.$destroy();
currentScope = null;
}
if(currentElement) {
$animate.leave(currentElement);
$animate.leave(currentElement, function() {
previousElement = null;
});
previousElement = currentElement;
currentElement = null;
}
};
Expand Down
25 changes: 22 additions & 3 deletions src/ng/directive/ngSwitch.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,31 @@ var ngSwitchDirective = ['$animate', function($animate) {
var watchExpr = attr.ngSwitch || attr.on,
selectedTranscludes,
selectedElements,
previousElements,
selectedScopes = [];

scope.$watch(watchExpr, function ngSwitchWatchAction(value) {
for (var i= 0, ii=selectedScopes.length; i<ii; i++) {
selectedScopes[i].$destroy();
$animate.leave(selectedElements[i]);
var i, ii = selectedScopes.length;
if(ii > 0) {
if(previousElements) {
for (i = 0; i < ii; i++) {
previousElements[i].remove();
}
previousElements = null;
}

previousElements = [];
for (i= 0; i<ii; i++) {
var selected = selectedElements[i];
selectedScopes[i].$destroy();
previousElements[i] = selected;
$animate.leave(selected, function() {
previousElements.splice(i, 1);
if(previousElements.length === 0) {
previousElements = null;
}
});
}
}

selectedElements = [];
Expand Down
21 changes: 21 additions & 0 deletions src/ngAnimate/animate.js
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,27 @@ angular.module('ngAnimate', ['ng'])
return;
}

if(animationEvent == 'leave') {
//there's no need to ever remove the listener since the element
//will be removed (destroyed) after the leave animation ends or
//is cancelled midway
element.one('$destroy', function(e) {
var element = angular.element(this);
var state = element.data(NG_ANIMATE_STATE) || {};
var activeLeaveAnimation = state.active['ng-leave'];
if(activeLeaveAnimation) {
var animations = activeLeaveAnimation.animations;

//if the before animation is completed then the element will be
//removed shortly after so there is no need to cancel the animation
if(!animations[0].beforeComplete) {
cancelAnimations(animations);
cleanup(element, 'ng-leave');
}
}
});
}

//the ng-animate class does nothing, but it's here to allow for
//parent animations to find and cancel child animations when needed
element.addClass(NG_ANIMATE_CLASS_NAME);
Expand Down
12 changes: 10 additions & 2 deletions src/ngRoute/directive/ngView.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,19 +189,27 @@ function ngViewFactory( $route, $anchorScroll, $animate) {
link: function(scope, $element, attr, ctrl, $transclude) {
var currentScope,
currentElement,
previousElement,
autoScrollExp = attr.autoscroll,
onloadExp = attr.onload || '';

scope.$on('$routeChangeSuccess', update);
update();

function cleanupLastView() {
if (currentScope) {
if(previousElement) {
previousElement.remove();
previousElement = null;
}
if(currentScope) {
currentScope.$destroy();
currentScope = null;
}
if(currentElement) {
$animate.leave(currentElement);
$animate.leave(currentElement, function() {
previousElement = null;
});
previousElement = currentElement;
currentElement = null;
}
}
Expand Down
38 changes: 38 additions & 0 deletions test/ng/directive/ngIfSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,42 @@ describe('ngIf animations', function () {
expect(element.children().length).toBe(0);
}));

it('should destroy the previous leave animation if a new one takes place', function() {
module(function($provide) {
$provide.value('$animate', {
enabled : function() { return true; },
leave : function() {
//DOM operation left blank
},
enter : function(element, parent) {
parent.append(element);
}
});
});
inject(function ($compile, $rootScope, $animate) {
var item;
var $scope = $rootScope.$new();
element = $compile(html(
'<div>' +
'<div ng-if="value">Yo</div>' +
'</div>'
))($scope);

$scope.$apply('value = true');

var destroyed, inner = element.children(0);
inner.on('$destroy', function() {
destroyed = true;
});

$scope.$apply('value = false');

$scope.$apply('value = true');

$scope.$apply('value = false');

expect(destroyed).toBe(true);
});
});

});
40 changes: 40 additions & 0 deletions test/ng/directive/ngIncludeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -663,4 +663,44 @@ describe('ngInclude animations', function() {
expect(itemA).not.toEqual(itemB);
}));

it('should destroy the previous leave animation if a new one takes place', function() {
module(function($provide) {
$provide.value('$animate', {
enabled : function() { return true; },
leave : function() {
//DOM operation left blank
},
enter : function(element, parent, after) {
angular.element(after).after(element);
}
});
});
inject(function ($compile, $rootScope, $animate, $templateCache) {
var item;
var $scope = $rootScope.$new();
element = $compile(html(
'<div>' +
'<div ng-include="inc">Yo</div>' +
'</div>'
))($scope);

$templateCache.put('one', [200, '<div>one</div>', {}]);
$templateCache.put('two', [200, '<div>two</div>', {}]);

$scope.$apply('inc = "one"');

var destroyed, inner = element.children(0);
inner.on('$destroy', function() {
destroyed = true;
});

$scope.$apply('inc = "two"');

$scope.$apply('inc = "one"');

$scope.$apply('inc = "two"');

expect(destroyed).toBe(true);
});
});
});
38 changes: 38 additions & 0 deletions test/ng/directive/ngSwitchSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -293,4 +293,42 @@ describe('ngSwitch animations', function() {
expect(item.element.text()).toBe('three');
}));

it('should destroy the previous leave animation if a new one takes place', function() {
module(function($provide) {
$provide.value('$animate', {
enabled : function() { return true; },
leave : function() {
//DOM operation left blank
},
enter : function(element, parent, after) {
angular.element(after).after(element);
}
});
});
inject(function ($compile, $rootScope, $animate, $templateCache) {
var item;
var $scope = $rootScope.$new();
element = $compile(html(
'<div ng-switch="inc">' +
'<div ng-switch-when="one">one</div>' +
'<div ng-switch-when="two">two</div>' +
'</div>'
))($scope);

$scope.$apply('inc = "one"');

var destroyed, inner = element.children(0);
inner.on('$destroy', function() {
destroyed = true;
});

$scope.$apply('inc = "two"');

$scope.$apply('inc = "one"');

$scope.$apply('inc = "two"');

expect(destroyed).toBe(true);
});
});
});
35 changes: 35 additions & 0 deletions test/ngAnimate/animateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3335,5 +3335,40 @@ describe("ngAnimate", function() {
expect(cancelReflowCallback).toHaveBeenCalled();
});
});

it('should immediately close off a leave animation if the element is removed from the DOM', function() {
var stat;
module(function($animateProvider) {
$animateProvider.register('.going', function() {
return {
leave : function() {
//left blank so it hangs
stat = 'leaving';
return function(cancelled) {
stat = cancelled && 'gone';
};
}
};
});
});
inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) {

$animate.enabled(true);

var element = $compile('<div id="parentGuy"></div>')($rootScope);
var child = $compile('<div class="going"></div>')($rootScope);
$rootElement.append(element);
element.append(child);

$animate.leave(child);
$rootScope.$digest();

expect(stat).toBe('leaving');

child.remove();

expect(stat).toBe('gone');
});
});
});
});
44 changes: 44 additions & 0 deletions test/ngRoute/directive/ngViewSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,50 @@ describe('ngView animations', function() {
}
});
});

it('should destroy the previous leave animation if a new one takes place', function() {
module(function($provide) {
$provide.value('$animate', {
enabled : function() { return true; },
leave : function() {
//DOM operation left blank
},
enter : function(element, parent, after) {
angular.element(after).after(element);
}
});
});
inject(function ($compile, $rootScope, $animate, $location) {
var item;
var $scope = $rootScope.$new();
element = $compile(html(
'<div>' +
'<div ng-view></div>' +
'</div>'
))($scope);

$scope.$apply('value = true');

$location.path('/bar');
$rootScope.$digest();

var destroyed, inner = element.children(0);
inner.on('$destroy', function() {
destroyed = true;
});

$location.path('/foo');
$rootScope.$digest();

$location.path('/bar');
$rootScope.$digest();

$location.path('/bar');
$rootScope.$digest();

expect(destroyed).toBe(true);
});
});
});


Expand Down