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

Refactor components to use a utility function to define jQuery plugins #32285

Merged
merged 8 commits into from Dec 8, 2020
Merged

Refactor components to use a utility function to define jQuery plugins #32285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
30a9123
refactor: use an utility function to define jquery plugins
alpadev Nov 30, 2020
45a279b
test: add spec for defineJQueryPlugin utility function
alpadev Nov 30, 2020
7517f6e
Merge branch 'main' into refactor-jquery-plugin
alpadev Dec 4, 2020
365e99c
test: update spec for defineJQueryPlugin utility function
alpadev Dec 4, 2020
5320bf1
Merge branch 'main' into refactor-jquery-plugin
XhmikosR Dec 6, 2020
c3e4b26
Update .bundlewatch.config.json
XhmikosR Dec 6, 2020
b0cb83b
Merge branch 'main' into refactor-jquery-plugin
XhmikosR Dec 7, 2020
d615077
Merge branch 'main' into refactor-jquery-plugin
XhmikosR Dec 8, 2020
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
4 changes: 2 additions & 2 deletions .bundlewatch.config.json
View file
Open in desktop
Expand Up @@ -38,7 +38,7 @@
},
{
"path": "./dist/js/bootstrap.bundle.min.js",
"maxSize": "21.75 kB"
"maxSize": "21.5 kB"
},
{
"path": "./dist/js/bootstrap.esm.js",
Expand All @@ -54,7 +54,7 @@
},
{
"path": "./dist/js/bootstrap.min.js",
"maxSize": "15.75 kB"
"maxSize": "15.5 kB"
}
],
"ci": {
Expand Down
17 changes: 2 additions & 15 deletions js/src/alert.js
View file
Open in desktop
Expand Up @@ -6,8 +6,7 @@
*/

import {
getjQuery,
onDOMContentLoaded,
defineJQueryPlugin,
TRANSITION_END,
emulateTransitionEnd,
getElementFromSelector,
Expand Down Expand Up @@ -137,18 +136,6 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DISMISS, Alert.handleDi
* add .Alert to jQuery only if jQuery is present
*/

onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[NAME]
$.fn[NAME] = Alert.jQueryInterface
$.fn[NAME].Constructor = Alert
$.fn[NAME].noConflict = () => {
$.fn[NAME] = JQUERY_NO_CONFLICT
return Alert.jQueryInterface
}
}
})
defineJQueryPlugin(NAME, Alert)

export default Alert
17 changes: 2 additions & 15 deletions js/src/button.js
View file
Open in desktop
Expand Up @@ -5,7 +5,7 @@
* --------------------------------------------------------------------------
*/

import { getjQuery, onDOMContentLoaded } from './util/index'
import { defineJQueryPlugin } from './util/index'
import Data from './dom/data'
import EventHandler from './dom/event-handler'
import BaseComponent from './base-component'
Expand Down Expand Up @@ -90,19 +90,6 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, event => {
* add .Button to jQuery only if jQuery is present
*/

onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[NAME]
$.fn[NAME] = Button.jQueryInterface
$.fn[NAME].Constructor = Button

$.fn[NAME].noConflict = () => {
$.fn[NAME] = JQUERY_NO_CONFLICT
return Button.jQueryInterface
}
}
})
defineJQueryPlugin(NAME, Button)

export default Button
17 changes: 2 additions & 15 deletions js/src/carousel.js
View file
Open in desktop
Expand Up @@ -6,8 +6,7 @@
*/

import {
getjQuery,
onDOMContentLoaded,
defineJQueryPlugin,
TRANSITION_END,
emulateTransitionEnd,
getElementFromSelector,
Expand Down Expand Up @@ -614,18 +613,6 @@ EventHandler.on(window, EVENT_LOAD_DATA_API, () => {
* add .Carousel to jQuery only if jQuery is present
*/

onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[NAME]
$.fn[NAME] = Carousel.jQueryInterface
$.fn[NAME].Constructor = Carousel
$.fn[NAME].noConflict = () => {
$.fn[NAME] = JQUERY_NO_CONFLICT
return Carousel.jQueryInterface
}
}
})
defineJQueryPlugin(NAME, Carousel)

