Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use more robust "find next carousel item" logic #14069

Merged
merged 1 commit into from
Jul 31, 2014
Merged

Use more robust "find next carousel item" logic #14069

merged 1 commit into from
Jul 31, 2014

Conversation

cvrebert
Copy link
Collaborator

@cvrebert cvrebert commented Jul 8, 2014

Fixes #13818 and adds a relevant additional test.
CC: @fat for review

@cvrebert cvrebert added the js label Jul 8, 2014
@cvrebert cvrebert added this to the v3.2.1 milestone Jul 8, 2014
@cvrebert
Copy link
Collaborator Author

cvrebert commented Jul 8, 2014

Travis failure is due to the unrelated transient IE8 issue that Jacob is gonna look into.

var delta = direction == 'prev' ? -1 : 1
var activeIndex = this.getItemIndex(active)
var itemIndex = (activeIndex + delta) % this.$items.length
return $(this.$items[itemIndex])
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about return this.$items.eq(itemIndex) here? (Could do the same thing here.)

@cvrebert
Copy link
Collaborator Author

@fat Ping.

@fat
Copy link
Member

fat commented Jul 31, 2014

lgtm

@fat
Copy link
Member

fat commented Jul 31, 2014

probably should squash these commits

Also adds another test for #9461: carousel next/prev should ignore non-items
@cvrebert
Copy link
Collaborator Author

Squashed.

cvrebert added a commit that referenced this pull request Jul 31, 2014
Use more robust "find next carousel item" logic
@cvrebert cvrebert merged commit f026cfb into master Jul 31, 2014
@cvrebert cvrebert deleted the fix-13818 branch July 31, 2014 18:36
@cvrebert cvrebert mentioned this pull request Jul 31, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

carousel prev selector
3 participants