Merge cef7e253e4 into 4c98145482

js/src/scrollspy.js

@@ -40,10 +40,10 @@ const SELECTOR_DROPDOWN_TOGGLE = '.dropdown-toggle'
 
 const Default = {
   offset: null, // TODO: v6 @deprecated, keep it for backwards compatibility reasons
-  rootMargin: '0px 0px -25%',
+  rootMargin: '0px',
   smoothScroll: false,
   target: null,
-  threshold: [0.1, 0.5, 1]
+  threshold: [0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1]
 }
 
 const DefaultType = {
@@ -65,13 +65,13 @@ class ScrollSpy extends BaseComponent {
     // this._element is the observablesContainer and config.target the menu links wrapper
     this._targetLinks = new Map()
     this._observableSections = new Map()
+    this._intersectionRatio = new Map()
     this._rootElement = getComputedStyle(this._element).overflowY === 'visible' ? null : this._element
     this._activeTarget = null
     this._observer = null
-    this._previousScrollData = {
+    // Sometimes scrolling to the section isn't enough to trigger a observation callback.
-      visibleEntryTop: 0,
+    // Scroll to up to 2px to make sure it triggers.
-      parentScrollTop: 0
+    this._scrollOffset = 2
-    }
     this.refresh() // initialize
   }
 
@@ -115,7 +115,7 @@ class ScrollSpy extends BaseComponent {
     config.target = getElement(config.target) || document.body
 
     // TODO: v6 Only for backwards compatibility reasons. Use rootMargin only
-    config.rootMargin = config.offset ? `${config.offset}px 0px -30%` : config.rootMargin
+    config.rootMargin = config.offset ? `${config.offset}px 0px 0px` : config.rootMargin
 
     if (typeof config.threshold === 'string') {
       config.threshold = config.threshold.split(',').map(value => Number.parseFloat(value))
@@ -125,9 +125,7 @@ class ScrollSpy extends BaseComponent {
   }
 
   _maybeEnableSmoothScroll() {
-    if (!this._config.smoothScroll) {
+    const scrollBehavior = this._config.smoothScroll ? 'smooth' : 'auto'
-      return
-    }
 
     // unregister any previous listeners
     EventHandler.off(this._config.target, EVENT_CLICK)
@@ -137,9 +135,10 @@ class ScrollSpy extends BaseComponent {
       if (observableSection) {
         event.preventDefault()
         const root = this._rootElement || window
-        const height = observableSection.offsetTop - this._element.offsetTop
+
+        const height = observableSection.offsetTop - this._element.offsetTop - this._scrollOffset
         if (root.scrollTo) {
-          root.scrollTo({ top: height, behavior: 'smooth' })
+          root.scrollTo({ top: height, behavior: scrollBehavior })
           return
         }
 
@@ -161,40 +160,21 @@ class ScrollSpy extends BaseComponent {
 
   // The logic of selection
   _observerCallback(entries) {
-    const targetElement = entry => this._targetLinks.get(`#${entry.target.id}`)
+    for (const entry of entries) {
-    const activate = entry => {
+      this._intersectionRatio.set(`#${entry.target.id}`, entry.intersectionRatio)
-      this._previousScrollData.visibleEntryTop = entry.target.offsetTop
-      this._process(targetElement(entry))
     }
 
-    const parentScrollTop = (this._rootElement || document.documentElement).scrollTop
+    let maxIntersectionRatio = 0
-    const userScrollsDown = parentScrollTop >= this._previousScrollData.parentScrollTop
+    let element = null
-    this._previousScrollData.parentScrollTop = parentScrollTop
+    for (const [key, val] of this._intersectionRatio.entries()) {
-
+      if (val > maxIntersectionRatio) {
-    for (const entry of entries) {
+        element = this._targetLinks.get(key)
-      if (!entry.isIntersecting) {
+        maxIntersectionRatio = val
-        this._activeTarget = null
-        this._clearActiveClass(targetElement(entry))
-
-        continue
       }
+    }
 
-      const entryIsLowerThanPrevious = entry.target.offsetTop >= this._previousScrollData.visibleEntryTop
+    if (element !== null) {
-      // if we are scrolling down, pick the bigger offsetTop
+      this._process(element)
-      if (userScrollsDown && entryIsLowerThanPrevious) {
-        activate(entry)
-        // if parent isn't scrolled, let's keep the first visible item, breaking the iteration
-        if (!parentScrollTop) {
-          return
-        }
-
-        continue
-      }
-
-      // if we are scrolling up, pick the smallest offsetTop
-      if (!userScrollsDown && !entryIsLowerThanPrevious) {
-        activate(entry)
-      }
     }
   }
 
@@ -216,6 +196,7 @@ class ScrollSpy extends BaseComponent {
       if (isVisible(observableSection)) {
         this._targetLinks.set(decodeURI(anchor.hash), anchor)
         this._observableSections.set(anchor.hash, observableSection)
+        this._intersectionRatio.set(anchor.hash, 0)
       }
     }
   }

js/tests/unit/scrollspy.spec.js

@@ -451,7 +451,7 @@ describe('ScrollSpy', () => {
       })
     })
 
-    it('should clear selection if above the first section', () => {
+    it('should remember previous selection', () => {
       return new Promise(resolve => {
         fixtureEl.innerHTML = [
           '<div id="header" style="height: 500px;"></div>',
@@ -483,9 +483,10 @@ describe('ScrollSpy', () => {
           expect(spy).toHaveBeenCalled()
 
           expect(fixtureEl.querySelectorAll('.active')).toHaveSize(1)
-          expect(active().getAttribute('id')).toEqual('two-link')
+          expect(active().getAttribute('id')).toEqual('one-link')
           onScrollStop(() => {
-            expect(active()).toBeNull()
+            expect(fixtureEl.querySelectorAll('.active')).toHaveSize(1)
+            expect(active().getAttribute('id')).toEqual('one-link')
             resolve()
           }, contentEl)
           scrollTo(contentEl, 0)
@@ -842,37 +843,54 @@ describe('ScrollSpy', () => {
   })
 
   describe('SmoothScroll', () => {
-    it('should not enable smoothScroll', () => {
+    it('should not enable smoothScroll', done => {
       fixtureEl.innerHTML = getDummyFixture()
-      const offSpy = spyOn(EventHandler, 'off').and.callThrough()
-      const onSpy = spyOn(EventHandler, 'on').and.callThrough()
 
       const div = fixtureEl.querySelector('.content')
-      const target = fixtureEl.querySelector('#navBar')
+      const link = fixtureEl.querySelector('[href="#div-jsm-1"]')
-      // eslint-disable-next-line no-new
+      const observable = fixtureEl.querySelector('#div-jsm-1')
-      new ScrollSpy(div, {
+      const clickSpy = getElementScrollSpy(div)
-        offset: 1
+
+      const scrollSpy = new ScrollSpy(div, {
+        offset: 1,
+        smoothScroll: false
       })
 
-      expect(offSpy).not.toHaveBeenCalledWith(target, 'click.bs.scrollspy')
+      setTimeout(() => {
-      expect(onSpy).not.toHaveBeenCalledWith(target, 'click.bs.scrollspy')
+        if (div.scrollTo) {
+          expect(clickSpy).toHaveBeenCalledWith({ top: observable.offsetTop - div.offsetTop - scrollSpy._scrollOffset, behavior: 'auto' })
+        } else {
+          expect(clickSpy).toHaveBeenCalledWith(observable.offsetTop - div.offsetTop - scrollSpy._scrollOffset)
+        }
+
+        done()
+      }, 100)
+      link.click()
     })
 
-    it('should enable smoothScroll', () => {
+    it('should enable smoothScroll', done => {
       fixtureEl.innerHTML = getDummyFixture()
-      const offSpy = spyOn(EventHandler, 'off').and.callThrough()
-      const onSpy = spyOn(EventHandler, 'on').and.callThrough()
 
       const div = fixtureEl.querySelector('.content')
-      const target = fixtureEl.querySelector('#navBar')
+      const link = fixtureEl.querySelector('[href="#div-jsm-1"]')
-      // eslint-disable-next-line no-new
+      const observable = fixtureEl.querySelector('#div-jsm-1')
-      new ScrollSpy(div, {
+      const clickSpy = getElementScrollSpy(div)
+
+      const scrollSpy = new ScrollSpy(div, {
         offset: 1,
         smoothScroll: true
       })
 
-      expect(offSpy).toHaveBeenCalledWith(target, 'click.bs.scrollspy')
+      setTimeout(() => {
-      expect(onSpy).toHaveBeenCalledWith(target, 'click.bs.scrollspy', '[href]', jasmine.any(Function))
+        if (div.scrollTo) {
+          expect(clickSpy).toHaveBeenCalledWith({ top: observable.offsetTop - div.offsetTop - scrollSpy._scrollOffset, behavior: 'smooth' })
+        } else {
+          expect(clickSpy).toHaveBeenCalledWith(observable.offsetTop - div.offsetTop - scrollSpy._scrollOffset)
+        }
+
+        done()
+      }, 100)
+      link.click()
     })
 
     it('should not smoothScroll to element if it not handles a scrollspy section', () => {
@@ -925,17 +943,17 @@ describe('ScrollSpy', () => {
       const link = fixtureEl.querySelector('[href="#div-jsm-1"]')
       const observable = fixtureEl.querySelector('#div-jsm-1')
       const clickSpy = getElementScrollSpy(div)
-      // eslint-disable-next-line no-new
+
-      new ScrollSpy(div, {
+      const scrollSpy = new ScrollSpy(div, {
         offset: 1,
         smoothScroll: true
       })
 
       setTimeout(() => {
         if (div.scrollTo) {
-          expect(clickSpy).toHaveBeenCalledWith({ top: observable.offsetTop - div.offsetTop, behavior: 'smooth' })
+          expect(clickSpy).toHaveBeenCalledWith({ top: observable.offsetTop - div.offsetTop - scrollSpy._scrollOffset, behavior: 'smooth' })
         } else {
-          expect(clickSpy).toHaveBeenCalledWith(observable.offsetTop - div.offsetTop)
+          expect(clickSpy).toHaveBeenCalledWith(observable.offsetTop - div.offsetTop - scrollSpy._scrollOffset)
         }
 
         done()
@@ -959,17 +977,17 @@ describe('ScrollSpy', () => {
       const link = fixtureEl.querySelector('[href="#présentation"]')
       const observable = fixtureEl.querySelector('#présentation')
       const clickSpy = getElementScrollSpy(div)
-      // eslint-disable-next-line no-new
+
-      new ScrollSpy(div, {
+      const scrollSpy = new ScrollSpy(div, {
         offset: 1,
         smoothScroll: true
       })
 
       setTimeout(() => {
         if (div.scrollTo) {
-          expect(clickSpy).toHaveBeenCalledWith({ top: observable.offsetTop - div.offsetTop, behavior: 'smooth' })
+          expect(clickSpy).toHaveBeenCalledWith({ top: observable.offsetTop - div.offsetTop - scrollSpy._scrollOffset, behavior: 'smooth' })
         } else {
-          expect(clickSpy).toHaveBeenCalledWith(observable.offsetTop - div.offsetTop)
+          expect(clickSpy).toHaveBeenCalledWith(observable.offsetTop - div.offsetTop - scrollSpy._scrollOffset)
         }
 
         done()