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

Commit

Permalink
fix($animate): ensure that animateable directives cancel expired leav…
Browse files Browse the repository at this point in the history
…e animations

If enter -> leave -> enter -> leave occurs then the first leave animation will
animate alongside the second. This causes the very first DOM node (the view in ngView
for example) to animate at the same time as the most recent DOM node which ends
up being an undesired effect. This fix takes care of this issue.

Closes #5886
  • Loading branch information
matsko committed Feb 26, 2014
1 parent c9245cf commit e988199
Show file tree
Hide file tree
Showing 10 changed files with 269 additions and 13 deletions.
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

0 comments on commit e988199

Please sign in to comment.