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

fix(carousel): disable transition until animation completes #3757

Closed

Conversation

wesleycho
Copy link
Contributor

  • Force the carousel indicator to not select the slide if the animation has not completed
  • Factor out goNext function to not redefine on each execution of select

Plunker of fix in action here.

This addresses #3729

@wesleycho wesleycho added this to the 0.13.1 (Performance) milestone Jun 4, 2015
@@ -1,6 +1,6 @@
<div ng-mouseenter="pause()" ng-mouseleave="play()" class="carousel" ng-swipe-right="prev()" ng-swipe-left="next()">
<ol class="carousel-indicators" ng-show="slides.length > 1">
<li ng-repeat="slide in slides | orderBy:'index' track by $index" ng-class="{active: isActive(slide)}" ng-click="select(slide)"></li>
<li ng-repeat="slide in slides | orderBy:'index' track by $index" ng-class="{active: isActive(slide)}" ng-click="!$currentTransition && select(slide)"></li>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this !$currentTransition be done inside the select() instead of explicitly having to set it within the ng-click?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm willing to make that change, but it should be noted that both the next and prev methods check that property as well, and if it is falsey, then select is called. Theoretically, I would prefer to have it be inside the function instead of being parsed by Angular, but then it requires thought about those methods. I could move the check into select instead and delete those lines from the next and prev functions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go for that indeed. Just keep the currentTransition variable as an internal flag like suggested also keeps the template cleaner.

We promote overwriting templates to implement custom designs/functionality. By keeping $currentTransition out of the template, there is one less variable to keep in mind that could potentially break a custom template.

- Force the carousel indicator to not select the slide if the animation has not completed
- Factor out `goNext` function to not redefine on each execution of `select`

chore(carousel): move `$currentTransition` check to `select` method
@wesleycho wesleycho closed this in ef45ecf Jun 7, 2015
bleggett pushed a commit to bleggett/bootstrap that referenced this pull request Jun 11, 2015
- Force the carousel indicator to not select the slide if the animation has not completed
- Factor out `goNext` function to not redefine on each execution of `select`

chore(carousel): move `$currentTransition` check to `select` method

Fixes angular-ui#3729
Closes angular-ui#3757
bleggett pushed a commit to bleggett/bootstrap that referenced this pull request Jun 11, 2015
- Force the carousel indicator to not select the slide if the animation has not completed
- Factor out `goNext` function to not redefine on each execution of `select`

chore(carousel): move `$currentTransition` check to `select` method

Fixes angular-ui#3729
Closes angular-ui#3757
@wesleycho wesleycho deleted the fix/carousel-indicators branch August 2, 2015 07:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants