fix($animate): correctly handle $animate.pin() host elements

This commit fixes two bugs: 1) Previously, animate would assume that a found host element was part of the $rootElement (while it's possible that it is also outside the root). 2) Previously, if a parent of the animated element was pinned to a host element, the host would not be checked regarding animations enabled status etc. Closes #13783
angular · Jan 19, 2016 · 7700e2d · 7700e2d
1 parent 3fb809e
commit 7700e2d
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 35 deletions.
diff --git a/src/ngAnimate/animateQueue.js b/src/ngAnimate/animateQueue.js
@@ -624,25 +624,22 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
         // there is no need to continue traversing at this point
         if (parentAnimationDetected && animateChildren === false) break;
 
-        if (!rootElementDetected) {
-          // angular doesn't want to attempt to animate elements outside of the application
-          // therefore we need to ensure that the rootElement is an ancestor of the current element
-          rootElementDetected = isMatchingElement(parentElement, $rootElement);
-          if (!rootElementDetected) {
-            parentHost = parentElement.data(NG_ANIMATE_PIN_DATA);
-            if (parentHost) {
-              parentElement = parentHost;
-              rootElementDetected = true;
-            }
-          }
-        }
-
         if (!bodyElementDetected) {
           // we also need to ensure that the element is or will be apart of the body element
           // otherwise it is pointless to even issue an animation to be rendered
           bodyElementDetected = isMatchingElement(parentElement, bodyElement);
         }
 
+        if (!rootElementDetected) {
+          // If no rootElement is detected, check if the parentElement is pinned to another element
+          parentHost = parentElement.data(NG_ANIMATE_PIN_DATA);
+          if (parentHost) {
+            // The pin target element becomes the next parent element
+            parentElement = parentHost;
+            continue;
+          }
+        }
+
         parentElement = parentElement.parent();
       }
 

diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js
@@ -1483,10 +1483,14 @@ describe("animations", function() {
           return new $$AnimateRunner();
         };
       });
+
+      return function($animate) {
+        $animate.enabled(true);
+      };
     }));
 
     it('should throw if the arguments are not elements',
-      inject(function($animate, $compile, $document, $rootScope, $rootElement) {
+      inject(function($animate, $rootElement) {
 
       var element = jqLite('<div></div>');
 
@@ -1505,7 +1509,7 @@ describe("animations", function() {
     they('should animate an element inside a pinned element that is the $prop element',
       ['same', 'parent', 'grandparent'],
       function(elementRelation) {
-        inject(function($animate, $compile, $document, $rootElement, $rootScope) {
+        inject(function($animate, $document, $rootElement, $rootScope) {
 
         var pinElement, animateElement;
 
@@ -1543,34 +1547,92 @@ describe("animations", function() {
       });
     });
 
-    it('should adhere to the disabled state of the hosted parent when an element is pinned',
-      inject(function($animate, $compile, $document, $rootElement, $rootScope) {
+    they('should not animate an element when the pinned ($prop) element, is pinned to an element that is not a child of the $rootElement',
+      ['same', 'parent', 'grandparent'],
+      function(elementRelation) {
+        inject(function($animate, $document, $rootElement, $rootScope) {
+
+        var pinElement, animateElement, pinTargetElement = jqLite('<div></div>');
 
-      var innerParent = jqLite('<div></div>');
-      jqLite($document[0].body).append(innerParent);
-      innerParent.append($rootElement);
-      var innerChild = jqLite('<div></div>');
-      $rootElement.append(innerChild);
+        var innerParent = jqLite('<div></div>');
+        jqLite($document[0].body).append(innerParent);
+        innerParent.append($rootElement);
 
-      var element = jqLite('<div></div>');
-      jqLite($document[0].body).append(element);
+        switch (elementRelation) {
+          case 'same':
+            pinElement = jqLite('<div id="animate"></div>');
+            break;
+          case 'parent':
+            pinElement = jqLite('<div><div id="animate"></div></div>');
+            break;
+          case 'grandparent':
+            pinElement = jqLite('<div><div><div id="animate"></div></div></div>');
+            break;
+        }
 
-      $animate.pin(element, innerChild);
+        // Append both the pin element and the pinTargetElement outside the app root
+        jqLite($document[0].body).append(pinElement);
+        jqLite($document[0].body).append(pinTargetElement);
 
-      $animate.enabled(innerChild, false);
+        animateElement = jqLite($document[0].getElementById('animate'));
 
-      $animate.addClass(element, 'blue');
-      $rootScope.$digest();
-      expect(capturedAnimation).toBeFalsy();
+        $animate.addClass(animateElement, 'red');
+        $rootScope.$digest();
+        expect(capturedAnimation).toBeFalsy();
 
-      $animate.enabled(innerChild, true);
+        $animate.pin(pinElement, pinTargetElement);
 
-      $animate.addClass(element, 'red');
-      $rootScope.$digest();
-      expect(capturedAnimation).toBeTruthy();
+        $animate.addClass(animateElement, 'blue');
+        $rootScope.$digest();
+        expect(capturedAnimation).toBeFalsy();
 
-      dealoc(element);
-    }));
+        dealoc(pinElement);
+      });
+    });
+
+    they('should adhere to the disabled state of the hosted parent when the $prop element is pinned',
+      ['same', 'parent', 'grandparent'],
+      function(elementRelation) {
+        inject(function($animate, $document, $rootElement, $rootScope) {
+
+        var pinElement, animateElement, pinHostElement = jqLite('<div></div>');
+
+        var innerParent = jqLite('<div></div>');
+        jqLite($document[0].body).append(innerParent);
+        innerParent.append($rootElement);
+
+        switch (elementRelation) {
+          case 'same':
+            pinElement = jqLite('<div id="animate"></div>');
+            break;
+          case 'parent':
+            pinElement = jqLite('<div><div id="animate"></div></div>');
+            break;
+          case 'grandparent':
+            pinElement = jqLite('<div><div><div id="animate"></div></div></div>');
+            break;
+        }
+
+        $rootElement.append(pinHostElement);
+        jqLite($document[0].body).append(pinElement);
+        animateElement = jqLite($document[0].getElementById('animate'));
+
+        $animate.pin(pinElement, pinHostElement);
+        $animate.enabled(pinHostElement, false);
+
+        $animate.addClass(animateElement, 'blue');
+        $rootScope.$digest();
+        expect(capturedAnimation).toBeFalsy();
+
+        $animate.enabled(pinHostElement, true);
+
+        $animate.addClass(animateElement, 'red');
+        $rootScope.$digest();
+        expect(capturedAnimation).toBeTruthy();
+
+        dealoc(pinElement);
+      });
+    });
   });
 
   describe('callbacks', function() {