Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

revert(collapse): remove $animateCss #4650

Closed
wants to merge 1 commit into from

Conversation

Foxandxss
Copy link
Contributor

This reverts commit 6f9f1fc.

$animateCss is giving lots of side effects and I see no real reason to have it. Reading the official docs I can read:

$animateCss is designed to be used inside of a registered JavaScript animation that is powered by ngAnimate.

So it seems that using $animateCss here is a bit overkill and it is adding lot of weird complexity and issues. I tried to fix them, but only more issues were coming out. It is better to leave it as it was before because it works perfect with all the angular versions we support.

The majority of issues comes without ngAnimate activated

Let's see those side issues.

First one: If there is a hidden element on screen, $animateCss will computate its height:

Broken Plunker, no ngAnimate
Broken Plunker, ngAnimate
no $animateCss, no ngAnimate
no $animateCss, ngAnimate

Second one: I don't know how to explain this one, but with $animateCss, it is not showing the navbar correctly (just drag it for the responsive behavior):

Broken Plunker, no ngAnimate
(Works with ngAnimate)
no $animateCss, no ngAnimate
(Works the same with ngAnimate)

Third one: Nested accordions not being rendered correctly:

Broken Plunker, no ngAnimate
(Works with ngAnimate)
no $animateCss, no ngAnimate
(Works the same with ngAnimate)

So the biggest issues are when we don't use ngAnimate (people not caring about animations) and in some other cases even with Animation.

I will probably raise an issue at the angular repo, but even if it ends being a bug, we cannot force all the users to move to the fixed version (not everyone can update) and the fix could take weeks. So I propose to revert this and if someday we see a real reason to use $animateCss we will use it, but the docs suggest me that we are barking to the wrong tree with this implementation.

Fixes #4647
Fixes #4628
Fixes #4561

@Foxandxss
Copy link
Contributor Author

Well, I am not happy if I don't really find the source. I tried to reproduce it and it seems like it was fixed in 1.4.5. Still, there is an issue with the current implementation so I will open a new PR.

@Foxandxss Foxandxss closed this Oct 18, 2015
@Foxandxss Foxandxss deleted the fix/careousel branch October 23, 2015 23:58
@hthetiot
Copy link

thx, fixed my issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants