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

(1.2 regression) ngAnimate clobbers ngClass #4949

Closed
geddski opened this issue Nov 14, 2013 · 6 comments
Closed

(1.2 regression) ngAnimate clobbers ngClass #4949

geddski opened this issue Nov 14, 2013 · 6 comments

Comments

@geddski
Copy link
Contributor

geddski commented Nov 14, 2013

@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
Copy link
Contributor

matsko commented Nov 14, 2013

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
Copy link

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.

matsko added a commit to matsko/angular.js that referenced this issue Nov 15, 2013
Depending on the animations placed on ngClass, the DOM operation may
run twice causing a race condition between addClass and removeClass.
Depending on what classes are removed and added via $compile this may
cause all CSS classes to be removed accidentally from the element
being animated.

Closes angular#4949
@mdedetrich
Copy link

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
Copy link
Contributor

matsko commented Nov 17, 2013

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

@matsko matsko mentioned this issue Nov 19, 2013
@matsko matsko closed this as completed in 7067a8f Nov 20, 2013
@mdedetrich
Copy link

@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

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
Depending on the animations placed on ngClass, the DOM operation may
run twice causing a race condition between addClass and removeClass.
Depending on what classes are removed and added via $compile this may
cause all CSS classes to be removed accidentally from the element
being animated.

Closes angular#4949
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
Depending on the animations placed on ngClass, the DOM operation may
run twice causing a race condition between addClass and removeClass.
Depending on what classes are removed and added via $compile this may
cause all CSS classes to be removed accidentally from the element
being animated.

Closes angular#4949
@oisinlavery
Copy link

+1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.