From a1aa0f8afdc33403604e5415aed649e605e93efd Mon Sep 17 00:00:00 2001 From: Caden Lovelace Date: Sat, 17 Jan 2015 01:14:22 +0000 Subject: [PATCH] Handle multiple zero-offset Scrollspy elements. 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: ```
Content
``` 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 | 2 +- js/tests/unit/scrollspy.js | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/js/scrollspy.js b/js/scrollspy.js index a72bb5350e..9b29edf490 100644 --- a/js/scrollspy.js +++ b/js/scrollspy.js @@ -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]) } } diff --git a/js/tests/unit/scrollspy.js b/js/tests/unit/scrollspy.js index 41d82e0e53..0fca144b65 100644 --- a/js/tests/unit/scrollspy.js +++ b/js/tests/unit/scrollspy.js @@ -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 = '' + + var contentHtml = '
' + + '
' + + '
div 2
' + + '
' + + '
' + + $(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()