Skip to content

Commit

Permalink
better targets handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@GeoSot
GeoSot committed Jun 4, 2021
1 parent a93ae7e commit f397a43
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 48 deletions.
56 changes: 31 additions & 25 deletions js/src/scrollspy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* --------------------------------------------------------------------------
*/

import { defineJQueryPlugin, isElement, typeCheckConfig } from './util/index'
import { defineJQueryPlugin, isElement, reflow, typeCheckConfig } from './util/index'
import EventHandler from './dom/event-handler'
import Manipulator from './dom/manipulator'
import SelectorEngine from './dom/selector-engine'
Expand All @@ -24,7 +24,7 @@ const DATA_API_KEY = '.data-api'

const Default = {
target: null,
rootMargin: '0px 0px -50%'
rootMargin: '0px'
}

const DefaultType = {
Expand All @@ -41,6 +41,7 @@ const CLASS_NAME_ACTIVE = 'active'
const SELECTOR_DATA_SPY = '[data-bs-spy="scroll"]'
const SELECTOR_NAV_LIST_GROUP = '.nav, .list-group'
const SELECTOR_NAV_LINKS = '.nav-link'
const SELECTOR_NAV_ITEMS = '.nav-item'
const SELECTOR_LIST_ITEMS = '.list-group-item'
const SELECTOR_DROPDOWN = '.dropdown'
const SELECTOR_DROPDOWN_TOGGLE = '.dropdown-toggle'
Expand All @@ -57,15 +58,13 @@ class ScrollSpy extends BaseComponent {

// this._element is the observablesContainer
this._config = this._getConfig(config)
this._targetsContainer = this._config.target
this._target = this._config.target

if (!isElement(this._targetsContainer)) {
if (!isElement(this._target)) {
throw new TypeError('Target Container is not defined')
// this.dispose()
// return
}

this._targets = []
this._targetLinks = []
this._activeTarget = null
this._observableSections = []
this._observer = null
Expand All @@ -86,14 +85,15 @@ class ScrollSpy extends BaseComponent {

refresh() {
// `${SELECTOR_NAV_LINKS}, ${SELECTOR_LIST_ITEMS}, .${CLASS_NAME_DROPDOWN_ITEM}`
this._targets = SelectorEngine
.find('[href]', this._targetsContainer)
this._targetLinks = SelectorEngine
.find('[href]', this._target)
.filter(el => el.hash.length > 0)// ensure that all have id

this._observableSections = this._targets
this._observableSections = this._targetLinks
.map(el => SelectorEngine.findOne(el.hash, this._element))
.filter(el => el)// filter nulls

reflow(this._element)
if (this._observer) {
this._observer.disconnect()
} else {
Expand Down Expand Up @@ -122,10 +122,6 @@ class ScrollSpy extends BaseComponent {
config.target = SelectorEngine.findOne(config.target)
}

// if (!config.target.id) {
// config.target.id = getUID(NAME)
// }

typeCheckConfig(NAME, config, DefaultType)

return config
Expand Down Expand Up @@ -157,11 +153,11 @@ class ScrollSpy extends BaseComponent {
.forEach(item => item.classList.add(CLASS_NAME_ACTIVE))

// Handle special case when .nav-link is inside .nav-item
// SelectorEngine.prev(listGroup, SELECTOR_NAV_ITEMS)
// .forEach(navItem => {
// SelectorEngine.children(navItem, SELECTOR_NAV_LINKS)
// .forEach(item => item.classList.add(CLASS_NAME_ACTIVE))
// })
SelectorEngine.prev(listGroup, SELECTOR_NAV_ITEMS)
.forEach(navItem => {
SelectorEngine.children(navItem, SELECTOR_NAV_LINKS)
.forEach(item => item.classList.add(CLASS_NAME_ACTIVE))
})
})
}

Expand All @@ -171,19 +167,29 @@ class ScrollSpy extends BaseComponent {
}

_clearActiveClass() {
SelectorEngine.find(`.${CLASS_NAME_ACTIVE}`, this._targetsContainer)
SelectorEngine.find(`.${CLASS_NAME_ACTIVE}`, this._target)
.forEach(node => node.classList.remove(CLASS_NAME_ACTIVE))
}

_getNewObserver() {
// IntersectionObserver triggers and return element only when is shown and when is gone (threshold: 0). So we keep visible elements here
const visibleEntries = new Map()

const callback = entries => {
const entry = entries
.filter(el => el.isIntersecting)
.sort((a, b) => (a.intersectionRect.height - b.intersectionRect.height))
entries.forEach(entry => {
const key = entry.target.id
if (entry.isIntersecting || entry.intersectionRatio > 0.5) {
visibleEntries.set(key, entry.target.offsetTop)
} else {
visibleEntries.delete(key)
}
})
const firstVisible = [...visibleEntries.entries()]
.sort((a, b) => a[1] - b[1])
.shift()

if (entry) {
this._activate(this._targets.find(el => el.hash === `#${entry.target.id}`))
if (firstVisible) {
this._activate(this._targetLinks.find(el => el.hash === `#${firstVisible[0]}`))
}
}

Expand Down
95 changes: 72 additions & 23 deletions js/tests/unit/scrollspy.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import ScrollSpy from '../../src/scrollspy'
import Manipulator from '../../src/dom/manipulator'

/** Test helpers */
import { clearFixture, getFixture, jQueryMock } from '../helpers/fixture'
import { clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture'

describe('ScrollSpy', () => {
let fixtureEl
Expand All @@ -25,7 +25,7 @@ describe('ScrollSpy', () => {
const target = fixtureEl.querySelector(targetSelector)

// add top padding to fix Chrome on Android failures
const paddingTop = 1
const paddingTop = 5
const scrollHeight = Math.ceil(contentEl.scrollTop + Manipulator.position(target).top) + paddingTop

function listener() {
Expand Down Expand Up @@ -100,7 +100,7 @@ describe('ScrollSpy', () => {
target: '#navigation'
})

expect(scrollSpy._targets.length).toEqual(2)
expect(scrollSpy._targetLinks.length).toEqual(2)
})

it('should only switch "active" class on current target', done => {
Expand Down Expand Up @@ -141,8 +141,7 @@ describe('ScrollSpy', () => {
done()
})

scrollSpyEl.scrollIntoView(true)
scrollSpyEl.querySelector('#detail').scrollIntoView(true)
scrollSpyEl.scrollTop = 350
})

it('should only switch "active" class on current target specified w element', done => {
Expand Down Expand Up @@ -183,8 +182,44 @@ describe('ScrollSpy', () => {
done()
})

scrollSpyEl.scrollIntoView(true)
scrollSpyEl.querySelector('#detail').scrollIntoView(true)
scrollSpyEl.scrollTop = 350
})

it('should correctly select middle navigation option when large offset is used', done => {
fixtureEl.innerHTML = [
'<div id="header" style="height: 500px;"></div>',
'<nav id="navigation" class="navbar">',
' <ul class="navbar-nav">',
' <li class="nav-item"><a class="nav-link active" id="one-link" href="#one">One</a></li>',
' <li class="nav-item"><a class="nav-link" id="two-link" href="#two">Two</a></li>',
' <li class="nav-item"><a class="nav-link" id="three-link" href="#three">Three</a></li>',
' </ul>',
'</nav>',
'<div id="content" style="height: 200px; overflow-y: auto;">',
' <div id="one" style="height: 500px;"></div>',
' <div id="two" style="height: 300px;"></div>',
' <div id="three" style="height: 10px;"></div>',
'</div>'
].join('')

const contentEl = fixtureEl.querySelector('#content')
const offsetTop = Manipulator.position(contentEl).top
const scrollSpy = new ScrollSpy(contentEl, {
target: '#navigation',
rootMargin: `${offsetTop} 0px 0px`
})

spyOn(scrollSpy, '_process').and.callThrough()

contentEl.addEventListener('scroll', () => {
expect(fixtureEl.querySelector('#one-link').classList.contains('active')).toEqual(false)
expect(fixtureEl.querySelector('#two-link').classList.contains('active')).toEqual(true)
expect(fixtureEl.querySelector('#three-link').classList.contains('active')).toEqual(false)
expect(scrollSpy._process).toHaveBeenCalled()
done()
})

contentEl.scrollTop = 550
})

it('should add the active class to the correct element', done => {
Expand All @@ -195,7 +230,7 @@ describe('ScrollSpy', () => {
' <li class="nav-item"><a class="nav-link" id="a-2" href="#div-2">div 2</a></li>',
' </ul>',
'</nav>',
'<div class="content" style="position:relative; overflow: auto; height: 50px">',
'<div class="content" style="overflow: auto; height: 50px">',
' <div id="div-1" style="height: 100px; padding: 0; margin: 0">div 1</div>',
' <div id="div-2" style="height: 200px; padding: 0; margin: 0">div 2</div>',
'</div>'
Expand Down Expand Up @@ -234,7 +269,7 @@ describe('ScrollSpy', () => {
' <a class="nav-link" id="a-2" href="#div-2">div 2</a>',
' </nav>',
'</nav>',
'<div class="content" style="position:relative; overflow: auto; height: 50px">',
'<div class="content" style="overflow: auto; height: 50px">',
' <div id="div-1" style="height: 100px; padding: 0; margin: 0">div 1</div>',
' <div id="div-2" style="height: 200px; padding: 0; margin: 0">div 2</div>',
'</div>'
Expand Down Expand Up @@ -274,7 +309,7 @@ describe('ScrollSpy', () => {
' <a class="list-group-item" id="a-2" href="#div-2">div 2</a>',
' </div>',
'</nav>',
'<div class="content" style="position:relative; overflow: auto; height: 50px">',
'<div class="content" style="overflow: auto; height: 50px">',
' <div id="div-1" style="height: 100px; padding: 0; margin: 0">div 1</div>',
' <div id="div-2" style="height: 200px; padding: 0; margin: 0">div 2</div>',
'</div>'
Expand Down Expand Up @@ -328,12 +363,11 @@ describe('ScrollSpy', () => {
target: '#navigation'
})

const active = fixtureEl.querySelector('.active')

fixtureEl.querySelector('#two').scrollIntoView(true)
const active = () => fixtureEl.querySelector('.active')

contentEl.scrollTop = 300
expect(fixtureEl.querySelectorAll('.active').length).toEqual(1)
expect(active.getAttribute('id')).toEqual('two-link')
expect(active().getAttribute('id')).toEqual('two-link')

fixtureEl.querySelector('#spacer').scrollIntoView(true)

Expand All @@ -351,20 +385,20 @@ describe('ScrollSpy', () => {
' <li class="nav-item"><a id="three-link" class="nav-link" href="#three">Three</a></li>',
' </ul>',
'</nav>',
'<div id="content" style="position:relative; height: 200px; overflow-y: auto;">',
'<div id="content" style="height: 200px; overflow-y: auto;">',
' <div id="one" style="height: 100px;"></div>',
' <div id="two" style="height: 100px;"></div>',
' <div id="three" style="height: 100px;"></div>',
' <div id="spacer" style="height: 100px;"></div>',
'</div>'
].join('')

const negativeHeight = -20
const startOfSectionTwo = 110
const negativeHeight = -10
const startOfSectionTwo = 101
const contentEl = fixtureEl.querySelector('#content')
const scrollSpy = new ScrollSpy(contentEl, {
target: '#navigation',
offset: contentEl.offsetTop
rootMargin: `${contentEl.offsetTop}px 0px 0px`
})
const spy = spyOn(scrollSpy, '_activate').and.callThrough()

Expand All @@ -386,11 +420,11 @@ describe('ScrollSpy', () => {
done()
}
})

contentEl.scrollIntoView(true)
contentEl.scrollTo({
top: startOfSectionTwo
})
contentEl.scrollTop = startOfSectionTwo
// contentEl.scrollIntoView(true)
// contentEl.scrollTo({
// top: startOfSectionTwo
// })
})

it('should correctly select navigation element on backward scrolling when each target section height is 100%', done => {
Expand Down Expand Up @@ -605,4 +639,19 @@ describe('ScrollSpy', () => {
expect(ScrollSpy.getInstance(fixtureEl)).toEqual(null)
})
})

describe('event handler', () => {
it('should create scrollspy on window load event', () => {
fixtureEl.innerHTML = [
'<div id="nav"></div>' +
'<div data-bs-spy="scroll" data-bs-target="#nav"></div>'
].join('')

const scrollSpyEl = fixtureEl.querySelector('div')

window.dispatchEvent(createEvent('load'))

expect(ScrollSpy.getInstance(scrollSpyEl)).not.toBeNull()
})
})
})

0 comments on commit f397a43

Please sign in to comment.