Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify ScrollSpy config #33250

Merged
merged 4 commits into from
Apr 6, 2021
Merged

Simplify ScrollSpy config #33250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
83ca243
Refactor ScrollSpy
GeoSot Mar 3, 2021
9b2cbee
use a basicJQueryInterface
GeoSot Mar 3, 2021
bc6dfdf
revert jQueryInterface
GeoSot Apr 5, 2021
9e1718a
Merge branch 'main' into simplify-scrollspy-config
XhmikosR Apr 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 9 additions & 12 deletions js/src/scrollspy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
isElement,
typeCheckConfig
} from './util/index'
import Data from './dom/data'
import EventHandler from './dom/event-handler'
import Manipulator from './dom/manipulator'
import SelectorEngine from './dom/selector-engine'
Expand Down Expand Up @@ -155,6 +154,7 @@ class ScrollSpy extends BaseComponent {
_getConfig(config) {
config = {
...Default,
...Manipulator.getDataAttributes(this._element),
...(typeof config === 'object' && config ? config : {})
}

Expand Down Expand Up @@ -278,20 +278,17 @@ class ScrollSpy extends BaseComponent {

static jQueryInterface(config) {
return this.each(function () {
let data = Data.get(this, DATA_KEY)
const _config = typeof config === 'object' && config
const data = ScrollSpy.getInstance(this) || new ScrollSpy(this, typeof config === 'object' ? config : {})

if (!data) {
data = new ScrollSpy(this, _config)
if (typeof config !== 'string') {
return
}

if (typeof config === 'string') {
if (typeof data[config] === 'undefined') {
throw new TypeError(`No method named "${config}"`)
}

data[config]()
if (typeof data[config] === 'undefined') {
throw new TypeError(`No method named "${config}"`)
}

data[config]()
})
}
}
Expand All @@ -304,7 +301,7 @@ class ScrollSpy extends BaseComponent {

EventHandler.on(window, EVENT_LOAD_DATA_API, () => {
SelectorEngine.find(SELECTOR_DATA_SPY)
.forEach(spy => new ScrollSpy(spy, Manipulator.getDataAttributes(spy)))
.forEach(spy => new ScrollSpy(spy))
})

/**
Expand Down
17 changes: 17 additions & 0 deletions js/tests/unit/scrollspy.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,23 @@ describe('ScrollSpy', () => {
expect(ScrollSpy.getInstance(div)).toBeDefined()
})

it('should create a scrollspy with given config', () => {
fixtureEl.innerHTML = '<div></div>'

const div = fixtureEl.querySelector('div')

jQueryMock.fn.scrollspy = ScrollSpy.jQueryInterface
jQueryMock.elements = [div]

jQueryMock.fn.scrollspy.call(jQueryMock, { offset: 15 })
spyOn(ScrollSpy.prototype, 'constructor')
expect(ScrollSpy.prototype.constructor).not.toHaveBeenCalledWith(div, { offset: 15 })

const scrollspy = ScrollSpy.getInstance(div)
expect(scrollspy).toBeDefined()
expect(scrollspy._config.offset).toBe(15)
})

it('should not re create a scrollspy', () => {
fixtureEl.innerHTML = '<div></div>'

Expand Down