diff options
| author | Matias Niemelä | 2014-02-21 03:43:50 -0500 | 
|---|---|---|
| committer | Matias Niemelä | 2014-02-26 14:53:57 -0500 | 
| commit | e9881991ca0a5019d3a4215477738ed247898ba0 (patch) | |
| tree | 5bca5a431522715543288b94772882dcd994bbe2 | |
| parent | c9245cf759108add2a10ffca4d41b1c68c1e8c76 (diff) | |
| download | angular.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.js | 17 | ||||
| -rw-r--r-- | src/ng/directive/ngInclude.js | 12 | ||||
| -rw-r--r-- | src/ng/directive/ngSwitch.js | 25 | ||||
| -rw-r--r-- | src/ngAnimate/animate.js | 21 | ||||
| -rw-r--r-- | src/ngRoute/directive/ngView.js | 12 | ||||
| -rwxr-xr-x | test/ng/directive/ngIfSpec.js | 38 | ||||
| -rw-r--r-- | test/ng/directive/ngIncludeSpec.js | 40 | ||||
| -rw-r--r-- | test/ng/directive/ngSwitchSpec.js | 38 | ||||
| -rw-r--r-- | test/ngAnimate/animateSpec.js | 35 | ||||
| -rw-r--r-- | test/ngRoute/directive/ngViewSpec.js | 44 | 
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); +        }); +      });      }); | 
