Merge pull request #14069 from twbs/fix-13818

Use more robust "find next carousel item" logic
twbs · Jul 31, 2014 · f026cfb · f026cfb
2 parents d38fd02 + cbba8e5
commit f026cfb
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 3 deletions.
diff --git a/js/carousel.js b/js/carousel.js
@@ -63,6 +63,13 @@
     return this.$items.index(item || this.$active)
   }
 
+  Carousel.prototype.getItemForDirection = function (direction, active) {
+    var delta = direction == 'prev' ? -1 : 1
+    var activeIndex = this.getItemIndex(active)
+    var itemIndex = (activeIndex + delta) % this.$items.length
+    return this.$items.eq(itemIndex)
+  }
+
   Carousel.prototype.to = function (pos) {
     var that        = this
     var activeIndex = this.getItemIndex(this.$active = this.$element.find('.item.active'))
@@ -72,7 +79,7 @@
     if (this.sliding)       return this.$element.one('slid.bs.carousel', function () { that.to(pos) }) // yes, "slid"
     if (activeIndex == pos) return this.pause().cycle()
 
-    return this.slide(pos > activeIndex ? 'next' : 'prev', $(this.$items[pos]))
+    return this.slide(pos > activeIndex ? 'next' : 'prev', this.$items.eq(pos))
   }
 
   Carousel.prototype.pause = function (e) {
@@ -100,7 +107,7 @@
 
   Carousel.prototype.slide = function (type, next) {
     var $active   = this.$element.find('.item.active')
-    var $next     = next || $active[type]()
+    var $next     = next || this.getItemForDirection(type, $active)
     var isCycling = this.interval
     var direction = type == 'next' ? 'left' : 'right'
     var fallback  = type == 'next' ? 'first' : 'last'

diff --git a/js/tests/unit/carousel.js b/js/tests/unit/carousel.js
@@ -349,7 +349,7 @@ $(function () {
     $carousel.remove()
   })
 
-  test('should skip over non-items', function () {
+  test('should skip over non-items when using item indices', function () {
     var templateHTML = '<div id="myCarousel" class="carousel" data-interval="1814">'
         + '<div class="carousel-inner">'
         + '<div class="item active">'
@@ -373,4 +373,29 @@ $(function () {
 
     strictEqual($template.find('.item')[1], $template.find('.active')[0], 'second item active')
   })
+
+  test('should skip over non-items when using next/prev methods', function () {
+    var templateHTML = '<div id="myCarousel" class="carousel" data-interval="1814">'
+        + '<div class="carousel-inner">'
+        + '<div class="item active">'
+        + '<img alt="">'
+        + '</div>'
+        + '<script type="text/x-metamorph" id="thingy"/>'
+        + '<div class="item">'
+        + '<img alt="">'
+        + '</div>'
+        + '<div class="item">'
+        + '</div>'
+        + '</div>'
+        + '</div>'
+    var $template = $(templateHTML)
+
+    $template.bootstrapCarousel()
+
+    strictEqual($template.find('.item')[0], $template.find('.active')[0], 'first item active')
+
+    $template.bootstrapCarousel('next')
+
+    strictEqual($template.find('.item')[1], $template.find('.active')[0], 'second item active')
+  })
 })