export default Carousel
17 changes: 2 additions & 15 deletions js/src/collapse.js
View file
Open in desktop
Expand Up @@ -6,8 +6,7 @@
*/

import {
getjQuery,
onDOMContentLoaded,
defineJQueryPlugin,
TRANSITION_END,
emulateTransitionEnd,
getSelectorFromElement,
Expand Down Expand Up @@ -407,18 +406,6 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (
* add .Collapse to jQuery only if jQuery is present
*/

onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[NAME]
$.fn[NAME] = Collapse.jQueryInterface
$.fn[NAME].Constructor = Collapse
$.fn[NAME].noConflict = () => {
$.fn[NAME] = JQUERY_NO_CONFLICT
return Collapse.jQueryInterface
}
}
})
defineJQueryPlugin(NAME, Collapse)

export default Collapse
17 changes: 2 additions & 15 deletions js/src/dropdown.js
View file
Open in desktop
Expand Up @@ -8,8 +8,7 @@
import * as Popper from '@popperjs/core'

import {
getjQuery,
onDOMContentLoaded,
defineJQueryPlugin,
getElementFromSelector,
isElement,
isVisible,
Expand Down Expand Up @@ -490,18 +489,6 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_FORM_CHILD, e => e.stop
* add .Dropdown to jQuery only if jQuery is present
*/

onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[NAME]
$.fn[NAME] = Dropdown.jQueryInterface
$.fn[NAME].Constructor = Dropdown
$.fn[NAME].noConflict = () => {
$.fn[NAME] = JQUERY_NO_CONFLICT
return Dropdown.jQueryInterface
}
}
})
defineJQueryPlugin(NAME, Dropdown)

export default Dropdown
17 changes: 2 additions & 15 deletions js/src/modal.js
View file
Open in desktop
Expand Up @@ -6,8 +6,7 @@
*/

import {
getjQuery,
onDOMContentLoaded,
defineJQueryPlugin,
TRANSITION_END,
emulateTransitionEnd,
getElementFromSelector,
Expand Down Expand Up @@ -602,18 +601,6 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (
* add .Modal to jQuery only if jQuery is present
*/

onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[NAME]
$.fn[NAME] = Modal.jQueryInterface
$.fn[NAME].Constructor = Modal
$.fn[NAME].noConflict = () => {
$.fn[NAME] = JQUERY_NO_CONFLICT
return Modal.jQueryInterface
}
}
})
defineJQueryPlugin(NAME, Modal)

export default Modal
16 changes: 2 additions & 14 deletions js/src/popover.js
View file
Open in desktop
Expand Up @@ -5,7 +5,7 @@
* --------------------------------------------------------------------------
*/

import { getjQuery, onDOMContentLoaded } from './util/index'
import { defineJQueryPlugin } from './util/index'
import Data from './dom/data'
import SelectorEngine from './dom/selector-engine'
import Tooltip from './tooltip'
Expand Down Expand Up @@ -165,18 +165,6 @@ class Popover extends Tooltip {
* add .Popover to jQuery only if jQuery is present
*/

onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[NAME]
$.fn[NAME] = Popover.jQueryInterface
$.fn[NAME].Constructor = Popover
$.fn[NAME].noConflict = () => {
$.fn[NAME] = JQUERY_NO_CONFLICT
return Popover.jQueryInterface
}
}
})
defineJQueryPlugin(NAME, Popover)

export default Popover
17 changes: 2 additions & 15 deletions js/src/scrollspy.js
View file
Open in desktop
Expand Up @@ -6,8 +6,7 @@
*/

import {
getjQuery,
onDOMContentLoaded,
defineJQueryPlugin,
getSelectorFromElement,
getUID,
isElement,
Expand Down Expand Up @@ -315,18 +314,6 @@ EventHandler.on(window, EVENT_LOAD_DATA_API, () => {
* add .ScrollSpy to jQuery only if jQuery is present
*/

onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[NAME]
$.fn[NAME] = ScrollSpy.jQueryInterface
$.fn[NAME].Constructor = ScrollSpy
$.fn[NAME].noConflict = () => {
$.fn[NAME] = JQUERY_NO_CONFLICT
return ScrollSpy.jQueryInterface
}
}
})
defineJQueryPlugin(NAME, ScrollSpy)

