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

fix(carousel): cancel goNext on scope destruction #1453

Closed
wants to merge 1 commit into from

Conversation

chrisirhc
Copy link
Contributor

The problem was that goNext be called after removeSlide when the slide's scope is destroyed. Since the destroy event is broadcasted, it fires on the parent before the child. As such what happens is:

  1. carousel's destroy event fires -> resetTimer()
  2. slide's destroy event fires -> ctrl.removeSlide(slide) -> ctrl.select(...) -> goNext() -> restartTimer()

Hence, the timer gets restarted by the slide's destroy event after the parent carousel's destroy event is fired.

Discovered after #1451

Note that the transition branch (if a transition is currently running and the scope is destroyed) isn't covered by the current tests but that will still work with this fix. Tests with transitions can get quite complicated because we don't have a helper to flush $transition events. However, I think this will be fixed very quickly once we switch over to ngAnimate. That's why I don't intend to complicate this code further as most of the code in the carousel will go away. For example, the whole goNext function will be gone if we can use the $animate service.

@pkozlowski-opensource
Copy link
Member

Landed as 7515df4, thnx!

@chrisirhc chrisirhc deleted the feature/carousel-leak branch December 24, 2013 22:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants