fix($animateCss): avoid flicker caused by temporary classes

[Narretz]

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
bug fix

What is the current behavior? (You can also link to an open issue here)
See #14015

Does this PR introduce a breaking change?
No

Please check if the PR fulfills these requirements

• The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
• Tests for the changes have been added (for bug fixes / features)
• [ ] Docs have been added / updated (for bug fixes / features)

Other information:

[gkalpak]

I wish we could come up with a more straightforward fix. Adding, removing, re-adding temporary classes doesn't feel good (both from a maintenance and a robustness perspective).

AFAICT, tempClasses is an undocumented, private feature that is used for ngShow/ngHide only. From a quick look, it doesn't seem to be necessary to apply the classes in the beforeStart() function. What would happen if we only applied tempClasses once the animation actually started?

[Narretz]

tempClasses is undocumented, but supposed to be documented, see #9549
If tempClasses is removed, a bunch of tests fail that check if the tempClasses are on the element :D
There's definitely a specific reason why ngShow uses tempClasses - it must be in the tests or code somewhere. But maybe it's obsolete, I don't know.
Still, tempClasses should be able to trigger an animation same as any other class, so if we keep them we'd need to apply them before animation detection is done.

[gkalpak]

tempClasses is undocumented, but supposed to be documented

The important thing is that it is not documented right now 😁
So, we still have the chance to fix any issues and document the new, correct behavior.

--

There's definitely a specific reason why ngShow uses tempClasses - it must be in the tests or code somewhere. But maybe it's obsolete, I don't know.

The reason why ngShow/ngHide use tempClasses is that we don't want to apply the display: none style on .ng-hide while animating, but we can't use the "standard" ng-animate class either (see 39d0b36). So we need the temp ng-hide-animate class in order to be able to write .ng-hide:not(.ng-hide-animate) { ... }

To be clear, I am not proposing to ditch tempClasses altogether. I am just saying that we should postpone adding them until they are necessary (instead of adding/removing them repeatedly during an animation). I.e. only add them here.

--

Still, tempClasses should be able to trigger an animation same as any other class, so if we keep them we'd need to apply them before animation detection is done.

I am not sure I agree. (OK, actually I am sure I disagree 😛)
I don't see why they should be able to trigger an animation. As far as I understand, the purpose of tempClasses is to offer more expressiveness/flexibility in CSS selectors during the actual animation. There is no indication that they are intended to be able to trigger an animation (and I can't think of a good reason why they should).

BTW, the only relevant mention I could find is the ngShow docs (but not in ngHide! 😃 - I'll fix that):

Note that the selector that needs to be used is actually .ng-hide:not(.ng-hide-animate) to cope with extra animation classes that can be added.

[gkalpak]

A PR is worth a thousand words 😃
Here is what I had in mind: #15472

[Narretz]

This PR doesn't work unfortunately in all cases.