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

TypeError in animateSetup on deletion of repeated element when using ng-include #4871

Closed
gregolsky opened this issue Nov 10, 2013 · 4 comments
Assignees
Milestone

Comments

@gregolsky
Copy link
Contributor

Plunkr: http://plnkr.co/edit/qMFvUdi1D5JQ8yEzSWvc?p=preview

The error occurs on click on one of the "Remove this" links.
I attach stack trace just in case.

TypeError: Cannot set property 'transitionProperty' of undefined
at animateSetup (http://code.angularjs.org/1.2.0/angular-animate.js:976:54)
at animateBefore (http://code.angularjs.org/1.2.0/angular-animate.js:1092:12)
at animate (http://code.angularjs.org/1.2.0/angular-animate.js:1112:37)
at Object.angular.module.config.$animateProvider.register.leave as before
at http://code.angularjs.org/1.2.0/angular-animate.js:680:33
at Array.forEach (native)
at q (http://code.angularjs.org/1.2.0/angular.min.js:7:261)
at invokeRegisteredAnimationFns (http://code.angularjs.org/1.2.0/angular-animate.js:665:11)
at performAnimation (http://code.angularjs.org/1.2.0/angular-animate.js:643:9)
at http://code.angularjs.org/1.2.0/angular-animate.js:394:13

This is very similar to #4548 .

The 'node' variable should be checked if its 'nodeType' property equals 1 (ELEMENT_NODE constant). Currently, it blows up because the node is a comment element in this case and does not have property 'style'.

@gregolsky
Copy link
Contributor Author

I used the solution I mentioned above. The exception is not thrown, however it looks like 'leave' animations do not work. I suspect they may not worked in the first place for this particular case.

      if (node.nodeType == ELEMENT_NODE) {
        node.style[TRANSITION_PROP + PROPERTY_KEY] = 'none';
      }

Here's working plunkr with the fix applied
http://plnkr.co/edit/GF2e3B1y58yw8Gx1YBzq?p=preview

I can create a pull request with this simple change, if this is what we want here (get rid of the error), but leave the 'leave' animations not working in this case. Perhaps we need to open another issue for it.
Thoughts?

gregolsky added a commit to gregolsky/angular.js that referenced this issue Nov 10, 2013
Fix TypeError in animateSetup on deletion of repeated element when using
ng-include. Added nodeType check to ensure we operate on element node.

Closes angular#4871
@ghost ghost assigned matsko Nov 22, 2013
@matsko
Copy link
Contributor

matsko commented Dec 4, 2013

@GregorL thank you for the commit, but this issue spans across all of ngAnimate/animate.js so I had to do a huge refactor. So I can't merge in your commit. But the issue is fixed: http://plnkr.co/edit/iVrtMJg92Iip9arTLXpx?p=preview

This will show up in 1.2.4. Thanks a bunch.

@gregolsky
Copy link
Contributor Author

Sure, I imagined the problem might be in more than one place. I'm glad it is fixed. Thanks!
However, the 'leave' animations don't seem to for me in your plunkr. Or perhaps I am using it in a wrong way? Should I open another ticket for it?

@matsko
Copy link
Contributor

matsko commented Dec 4, 2013

Ah right. Didn't notice that. It looks like both ngInclude and ngRepeat are competing for who gets to animate the leave operation. Normally this isn't a problem, but when a destructive DOM operation happens which removes the element before the next animation takes place, then this leads up to the next animation having no element to perform the animation on.

Please open an issue and mention me in there so I can make a new fix for that.

@matsko matsko closed this as completed Dec 4, 2013
matsko added a commit to matsko/angular.js that referenced this issue Dec 5, 2013
matsko added a commit that referenced this issue 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.

2 participants