Skip to content

Commit

Permalink
Closes #21055: Prevents ScrollSpy from clearing active item when Safa…
Browse files Browse the repository at this point in the history
…ri rubberbands

When the rubberband effect causes Safari to scroll past the top of the
page, the value of scrollTop becomes negative. If the offset of the first
ScrollSpy target is 0 - essentially if the target is at the top of the
page - then ScrollSpy should not clear the active item. Conceptually, the
first item should remain active when rubberbanding past the top of the
page.

This commit fixes issue #21055 by verifying the first scrollspy target is
not at the top of the page before clearing the active nav-item.
  • Loading branch information
RobRuana committed Nov 1, 2016
1 parent 5616e62 commit c62e4b2
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
2 changes: 1 addition & 1 deletion js/src/scrollspy.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ const ScrollSpy = (($) => {
}
}

if (this._activeTarget && scrollTop < this._offsets[0]) {
if (this._activeTarget && scrollTop < this._offsets[0] && this._offsets[0] > 0) {
this._activeTarget = null
this._clear()
return
Expand Down
44 changes: 44 additions & 0 deletions js/tests/unit/scrollspy.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,50 @@ $(function () {
.scrollTop(201)
})

QUnit.test('should NOT clear selection if above the first section and first section is at the top', function (assert) {
assert.expect(4)
var done = assert.async()

var sectionHTML = '<div id="header" style="height: 500px;"></div>'
+ '<nav id="navigation" class="navbar">'
+ '<ul class="nav navbar-nav">'
+ '<li><a id="one-link" class="nav-link active" href="#one">One</a></li>'
+ '<li><a id="two-link" class="nav-link" href="#two">Two</a></li>'
+ '<li><a id="three-link" class="nav-link" href="#three">Three</a></li>'
+ '</ul>'
+ '</nav>'
$(sectionHTML).appendTo('#qunit-fixture')

var negativeHeight = -10
var startOfSectionTwo = 101

var scrollspyHTML = '<div id="content" style="height: 200px; overflow-y: auto;">'
+ '<div id="one" style="height: 100px;"/>'
+ '<div id="two" style="height: 100px;"/>'
+ '<div id="three" style="height: 100px;"/>'
+ '<div id="spacer" style="height: 100px;"/>'
+ '</div>'
var $scrollspy = $(scrollspyHTML).appendTo('#qunit-fixture')

$scrollspy
.bootstrapScrollspy({
target: '#navigation',
offset: $scrollspy.position().top
})
.one('scroll', function () {
assert.strictEqual($('.active').length, 1, '"active" class on only one element present')
assert.strictEqual($('.active').is('#two-link'), true, '"active" class on second section')
$scrollspy
.one('scroll', function () {
assert.strictEqual($('.active').length, 1, '"active" class on only one element present')
assert.strictEqual($('.active').is('#one-link'), true, '"active" class on first section')
done()
})
.scrollTop(negativeHeight)
})
.scrollTop(startOfSectionTwo)
})

QUnit.test('should correctly select navigation element on backward scrolling when each target section height is 100%', function (assert) {
assert.expect(5)
var navbarHtml =
Expand Down

0 comments on commit c62e4b2

Please sign in to comment.