Skip to content

Commit 6e1c909

Browse files
GeoSotGeoSot
authored
Move get element functionality to a helper (#33327)
Looking around on js components I found out many checks, different expressed but with same purpose. Some of them are trying to parse string to element, others, jQuery element to js simple nodeElement etc With this Pr, I am trying to give a standard way to parse an element So this pr: * Creates `getElement` helper that tries to parse an argument to element or null * Changes `isElement` to make explicit checks and return Boolean * fixes tests deficiencies
1 parent e376142 commit 6e1c909

8 files changed

Lines changed: 85 additions & 49 deletions

File tree

‎js/src/base-component.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import Data from './dom/data'
99
import {
1010
emulateTransitionEnd,
1111
execute,
12+
getElement,
1213
getTransitionDurationFromElement
1314
} from './util/index'
1415
import EventHandler from './dom/event-handler'
@@ -23,7 +24,7 @@ const VERSION = '5.0.0'
2324

2425
class BaseComponent {
2526
constructor(element) {
26-
element = typeof element === 'string' ? document.querySelector(element) : element
27+
element = getElement(element)
2728

2829
if (!element) {
2930
return

‎js/src/collapse.js‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77

88
import {
99
defineJQueryPlugin,
10+
getElement,
1011
getSelectorFromElement,
1112
getElementFromSelector,
12-
isElement,
1313
reflow,
1414
typeCheckConfig
1515
} from './util/index'
@@ -272,14 +272,7 @@ class Collapse extends BaseComponent {
272272
_getParent() {
273273
let { parent } = this._config
274274

275-
if (isElement(parent)) {
276-
// it's a jQuery object
277-
if (typeof parent.jquery !== 'undefined' || typeof parent[0] !== 'undefined') {
278-
parent = parent[0]
279-
}
280-
} else {
281-
parent = SelectorEngine.findOne(parent)
282-
}
275+
parent = getElement(parent)
283276

284277
const selector = `${SELECTOR_DATA_TOGGLE}[data-bs-parent="${parent}"]`
285278

‎js/src/dropdown.js‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import * as Popper from '@popperjs/core'
99

1010
import {
1111
defineJQueryPlugin,
12+
getElement,
1213
getElementFromSelector,
1314
isDisabled,
1415
isElement,
@@ -166,12 +167,7 @@ class Dropdown extends BaseComponent {
166167
if (this._config.reference === 'parent') {
167168
referenceElement = parent
168169
} else if (isElement(this._config.reference)) {
169-
referenceElement = this._config.reference
170-
171-
// Check if it's jQuery element
172-
if (typeof this._config.reference.jquery !== 'undefined') {
173-
referenceElement = this._config.reference[0]
174-
}
170+
referenceElement = getElement(this._config.reference)
175171
} else if (typeof this._config.reference === 'object') {
176172
referenceElement = this._config.reference
177173
}

‎js/src/tooltip.js‎

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import * as Popper from '@popperjs/core'
1010
import {
1111
defineJQueryPlugin,
1212
findShadowRoot,
13+
getElement,
1314
getUID,
1415
isElement,
1516
isRTL,
@@ -256,7 +257,7 @@ class Tooltip extends BaseComponent {
256257
const attachment = this._getAttachment(placement)
257258
this._addAttachmentClass(attachment)
258259

259-
const container = this._getContainer()
260+
const { container } = this._config
260261
Data.set(tip, this.constructor.DATA_KEY, this)
261262

262263
if (!this._element.ownerDocument.documentElement.contains(this.tip)) {
@@ -385,10 +386,8 @@ class Tooltip extends BaseComponent {
385386
return
386387
}
387388

388-
if (typeof content === 'object' && isElement(content)) {
389-
if (content.jquery) {
390-
content = content[0]
391-
}
389+
if (isElement(content)) {
390+
content = getElement(content)
392391

393392
// content is a DOM node or a jQuery
394393
if (this._config.html) {
@@ -518,18 +517,6 @@ class Tooltip extends BaseComponent {
518517
this.getTipElement().classList.add(`${CLASS_PREFIX}-${this.updateAttachment(attachment)}`)
519518
}
520519

521-
_getContainer() {
522-
if (this._config.container === false) {
523-
return document.body
524-
}
525-
526-
if (isElement(this._config.container)) {
527-
return this._config.container
528-
}
529-
530-
return SelectorEngine.findOne(this._config.container)
531-
}
532-
533520
_getAttachment(placement) {
534521
return AttachmentMap[placement.toUpperCase()]
535522
}
@@ -664,16 +651,14 @@ class Tooltip extends BaseComponent {
664651
}
665652
})
666653

667-
if (config && typeof config.container === 'object' && config.container.jquery) {
668-
config.container = config.container[0]
669-
}
670-
671654
config = {
672655
...this.constructor.Default,
673656
...dataAttributes,
674657
...(typeof config === 'object' && config ? config : {})
675658
}
676659

660+
config.container = config.container === false ? document.body : getElement(config.container)
661+
677662
if (typeof config.delay === 'number') {
678663
config.delay = {
679664
show: config.delay,

‎js/src/util/index.js‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import SelectorEngine from '../dom/selector-engine'
2+
13
/**
24
* --------------------------------------------------------------------------
35
* Bootstrap (v5.0.0): util/index.js
@@ -100,7 +102,29 @@ const triggerTransitionEnd = element => {
100102
element.dispatchEvent(new Event(TRANSITION_END))
101103
}
102104

103-
const isElement = obj => (obj[0] || obj).nodeType
105+
const isElement = obj => {
106+
if (!obj || typeof obj !== 'object') {
107+
return false
108+
}
109+
110+
if (typeof obj.jquery !== 'undefined') {
111+
obj = obj[0]
112+
}
113+
114+
return typeof obj.nodeType !== 'undefined'
115+
}
116+
117+
const getElement = obj => {
118+
if (isElement(obj)) { // it's a jQuery object or a node element
119+
return obj.jquery ? obj[0] : obj
120+
}
121+
122+
if (typeof obj === 'string' && obj.length > 0) {
123+
return SelectorEngine.findOne(obj)
124+
}
125+
126+
return null
127+
}
104128

105129
const emulateTransitionEnd = (element, duration) => {
106130
let called = false
@@ -238,6 +262,7 @@ const execute = callback => {
238262
}
239263

240264
export {
265+
getElement,
241266
getUID,
242267
getSelectorFromElement,
243268
getElementFromSelector,

‎js/tests/unit/collapse.spec.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ describe('Collapse', () => {
5858
const collapseEl = fixtureEl.querySelector('div.collapse')
5959
const myCollapseEl = fixtureEl.querySelector('.my-collapse')
6060
const fakejQueryObject = {
61-
0: myCollapseEl
61+
0: myCollapseEl,
62+
jquery: 'foo'
6263
}
6364
const collapse = new Collapse(collapseEl, {
6465
parent: fakejQueryObject

‎js/tests/unit/dropdown.spec.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import EventHandler from '../../src/dom/event-handler'
33
import { noop } from '../../src/util'
44

55
/** Test helpers */
6-
import { getFixture, clearFixture, createEvent, jQueryMock } from '../helpers/fixture'
6+
import { clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture'
77

88
describe('Dropdown', () => {
99
let fixtureEl
@@ -467,6 +467,7 @@ describe('Dropdown', () => {
467467

468468
const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]')
469469
const virtualElement = {
470+
nodeType: 1,
470471
getBoundingClientRect() {
471472
return {
472473
width: 0,

‎js/tests/unit/util/index.spec.js‎

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as Util from '../../../src/util/index'
22

33
/** Test helpers */
4-
import { getFixture, clearFixture } from '../../helpers/fixture'
4+
import { clearFixture, getFixture } from '../../helpers/fixture'
55

66
describe('Util', () => {
77
let fixtureEl
@@ -171,24 +171,58 @@ describe('Util', () => {
171171
})
172172

173173
describe('isElement', () => {
174-
it('should detect if the parameter is an element or not', () => {
175-
fixtureEl.innerHTML = '<div></div>'
174+
it('should detect if the parameter is an element or not and return Boolean', () => {
175+
fixtureEl.innerHTML =
176+
[
177+
'<div id="foo" class="test"></div>',
178+
'<div id="bar" class="test"></div>'
179+
].join('')
176180

177-
const el = document.querySelector('div')
181+
const el = fixtureEl.querySelector('#foo')
178182

179-
expect(Util.isElement(el)).toEqual(el.nodeType)
180-
expect(Util.isElement({})).toEqual(undefined)
183+
expect(Util.isElement(el)).toEqual(true)
184+
expect(Util.isElement({})).toEqual(false)
185+
expect(Util.isElement(fixtureEl.querySelectorAll('.test'))).toEqual(false)
181186
})
182187

183188
it('should detect jQuery element', () => {
184189
fixtureEl.innerHTML = '<div></div>'
185190

186-
const el = document.querySelector('div')
191+
const el = fixtureEl.querySelector('div')
187192
const fakejQuery = {
188-
0: el
193+
0: el,
194+
jquery: 'foo'
195+
}
196+
197+
expect(Util.isElement(fakejQuery)).toEqual(true)
198+
})
199+
})
200+
201+
describe('getElement', () => {
202+
it('should try to parse element', () => {
203+
fixtureEl.innerHTML =
204+
[
205+
'<div id="foo" class="test"></div>',
206+
'<div id="bar" class="test"></div>'
207+
].join('')
208+
209+
const el = fixtureEl.querySelector('div')
210+
211+
expect(Util.getElement(el)).toEqual(el)
212+
expect(Util.getElement('#foo')).toEqual(el)
213+
expect(Util.getElement('#fail')).toBeNull()
214+
expect(Util.getElement({})).toBeNull()
215+
expect(Util.getElement([])).toBeNull()
216+
expect(Util.getElement()).toBeNull()
217+
expect(Util.getElement(null)).toBeNull()
218+
expect(Util.getElement(fixtureEl.querySelectorAll('.test'))).toBeNull()
219+
220+
const fakejQueryObject = {
221+
0: el,
222+
jquery: 'foo'
189223
}
190224

191-
expect(Util.isElement(fakejQuery)).toEqual(el.nodeType)
225+
expect(Util.getElement(fakejQueryObject)).toEqual(el)
192226
})
193227
})
194228

0 commit comments

Comments
 (0)