]> git.ipfire.org Git - thirdparty/bootstrap.git/commitdiff
Handle multiple zero-offset Scrollspy elements. 15593/head
authorCaden Lovelace <caden@herostrat.us>
Sat, 17 Jan 2015 01:14:22 +0000 (01:14 +0000)
committerCaden Lovelace <caden@herostrat.us>
Sun, 1 Mar 2015 23:55:39 +0000 (23:55 +0000)
When the first two elements in a scrollspy content block have a document
offset of zero (i.e. they're hard against the top of the page),
Scrollspy would switch between them on every scroll event.

This could happen, for example, in a system of nested sections:

```
<section id="animals">
  <section id="dogs">
Content
  </section>
</section>
```

This ocurred because Scrollspy's check to see if it's at the end of the
array of sections uses `!arr[index]`. This misses the case where
`arr[index]` does exist and is zero.

This commit explicitly checks the array bounds.

js/scrollspy.js
js/tests/unit/scrollspy.js

index a72bb5350e63e50b0b3fc9b7cbf4da5bade9998d..9b29edf490f1ea597712f25ca65b17a2a93f5d2c 100644 (file)
@@ -96,7 +96,7 @@
     for (i = offsets.length; i--;) {
       activeTarget != targets[i]
         && scrollTop >= offsets[i]
-        && (!offsets[i + 1] || scrollTop <= offsets[i + 1])
+        && (offsets[i + 1] === undefined || scrollTop <= offsets[i + 1])
         && this.activate(targets[i])
     }
   }
index 41d82e0e538343d78d1682e5e9bc97f3d77df010..0fca144b6584d973b8ccee62626278b8f31993b7 100644 (file)
@@ -142,6 +142,44 @@ $(function () {
       .then(function () { return testElementIsActiveAfterScroll('#li-2', '#div-2') })
   })
 
+  QUnit.test('should add the active class correctly when there are nested elements at 0 scroll offset', function (assert) {
+    var times = 0
+    var done = assert.async()
+    var navbarHtml = '<nav id="navigation" class="navbar">'
+      + '<ul class="nav">'
+      + '<li id="li-1"><a href="#div-1">div 1</a>'
+      + '<ul>'
+      + '<li id="li-2"><a href="#div-2">div 2</a></li>'
+      + '</ul>'
+      + '</li>'
+      + '</ul>'
+      + '</nav>'
+
+    var contentHtml = '<div class="content" style="position: absolute; top: 0px; overflow: auto; height: 50px">'
+      + '<div id="div-1" style="padding: 0; margin: 0">'
+      + '<div id="div-2" style="height: 200px; padding: 0; margin: 0">div 2</div>'
+      + '</div>'
+      + '</div>'
+
+    $(navbarHtml).appendTo('#qunit-fixture')
+
+    var $content = $(contentHtml)
+      .appendTo('#qunit-fixture')
+      .bootstrapScrollspy({ offset: 0, target: '#navigation' })
+
+    !function testActiveElements() {
+      if (++times > 3) return done()
+
+      $content.one('scroll', function () {
+        assert.ok($('#li-1').hasClass('active'), 'nav item for outer element has "active" class')
+        assert.ok($('#li-2').hasClass('active'), 'nav item for inner element has "active" class')
+        testActiveElements()
+      })
+
+      $content.scrollTop($content.scrollTop() + 10)
+    }()
+  })
+
   QUnit.test('should clear selection if above the first section', function (assert) {
     var done = assert.async()