css "transition" on repeated elements disables animation

[olostan]

There is quite unexpected behavior of $animate: if there is any "transition" present on element before adding/removing class, the animation is skipped.

This is caused by https://github.com/angular/angular.js/blob/master/src/ngAnimate/animate.js#L850
In this line there is a check if there is transition on the element before adding class. But this does not mean that this transition is currently in progress.

Here is common use case when this issue can cause big surprise for developers:

I have a list of elements that should be highlighted when hovered. If I put "transition" on element + use ngRepeat's animation (enter/leave/move).

Here is a plunker to illustrate that:
http://plnkr.co/edit/vI7LvG?p=preview

So currently the only workaround is to put "transitioned" elements inside elements that are repeated by ngRepeat

[matsko]

This is a big issue and with 1.2.1 that detection feature will be removed. The reason why this is in place is to allow class-based animations to do their thing. So with 1.2.1 the CSS class definition style will change and that way ngAnimate can go back to allowing transitions/animations on the base class. This way the animation code will be close to the same as without ngAnimate.

While this fix is easier than it sounds, it also effects the JS API. So both need to be changed. That's why it's scheduled for 1.2.1.

Release cycles will be faster once 1.2 is out.

[matsko]

The CSS structure will look like this.

.my-class { transition: 1s linear all; }
.my-class.ng-enter {}
.my-class.ng-enter-active {}
.my-class.ng-leave {}
.my-class.ng-leave-active {}
.my-class.other-class-add
.my-class.other-class {} //or .my-class.other-class-add-active {}
.my-class.other-class.other-class-remove {}
.my-class:not(.other-class) //or my-class.other-class-remove-active {}

[olostan]

@matsko wow... so there going to be a big change? Now we're developing prototype using 1.2.0 style of CSS-defined animations. Should we expect a big change so we will need to rewrite a lot?

The structure you've posted looks like it is now.. isn't it?

[matsko]

Yeah there's nothing much changing on the outside other than the cancelling transitions fix. The only difference is the way the timing of how the class-add-active and class-remove-active are applied.

[mgol]

I'm a little concerned about versioning here. It's not intuitive (and against semver) for the patch bump to introduce breaking changes.

[matsko]

It is unfortunate and I wouldn't introduce a breaking change like this, but it solves a lot more problems than it introduces. The restrictions on the animation skipping are causing a lot of debugging issues for developers and this is the best way to patch that.

Also the RC code is on unstable which means the API is still being figured out. 1.2 will be the new stable afterwards so the API will stay the same.

[mgol]

@matsko I know RC is unstable but you're speaking about switching to the new syntax in the 1.2.0 -> 1.2.1 upgrade, right? So it will be breaking semver.

Obviously, there are situations where breaking a rule will cause less problem than not doing that, it's not for me to decide if that's the case.

[matsko]

This syntax change will actually be present in 1.2 (any day now). The only change is how when the klass-add-active and klass-remove-active are applied. Everything else is same.

[mgol]

BTW, there are other even larger breaking changes planned for 1.2.1: #3969. IMO that's not good...

[mgol]

@matsko Ah, OK. That's good. :)

[olostan]

May be change 1.2.1 to 1.3 :)

[matsko]

@olostan can you please email me your email. I need your help to test this (the patch is ready).

[matsko]

Fixed on current master. Landed as 9d69a0a