Closes #21055: Prevents ScrollSpy from clearing active item when Safari 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.

diff --git a/js/src/scrollspy.js b/js/src/scrollspy.js
index 9b39acd3628ac613864c1427fe31273c60596c96..9cb1438ca79a12a0a3badd76da05f5c3e2d6c606 100644 (file)
--- a/js/src/scrollspy.js
+++ b/js/src/scrollspy.js
@@ -221,7 +221,7 @@ const ScrollSpy = (($) => {
         return
       }
 
-      if (this._activeTarget && scrollTop < this._offsets[0]) {
+      if (this._activeTarget && scrollTop < this._offsets[0] && this._offsets[0] > 0) {
         this._activeTarget = null
         this._clear()
         return

diff --git a/js/tests/unit/scrollspy.js b/js/tests/unit/scrollspy.js
index a349b5337ee2aafaf2a10ad2cddcc9b3abb82a51..877ec67a2abf3f8e5e7865048205df4a5d9b0128 100644 (file)
--- a/js/tests/unit/scrollspy.js
+++ b/js/tests/unit/scrollspy.js
@@ -287,6 +287,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 =