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

Carousel still wraps even with wrap = false (on bootstrap 3.3.1) #15144

Closed
KostyaTretyak opened this issue Nov 15, 2014 · 3 comments · Fixed by #15152
Closed

Carousel still wraps even with wrap = false (on bootstrap 3.3.1) #15144

KostyaTretyak opened this issue Nov 15, 2014 · 3 comments · Fixed by #15152

Comments

@KostyaTretyak
Copy link

If I use https://maxcdn.bootstrapcdn.com/bootstrap/3.3.1/js/bootstrap.min.js
Carousel not stops with wrap = false

http://jsfiddle.net/ktretyak/nxaHg/21/

@KostyaTretyak KostyaTretyak changed the title Carousel not stops with wrap = true (on bootstrap 3.3.1) Carousel not stops with wrap = false (on bootstrap 3.3.1) Nov 15, 2014
@juthilo juthilo added the js label Nov 15, 2014
@cvrebert cvrebert added this to the v3.3.2 milestone Nov 16, 2014
@cvrebert
Copy link
Collaborator

This was probably broken by #14069 since getItemForDirection(), unlike the previous code, always produces an item, which means that

if (!$next.length) {
is dead code, and that's the only place wrap is used.
This wasn't caught because we lack a unit test for wrap: false 😞
If we had code coverage tracking, that would also have caught this.

@cvrebert cvrebert changed the title Carousel not stops with wrap = false (on bootstrap 3.3.1) Carousel still wraps even with wrap = false (on bootstrap 3.3.1) Nov 17, 2014
@austb25
Copy link

austb25 commented Nov 21, 2014

Hi,
I just updated my carousel.js to the latest (that was updated 4 days ago) and upgraded to Bootstrap 3.3.1 and I am still having this issue

@cvrebert
Copy link
Collaborator

@austb25 The fix is b7398bc which will be in v3.3.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants