Collapse broken in combination with ngRepeat/dynamic content (height 0), 0.7.0

[elgerlambert]

Hi guys,

This commit a72c635 as a response to issue #1222 has reintroduced a bug. The problem seems to be similar to issue #82 (the reason why the poor performance $watch function was initially introduced).

Here is a plunker that demonstrates the problem: http://plnkr.co/edit/SEKoUGWWTIf4qvB0l6KT?p=preview

I noticed an additional step is required to reproduce this issue through plunker. Open the plunker and before you continue, click into the input box and then out again.

Next, click back into the input and start typing. Notice how what you type doesn't appear (the problem, the collapsable element isn't changed to height: auto, instead it remains on height: 0). Now leave what you have written inside of the input box, click outside of it and back in. Now the content should appear. As long as you keep focus, it is even possible to clear the content and then have it reappear as soon as you start typing again. The difference, the collapsable element now has height: auto.

My use case -where I ran into this issue- is a search bar. When the user starts typing, a search query is sent to the server. The collapse is used to hide the results when the user click's out of the search bar.

Kind regards, Elger

[chrisirhc]

This happens because collapse is attempting to animate height 0 to height 0. The transitionend event doesn't fire because the height was already set to 0, no transition occurred. Height auto is only set at the end of the transition, which is never gets called.

AngularJS's solution to such situations in ngAnimate, and also in Twitter Bootstrap's JS is to set a fallback timer that fires after the length of time the animation was supposed to execute. The transitionend can cancel this timer if it occurs first. Cancellation of the transition can also cancel this timer.

From Twitter Bootstrap's CSS, this transition is 0.35s. I vote to fix this by adding this time as a constant. This means that if people customize the Bootstrap CSS to have a different time, it's going to look a little weird. I think that might be a small number of people though (do correct me if you think otherwise).

In the future, if we use ngAnimate for animations, this wouldn't be a problem.

[pkozlowski-opensource]

@elgerlambert it is fixed in master now: http://plnkr.co/edit/SEKoUGWWTIf4qvB0l6KT?p=preview

@chrisirhc took a different approach to quickly fix it order to not block a release. But I do agree that the current way of doing things is far from being robust so we should definitively look into ngAnimate as soon as NG1.2 support is out.