export default ScrollSpy
17 changes: 2 additions & 15 deletions js/src/tab.js
View file
Open in desktop
Expand Up @@ -6,8 +6,7 @@
*/

import {
getjQuery,
onDOMContentLoaded,
defineJQueryPlugin,
TRANSITION_END,
emulateTransitionEnd,
getElementFromSelector,
Expand Down Expand Up @@ -219,18 +218,6 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (
* add .Tab to jQuery only if jQuery is present
*/

onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[NAME]
$.fn[NAME] = Tab.jQueryInterface
$.fn[NAME].Constructor = Tab
$.fn[NAME].noConflict = () => {
$.fn[NAME] = JQUERY_NO_CONFLICT
return Tab.jQueryInterface
}
}
})
defineJQueryPlugin(NAME, Tab)

export default Tab
17 changes: 2 additions & 15 deletions js/src/toast.js
View file
Open in desktop
Expand Up @@ -6,8 +6,7 @@
*/

import {
getjQuery,
onDOMContentLoaded,
defineJQueryPlugin,
TRANSITION_END,
emulateTransitionEnd,
getTransitionDurationFromElement,
Expand Down Expand Up @@ -216,18 +215,6 @@ class Toast extends BaseComponent {
* add .Toast to jQuery only if jQuery is present
*/

onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[NAME]
$.fn[NAME] = Toast.jQueryInterface
$.fn[NAME].Constructor = Toast
$.fn[NAME].noConflict = () => {
$.fn[NAME] = JQUERY_NO_CONFLICT
return Toast.jQueryInterface
}
}
})
defineJQueryPlugin(NAME, Toast)

export default Toast
17 changes: 2 additions & 15 deletions js/src/tooltip.js
View file
Open in desktop
Expand Up @@ -8,8 +8,7 @@
import * as Popper from '@popperjs/core'

import {
getjQuery,
onDOMContentLoaded,
defineJQueryPlugin,
TRANSITION_END,
emulateTransitionEnd,
findShadowRoot,
Expand Down Expand Up @@ -786,18 +785,6 @@ class Tooltip extends BaseComponent {
* add .Tooltip to jQuery only if jQuery is present
*/

onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[NAME]
$.fn[NAME] = Tooltip.jQueryInterface
$.fn[NAME].Constructor = Tooltip
$.fn[NAME].noConflict = () => {
$.fn[NAME] = JQUERY_NO_CONFLICT
return Tooltip.jQueryInterface
}
}
})
defineJQueryPlugin(NAME, Tooltip)

export default Tooltip
19 changes: 18 additions & 1 deletion js/src/util/index.js
View file
Open in desktop
Expand Up @@ -188,6 +188,22 @@ const onDOMContentLoaded = callback => {

const isRTL = document.documentElement.dir === 'rtl'

const defineJQueryPlugin = (name, plugin) => {
onDOMContentLoaded(() => {
const $ = getjQuery()
/* istanbul ignore if */
if ($) {
const JQUERY_NO_CONFLICT = $.fn[name]
$.fn[name] = plugin.jQueryInterface
$.fn[name].Constructor = plugin
$.fn[name].noConflict = () => {
$.fn[name] = JQUERY_NO_CONFLICT
return plugin.jQueryInterface
}
}
})
}

export {
TRANSITION_END,
getUID,
Expand All @@ -204,5 +220,6 @@ export {
reflow,
getjQuery,
onDOMContentLoaded,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the onDOMContentLoaded export still used?

Copy link
Contributor Author

@alpadev alpadev Nov 30, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I'm aware of. I kept it like that because there is already a spec for it and to keep it testable. Also I wasn't sure if there would be some use case for it in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the last resort we could rename this to _onDOMContentLoaded, but since we test jQuery already, testing these individually might not be needed after all.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a better approach

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, NVM my comment. Util isn't exposed in our public API https://github.com/twbs/bootstrap/blob/main/js/index.umd.js

So, I'd say @alpadev you can go ahead with this PR and ping us when it's ready. 👍

isRTL
isRTL,
defineJQueryPlugin
}