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

ngAnimate throws errors (leaves stale items) for ngRepeat with nested ngInclude #4716

Closed
alexjc opened this issue Oct 30, 2013 · 8 comments
Closed
Assignees
Milestone

Comments

@alexjc
Copy link

alexjc commented Oct 30, 2013

The title describes the bug well enough I think, and here's my local hack to fix it:

index 64be7ba..7ef8d6d 100644
--- a/src/ngAnimate/animate.js
+++ b/src/ngAnimate/animate.js
@@ -256,6 +256,8 @@ angular.module('ngAnimate', ['ng'])
     var selectors = $animateProvider.$$selectors;

     var ELEMENT_NODE = 1;
+    var ELEMENT_DOCUMENTATION = 8;
+
     var NG_ANIMATE_STATE = '$$ngAnimateState';
     var NG_ANIMATE_CLASS_NAME = 'ng-animate';
     var rootAnimateState = {disabled:true};
@@ -883,6 +885,13 @@ angular.module('ngAnimate', ['ng'])
           startTime = Date.now(),
           node = element[0];

+          // When using ng-include within an ng-repeat, it's not always the first
+          // child element, that's a comment inserted by ng-include.  In the case
+          // if this node is a comment, get the next one.
+          if (node.nodeType == ELEMENT_DOCUMENTATION) {
+            node = element[1];
+          }
+
       //temporarily disable the transition so that the enter styles
       //don't animate twice (this is here to avoid a bug in Chrome/FF).
       if(timings.transitionDuration > 0) {

If this looks sensible to you let me know and I can submit a pull request. If not, let me know ;-)

Alex

@wesleycho
Copy link
Contributor

If it is a comment though, couldn't it be something from ng-if, or ng-repeat?

@alexjc
Copy link
Author

alexjc commented Oct 31, 2013

Yes, ng-repeat uses comments heavily but it's not the problem. I have an ng-repeat on its own elsewhere and obviously it works -- or it would have been reported already :-).

For me ng-include leaves comments like this:

<!-- ngInclude:  -->

Having that within the repeater, on the same div, confuses ngAnimation. It's the fix I submitted, you need to get the next element to have the actual ng-included item.

(It's my first bug report / fix here, I hope I'm making sense.)

@matsko
Copy link
Contributor

matsko commented Oct 31, 2013

Can you provide a plunkr please? I responded to an issue earlier today which may be related to this.

@alexjc
Copy link
Author

alexjc commented Nov 3, 2013

http://plnkr.co/edit/sVhmdeqsVx3XfiNCbtS8

Sorry for the delay, it took a bit of time to set up a minimal example. Click on the button and watch the console for the exception.

EDIT: There were two bugs I had to fix in RC3 to get it working, but in the trunk of AngularJS you already fixed one. You may have to use that fix and the latest code to get to the second -- posted above.

@matsko
Copy link
Contributor

matsko commented Nov 3, 2013

What is the other bug? The comment exception one was fixed with: b9557b0

@alexjc
Copy link
Author

alexjc commented Nov 4, 2013

The other bug is fixed by my diff above.

@matsko
Copy link
Contributor

matsko commented Nov 4, 2013

OK cool. I see what's going on. I'm working on a bigger commit so I'll add that one in.

@matsko
Copy link
Contributor

matsko commented Dec 4, 2013

matsko added a commit to matsko/angular.js that referenced this issue Dec 5, 2013
@matsko matsko closed this as completed in 958d3d5 Dec 5, 2013
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants