Fix collapse animation

[inukshuk]

This is a tricky one. If you look at TWBS js, you will notice that the class in is added after the expand transition is complete and removed as the collapse transition starts This is particularly confusing because it works differently than .fade.in for example and it differs slightly from our current behaviour of adding the in class using $animate which adds the class immediately at the start of the transition.

This discrepancy is not visible in the collapse example on the homepage which opens and closes a well, but it is visible, for example when collapsing a navbar (mobile version) -- a very common use case in TWBS version 3. @flachware adjusted the Plunker example accordingly, and, when you open/close the menu, you will see a scrollbar during the expand animation (depending on your browser/OS handling of scrollbars obviously).

Now, this is isn't a big deal, and can be worked around by inserting CSS like this .navbar-collapse.collapsing.in { overflow: hidden; }, but if ui-bootstrap is supposed to just work out of the box given the official Bootstrap CSS, I think we should emulate the behaviour of Bootstrap JS as closely as possible. This PR tries to do just that by adding the in class in expandDone and removing it at the start of the collapsing transition. If you would prefer a different solution just let me know!

[inukshuk]

(Will look at the failing tests if you agree that we should indeed adjust the behaviour)

[Foxandxss]

If it is for the better and it doesn't hurt anybody, I am OK with it.

[wesleycho]

This is a quite interesting find, but I'm not sure these changes fixes this either - see this fork based on your changes here

[inukshuk]

@wesleycho if you change the name of the directive to uib-collapse it works, though, or am I missing something?

[inukshuk]

I updated the plunker here with the correct directive name -- there you should see that the scrollbar is not visible.

[wesleycho]

Ah, that is correct, I forgot about that!

Can you look into fixing the tests? This is a good fix.

[inukshuk]

@wesleycho thanks! I looked at the tests and noticed two separate issues. The big one is that $animate.flush does not seem to work as expected when $animateCss is used; in fact, when using $animateCss the tests only seem to work by accident right now. The problem is that the callbacks expandDone collapseDone are not called after $animate.flush() when the animation is set up using $animateCss -- if I don't use $animateCss then all is fine, the callbacks are called after flush and the tests are green.

Are you aware of this issue? I have not dug into $animateCss so just asking to see if you have any pointers for me. Thanks!

[icfantv]

IIRC, angular 1.4.5 introduced an issue with $animate and we had to change all our tests to accommodate. It would not surprise me if $animate.flush() didn't work as expected.

[wesleycho]

The test failure is an angular bug - $animate in ngMock needs to flush all $animateCss animations as well.

[bnjasim]

Could you please look into this issue: http://stackoverflow.com/questions/33225665/why-does-finally-and-done-of-animatecss-behave-differently-in-ui-bootstrap-angu

[Foxandxss]

I went to change it to finally because with .done it was never triggering a $digest so the collapse wasnt working properly without ngAnimate. With finally it works correctly both without ngAnimate and with ngAnimate. There was a bug with $animateCss that got resolved with 1.4.5.

What you see in our demos is using done and not finally, the finally is still on master and not in the demos.

So, is there any problem or there is something wrong? I would like to fix it if my solution wasn't the best one.

[Foxandxss]

I have to admit that animation in angular is a bit weird so it wouldnt be weird if I missed something important here.

[bnjasim]

Hey, Thanks for the prompt reply. I was implementing a sliding panel from left (for mobile displays) tweaking the collapse code. I guess I will go with basic css3 animation for the time being.

[icfantv]

@bnjasim please do not use the comment section on PRs (or the issues forum) for support related requests. Rather, please follow the instructions here.