fix($animate): ensure that animateable directives cancel expired leave 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
This commit is contained in:
Matias Niemelä
2014-02-21 03:43:50 -05:00
parent c9245cf759
commit e9881991ca
10 changed files with 269 additions and 13 deletions

View File

@@ -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)) {
@@ -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;
}
}

View File

@@ -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;
}
};

View File

@@ -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 = [];

View File

@@ -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);

View File

@@ -189,6 +189,7 @@ function ngViewFactory( $route, $anchorScroll, $animate) {
link: function(scope, $element, attr, ctrl, $transclude) {
var currentScope,
currentElement,
previousElement,
autoScrollExp = attr.autoscroll,
onloadExp = attr.onload || '';
@@ -196,12 +197,19 @@ function ngViewFactory( $route, $anchorScroll, $animate) {
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;
}
}

View File

@@ -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);
});
});
});

View File

@@ -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);
});
});
});

View File

@@ -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);
});
});
});

View File

@@ -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');
});
});
});
});

View File

@@ -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);
});
});
});