angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.86k stars 27.52k forks source link

(1.2 regression) ngAnimate clobbers ngClass #4949

Closed geddski closed 10 years ago

geddski commented 10 years ago

@IgorMinar @petebacondarwin @matsko I was able to reproduce the issue finally: http://plnkr.co/edit/I57CKjdHBZeqEHtZ4Bn9?p=preview

In this example, ngClass applies 3 classes to the element. After a two second timeout, ngClass removes one of the classes. But if ngAnimate is declared as a dependency, ngAnimate removes all 3 classes.

But only if any of those classes has a transition duration.

The place where this happens is during ngAnimate's cleanup phase: element.removeData(NG_ANIMATE_STATE)

This was maddening to track down :)

matsko commented 10 years ago

Great find! This was actually caused by a race condition with the animation between addClass and removeClass. $compile likes to remove ALL classes and then add back ALL classes again during whenever ngClass changes (depending on the order of the dynamic classes). This caused the animation to cancel twice and therefore you got removeClass -> addClass -> removeClass.

jeffwhelpley commented 10 years ago

Wow. I was just banging my head against the wall for the past two days on this one. Thanks @geddski for tracking this down. @matsko, thanks for responding to this so quickly.

mdedetrich commented 10 years ago

I can confirm that this is currently breaking my CSS3 animations that don't use the ngAnimate module (I have a node that uses the ngClass directive to set a class, and a simple CSS3 animation that is set up in a similar way as is here on the documentation http://docs.angularjs.org/api/ng.directive:ngClass).

The only difference is that the variable bound to ngClass is changed via a controller, and not through a click even. In any case, this behavior worked in rc.3 and broke in release (1.2.0 and 1.2.1)

matsko commented 10 years ago

There are two commits in the PR that I'm awaiting to get in. @mdedetrich did you test on both of them?

mdedetrich commented 10 years ago

@matsko After having a look at the CSS, its working with the latest 1.2.3 after some adjustments, it broke most likely due to changes made from rc.3 to release

oisinlavery commented 10 years ago

+1