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 Modal Config #33249

Merged
merged 2 commits into from Apr 6, 2021
Merged

Simplify Modal Config #33249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42276b1
Simplify Modal config
GeoSot Mar 3, 2021
0801f13
Omit getting data-attributes from toggle element
GeoSot Mar 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 11 additions & 26 deletions js/src/modal.js
View file
Open in desktop
Expand Up @@ -10,12 +10,11 @@ import {
emulateTransitionEnd,
getElementFromSelector,
getTransitionDurationFromElement,
isVisible,
isRTL,
isVisible,
reflow,
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 @@ -222,6 +221,7 @@ class Modal extends BaseComponent {
_getConfig(config) {
config = {
...Default,
...Manipulator.getDataAttributes(this._element),
...config
}
typeCheckConfig(NAME, config, DefaultType)
Expand Down Expand Up @@ -509,24 +509,17 @@ class Modal extends BaseComponent {

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

if (!data) {
data = new Modal(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](relatedTarget)
if (typeof data[config] === 'undefined') {
throw new TypeError(`No method named "${config}"`)
}

data[config](relatedTarget)
})
}
}
Expand All @@ -540,7 +533,7 @@ class Modal extends BaseComponent {
EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (event) {
const target = getElementFromSelector(this)

if (this.tagName === 'A' || this.tagName === 'AREA') {
if (['A', 'AREA'].includes(this.tagName)) {
event.preventDefault()
}

Expand All @@ -557,15 +550,7 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (
})
})

let data = Data.get(target, DATA_KEY)
if (!data) {
const config = {
...Manipulator.getDataAttributes(target),
...Manipulator.getDataAttributes(this)
}

data = new Modal(target, config)
}
const data = Modal.getInstance(target) || new Modal(target)

data.toggle(this)
})
Expand Down
19 changes: 18 additions & 1 deletion js/tests/unit/modal.spec.js
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@ import Modal from '../../src/modal'
import EventHandler from '../../src/dom/event-handler'

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

describe('Modal', () => {
let fixtureEl
Expand Down Expand Up @@ -1090,6 +1090,23 @@ describe('Modal', () => {
expect(Modal.getInstance(div)).toBeDefined()
})

it('should create a modal with given config', () => {
fixtureEl.innerHTML = '<div class="modal"><div class="modal-dialog"></div></div>'

const div = fixtureEl.querySelector('div')

jQueryMock.fn.modal = Modal.jQueryInterface
jQueryMock.elements = [div]

jQueryMock.fn.modal.call(jQueryMock, { keyboard: false })
spyOn(Modal.prototype, 'constructor')
expect(Modal.prototype.constructor).not.toHaveBeenCalledWith(div, { keyboard: false })

const modal = Modal.getInstance(div)
expect(modal).toBeDefined()
expect(modal._config.keyboard).toBe(false)
})

it('should not re create a modal', () => {
fixtureEl.innerHTML = '<div class="modal"><div class="modal-dialog"></div></div>'

Expand Down