aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatias Niemelä2014-02-21 03:43:50 -0500
committerMatias Niemelä2014-02-26 14:53:57 -0500
commite9881991ca0a5019d3a4215477738ed247898ba0 (patch)
tree5bca5a431522715543288b94772882dcd994bbe2
parentc9245cf759108add2a10ffca4d41b1c68c1e8c76 (diff)
downloadangular.js-e9881991ca0a5019d3a4215477738ed247898ba0.tar.bz2
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
-rw-r--r--src/ng/directive/ngIf.js17
-rw-r--r--src/ng/directive/ngInclude.js12
-rw-r--r--src/ng/directive/ngSwitch.js25
-rw-r--r--src/ngAnimate/animate.js21
-rw-r--r--src/ngRoute/directive/ngView.js12
-rwxr-xr-xtest/ng/directive/ngIfSpec.js38
-rw-r--r--test/ng/directive/ngIncludeSpec.js40
-rw-r--r--test/ng/directive/ngSwitchSpec.js38
-rw-r--r--test/ngAnimate/animateSpec.js35
-rw-r--r--test/ngRoute/directive/ngViewSpec.js44
10 files changed, 269 insertions, 13 deletions
diff --git a/src/ng/directive/ngIf.js b/src/ng/directive/ngIf.js
index 000fba82..a31015b2 100644
--- a/src/ng/directive/ngIf.js
+++ b/src/ng/directive/ngIf.js
@@ -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;
}
}
diff --git a/src/ng/directive/ngInclude.js b/src/ng/directive/ngInclude.js
index 29e3abce..272e199a 100644
--- a/src/ng/directive/ngInclude.js
+++ b/src/ng/directive/ngInclude.js
@@ -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;
}
};
diff --git a/src/ng/directive/ngSwitch.js b/src/ng/directive/ngSwitch.js
index 92594978..378c0b50 100644
--- a/src/ng/directive/ngSwitch.js
+++ b/src/ng/directive/ngSwitch.js
@@ -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 = [];
diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js
index c09e714e..6b1eedfc 100644
--- a/src/ngAnimate/animate.js
+++ b/src/ngAnimate/animate.js
@@ -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);
diff --git a/src/ngRoute/directive/ngView.js b/src/ngRoute/directive/ngView.js
index 3fa851a7..448e375c 100644
--- a/src/ngRoute/directive/ngView.js
+++ b/src/ngRoute/directive/ngView.js
@@ -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;
}
}
diff --git a/test/ng/directive/ngIfSpec.js b/test/ng/directive/ngIfSpec.js
index d40e6812..771e264a 100755
--- a/test/ng/directive/ngIfSpec.js
+++ b/test/ng/directive/ngIfSpec.js
@@ -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);
+ });
+ });
+
});
diff --git a/test/ng/directive/ngIncludeSpec.js b/test/ng/directive/ngIncludeSpec.js
index 9f37d1fe..510fa38a 100644
--- a/test/ng/directive/ngIncludeSpec.js
+++ b/test/ng/directive/ngIncludeSpec.js
@@ -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);
+ });
+ });
});
diff --git a/test/ng/directive/ngSwitchSpec.js b/test/ng/directive/ngSwitchSpec.js
index e039c4d5..2b771a0c 100644
--- a/test/ng/directive/ngSwitchSpec.js
+++ b/test/ng/directive/ngSwitchSpec.js
@@ -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);
+ });
+ });
});
diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js
index 55ec4ae8..a30b5fe9 100644
--- a/test/ngAnimate/animateSpec.js
+++ b/test/ngAnimate/animateSpec.js
@@ -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');
+ });
+ });
});
});
diff --git a/test/ngRoute/directive/ngViewSpec.js b/test/ngRoute/directive/ngViewSpec.js
index 259940c6..11a13e40 100644
--- a/test/ngRoute/directive/ngViewSpec.js
+++ b/test/ngRoute/directive/ngViewSpec.js
@@ -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);
+ });
+ });
});