ngClass + ngAnimate fixes #4958
Conversation
|
LGTM. Here is a slight change to @geddski's demo that fails with 1.2.0: http://plnkr.co/edit/Q6IrptbKTegTPbofyFYS?p=preview |
|
lgtm |
|
I think that is a go-ahead to merge? |
|
Not yet. Waiting for Vojta to let me know about the ngClass fix. |
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
|
@IgorMinar @petebacondarwin there's a second commit. Please review. |
ngClass works by removing all the former classes and then adding all the new classes to the element during each watch change operation. This may cause transition animations to never render. The ngClass directive will now only add and remove the classes that change during each watch operation. Closes angular#4960
| @@ -677,8 +677,8 @@ function $CompileProvider($provide) { | |||
| if(key == 'class') { | |||
| value = value || ''; | |||
| var current = this.$$element.attr('class') || ''; | |||
| this.$removeClass(tokenDifference(current, value).join(' ')); | |||
| this.$addClass(tokenDifference(value, current).join(' ')); | |||
| this.$removeClass(tokenDifference(current, value)); | |||
There was a problem hiding this comment.
I think here is a problem - you are comparing to current, which is the current value of the class on the element. We need to however compare to the previous value set by this particular component, which means that previous value has to be passed as second argument to $set().
Say, interpolation (class="{{cls}}") calls set "a b c".
Then, ng-class="xxx" calls set to "d".
This will remove "a b c" and add "d", which is not correct.
I suggested a solution in #4960 (comment)
|
Replaced by: #5015 @vojtajina me and @IgorMinar came up with a solution that fixes the problem that you addressed. |
These fixes ensure that ngClass doesn't abuse the addClass/removeClass DOM operations (which cause ngAnimate to mess up).
Closes #4949
Closes #4960