Skip to content

Commit caf794b

Browse files
mdomdo
committed
Fix some JS todos and warnings
1 parent 6275ada commit caf794b

File tree

9 files changed

+57
-114
lines changed

9 files changed

+57
-114
lines changed

‎build/css-minify.mjs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,14 @@ const targets = browserslistToTargets(['> 0.5%', 'last 2 versions', 'Firefox ESR
2323
for (const file of cssFiles) {
2424
const inputPath = path.join(distDir, file)
2525
const outputPath = path.join(distDir, file.replace('.css', '.min.css'))
26-
const mapPath = outputPath + '.map'
26+
const mapPath = `${outputPath}.map`
2727

2828
console.log(`Minifying ${file}...`)
2929

3030
const inputCss = fs.readFileSync(inputPath, 'utf8')
31-
const inputMap = fs.existsSync(inputPath + '.map')
32-
? JSON.parse(fs.readFileSync(inputPath + '.map', 'utf8'))
33-
: undefined
31+
const inputMap = fs.existsSync(`${inputPath}.map`) ?
32+
JSON.parse(fs.readFileSync(`${inputPath}.map`, 'utf8')) :
33+
undefined
3434

3535
try {
3636
const result = transform({
@@ -43,7 +43,7 @@ for (const file of cssFiles) {
4343
})
4444

4545
// Write minified CSS with source map reference
46-
const minifiedCss = result.code.toString() + `\n/*# sourceMappingURL=${path.basename(mapPath)} */`
46+
const minifiedCss = `${result.code.toString()}\n/*# sourceMappingURL=${path.basename(mapPath)} */`
4747
fs.writeFileSync(outputPath, minifiedCss)
4848

4949
// Write source map

‎js/src/carousel.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ const Default = {
7676
}
7777

7878
const DefaultType = {
79-
interval: '(number|boolean)', // TODO:v6 remove boolean support
79+
interval: 'number',
8080
keyboard: 'boolean',
8181
pause: '(string|boolean)',
8282
ride: '(boolean|string)',
@@ -125,10 +125,9 @@ class Carousel extends BaseComponent {
125125
}
126126

127127
nextWhenVisible() {
128-
// FIXME TODO use `document.visibilityState`
129128
// Don't call next when the page isn't visible
130129
// or the carousel or its parent isn't visible
131-
if (!document.hidden && isVisible(this._element)) {
130+
if (document.visibilityState === 'visible' && isVisible(this._element)) {
132131
this.next()
133132
}
134133
}
@@ -328,7 +327,6 @@ class Carousel extends BaseComponent {
328327

329328
if (!activeElement || !nextElement) {
330329
// Some weirdness is happening, so we bail
331-
// TODO: change tests that use empty divs to avoid this check
332330
return
333331
}
334332

‎js/src/dom/event-handler.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ function findHandler(events, callable, delegationSelector = null) {
126126

127127
function normalizeParameters(originalTypeEvent, handler, delegationFunction) {
128128
const isDelegated = typeof handler === 'string'
129-
// TODO: tooltip passes `false` instead of selector, so we need to check
130129
const callable = isDelegated ? delegationFunction : (handler || delegationFunction)
131130
let typeEvent = getTypeEvent(originalTypeEvent)
132131

‎js/src/dom/selector-engine.js‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -70,20 +70,6 @@ const SelectorEngine = {
7070

7171
return []
7272
},
73-
// TODO: this is now unused; remove later along with prev()
74-
next(element, selector) {
75-
let next = element.nextElementSibling
76-
77-
while (next) {
78-
if (next.matches(selector)) {
79-
return [next]
80-
}
81-
82-
next = next.nextElementSibling
83-
}
84-
85-
return []
86-
},
8773

8874
focusableChildren(element) {
8975
const focusables = [

‎js/src/dropdown.js‎

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,7 @@ class Dropdown extends BaseComponent {
140140
this._submenuCloseTimeouts = new Map() // Map of submenu element -> timeout ID
141141
this._hoverIntentData = null // For safe triangle calculation
142142

143-
// TODO: v6 revert #37011 & change markup https://getbootstrap.com/docs/5.3/forms/input-group/
144-
this._menu = SelectorEngine.next(this._element, SELECTOR_MENU)[0] ||
145-
SelectorEngine.prev(this._element, SELECTOR_MENU)[0] ||
146-
SelectorEngine.findOne(SELECTOR_MENU, this._parent)
143+
this._menu = SelectorEngine.findOne(SELECTOR_MENU, this._parent)
147144

148145
// Parse responsive placements on init
149146
this._parseResponsivePlacements()
@@ -860,6 +857,26 @@ class Dropdown extends BaseComponent {
860857
return false
861858
}
862859

860+
_handleEscapeKey(event, toggleButton) {
861+
// If in a submenu, close just that submenu
862+
const currentMenu = event.target.closest(SELECTOR_MENU)
863+
const parentSubmenuWrapper = currentMenu?.closest(SELECTOR_SUBMENU)
864+
865+
if (parentSubmenuWrapper && this._openSubmenus.size > 0) {
866+
const parentTrigger = SelectorEngine.findOne(SELECTOR_SUBMENU_TOGGLE, parentSubmenuWrapper)
867+
this._closeSubmenu(currentMenu, parentSubmenuWrapper)
868+
if (parentTrigger) {
869+
parentTrigger.focus()
870+
}
871+
872+
return
873+
}
874+
875+
// Otherwise close the whole dropdown
876+
this.hide()
877+
toggleButton.focus()
878+
}
879+
863880
static clearMenus(event) {
864881
if (event.button === RIGHT_MOUSE_BUTTON || (event.type === 'keyup' && event.key !== TAB_KEY)) {
865882
return
@@ -919,12 +936,9 @@ class Dropdown extends BaseComponent {
919936
return
920937
}
921938

922-
// TODO: v6 revert #37011 & change markup https://getbootstrap.com/docs/5.3/forms/input-group/
923939
const getToggleButton = this.matches(SELECTOR_DATA_TOGGLE) ?
924940
this :
925-
(SelectorEngine.prev(this, SELECTOR_DATA_TOGGLE)[0] ||
926-
SelectorEngine.next(this, SELECTOR_DATA_TOGGLE)[0] ||
927-
SelectorEngine.findOne(SELECTOR_DATA_TOGGLE, event.delegateTarget.parentNode))
941+
SelectorEngine.findOne(SELECTOR_DATA_TOGGLE, event.delegateTarget.parentNode)
928942

929943
if (!getToggleButton) {
930944
return
@@ -950,24 +964,7 @@ class Dropdown extends BaseComponent {
950964
if (isEscapeEvent && instance._isShown()) {
951965
event.preventDefault()
952966
event.stopPropagation()
953-
954-
// If in a submenu, close just that submenu
955-
const currentMenu = event.target.closest(SELECTOR_MENU)
956-
const parentSubmenuWrapper = currentMenu?.closest(SELECTOR_SUBMENU)
957-
958-
if (parentSubmenuWrapper && instance._openSubmenus.size > 0) {
959-
const parentTrigger = SelectorEngine.findOne(SELECTOR_SUBMENU_TOGGLE, parentSubmenuWrapper)
960-
instance._closeSubmenu(currentMenu, parentSubmenuWrapper)
961-
if (parentTrigger) {
962-
parentTrigger.focus()
963-
}
964-
965-
return
966-
}
967-
968-
// Otherwise close the whole dropdown
969-
instance.hide()
970-
getToggleButton.focus()
967+
instance._handleEscapeKey(event, getToggleButton)
971968
}
972969
}
973970
}

‎js/src/scrollspy.js‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,13 @@ const SELECTOR_DROPDOWN = '.dropdown'
3939
const SELECTOR_DROPDOWN_TOGGLE = '.dropdown-toggle'
4040

4141
const Default = {
42-
offset: null, // TODO: v6 @deprecated, keep it for backwards compatibility reasons
4342
rootMargin: '0px 0px -25%',
4443
smoothScroll: false,
4544
target: null,
4645
threshold: [0.1, 0.5, 1]
4746
}
4847

4948
const DefaultType = {
50-
offset: '(number|null)', // TODO v6 @deprecated, keep it for backwards compatibility reasons
5149
rootMargin: 'string',
5250
smoothScroll: 'boolean',
5351
target: 'element',
@@ -111,12 +109,8 @@ class ScrollSpy extends BaseComponent {
111109

112110
// Private
113111
_configAfterMerge(config) {
114-
// TODO: on v6 target should be given explicitly & remove the {target: 'ss-target'} case
115112
config.target = getElement(config.target) || document.body
116113

117-
// TODO: v6 Only for backwards compatibility reasons. Use rootMargin only
118-
config.rootMargin = config.offset ? `${config.offset}px 0px -30%` : config.rootMargin
119-
120114
if (typeof config.threshold === 'string') {
121115
config.threshold = config.threshold.split(',').map(value => Number.parseFloat(value))
122116
}

‎js/src/tab.js‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ const NOT_SELECTOR_DROPDOWN_TOGGLE = `:not(${SELECTOR_DROPDOWN_TOGGLE})`
4545
const SELECTOR_TAB_PANEL = '.list-group, .nav, [role="tablist"]'
4646
const SELECTOR_OUTER = '.nav-item, .list-group-item'
4747
const SELECTOR_INNER = `.nav-link${NOT_SELECTOR_DROPDOWN_TOGGLE}, .list-group-item${NOT_SELECTOR_DROPDOWN_TOGGLE}, [role="tab"]${NOT_SELECTOR_DROPDOWN_TOGGLE}`
48-
const SELECTOR_DATA_TOGGLE = '[data-bs-toggle="tab"], [data-bs-toggle="pill"], [data-bs-toggle="list"]' // TODO: could only be `tab` in v6
48+
const SELECTOR_DATA_TOGGLE = '[data-bs-toggle="tab"]'
4949
const SELECTOR_INNER_ELEM = `${SELECTOR_INNER}, ${SELECTOR_DATA_TOGGLE}`
5050

51-
const SELECTOR_DATA_TOGGLE_ACTIVE = `.${CLASS_NAME_ACTIVE}[data-bs-toggle="tab"], .${CLASS_NAME_ACTIVE}[data-bs-toggle="pill"], .${CLASS_NAME_ACTIVE}[data-bs-toggle="list"]`
51+
const SELECTOR_DATA_TOGGLE_ACTIVE = `.${CLASS_NAME_ACTIVE}[data-bs-toggle="tab"]`
5252

5353
/**
5454
* Class definition
@@ -60,9 +60,7 @@ class Tab extends BaseComponent {
6060
this._parent = this._element.closest(SELECTOR_TAB_PANEL)
6161

6262
if (!this._parent) {
63-
return
64-
// TODO: should throw exception in v6
65-
// throw new TypeError(`${element.outerHTML} has not a valid parent ${SELECTOR_INNER_ELEM}`)
63+
throw new TypeError(`${element.outerHTML} has not a valid parent ${SELECTOR_TAB_PANEL}`)
6664
}
6765

6866
// Set up initial aria attributes

‎js/src/tooltip.js‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -320,13 +320,7 @@ class Tooltip extends BaseComponent {
320320
_createTipElement(content) {
321321
const tip = this._getTemplateFactory(content).toHtml()
322322

323-
// TODO: remove this check in v6
324-
if (!tip) {
325-
return null
326-
}
327-
328323
tip.classList.remove(CLASS_NAME_FADE, CLASS_NAME_SHOW)
329-
// TODO: v6 the following can be achieved with CSS only
330324
tip.classList.add(`bs-${this.constructor.NAME}-auto`)
331325

332326
const tipId = getUID(this.constructor.NAME).toString()

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

Lines changed: 24 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -59,60 +59,37 @@ describe('Dropdown', () => {
5959
expect(dropdownByElement._element).toEqual(btnDropdown)
6060
})
6161

62-
it('should work on invalid markup', () => {
63-
return new Promise(resolve => {
64-
// TODO: REMOVE in v6
65-
fixtureEl.innerHTML = [
66-
'<div class="dropdown">',
67-
' <div class="dropdown-menu">',
68-
' <a class="dropdown-item" href="#">Link</a>',
69-
' </div>',
70-
'</div>'
71-
].join('')
72-
73-
const dropdownElem = fixtureEl.querySelector('.dropdown-menu')
74-
const dropdown = new Dropdown(dropdownElem)
75-
76-
dropdownElem.addEventListener('shown.bs.dropdown', () => {
77-
resolve()
78-
})
62+
it('should create offset modifier correctly when offset option is a function', async () => {
63+
fixtureEl.innerHTML = [
64+
'<div class="dropdown">',
65+
' <button class="btn dropdown-toggle" data-bs-toggle="dropdown">Dropdown</button>',
66+
' <div class="dropdown-menu">',
67+
' <a class="dropdown-item" href="#">Secondary link</a>',
68+
' </div>',
69+
'</div>'
70+
].join('')
7971

80-
expect().nothing()
81-
dropdown.show()
72+
const getOffset = jasmine.createSpy('getOffset').and.returnValue([10, 20])
73+
const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]')
74+
const dropdown = new Dropdown(btnDropdown, {
75+
offset: getOffset
8276
})
83-
})
84-
85-
it('should create offset modifier correctly when offset option is a function', () => {
86-
return new Promise(resolve => {
87-
fixtureEl.innerHTML = [
88-
'<div class="dropdown">',
89-
' <button class="btn dropdown-toggle" data-bs-toggle="dropdown">Dropdown</button>',
90-
' <div class="dropdown-menu">',
91-
' <a class="dropdown-item" href="#">Secondary link</a>',
92-
' </div>',
93-
'</div>'
94-
].join('')
9577

96-
const getOffset = jasmine.createSpy('getOffset').and.returnValue([10, 20])
97-
const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]')
98-
const dropdown = new Dropdown(btnDropdown, {
99-
offset: getOffset
100-
})
78+
const offset = dropdown._getOffset()
79+
expect(typeof offset).toEqual('function')
10180

102-
btnDropdown.addEventListener('shown.bs.dropdown', () => {
103-
// Floating UI calls offset function asynchronously
104-
setTimeout(() => {
105-
expect(getOffset).toHaveBeenCalled()
106-
resolve()
107-
}, 20)
108-
})
109-
110-
const offset = dropdown._getOffset()
81+
const shownPromise = new Promise(resolve => {
82+
btnDropdown.addEventListener('shown.bs.dropdown', resolve)
83+
})
11184

112-
expect(typeof offset).toEqual('function')
85+
dropdown.show()
86+
await shownPromise
11387

114-
dropdown.show()
88+
// Floating UI calls offset function asynchronously
89+
await new Promise(resolve => {
90+
setTimeout(resolve, 20)
11591
})
92+
expect(getOffset).toHaveBeenCalled()
11693
})
11794

11895
it('should create offset modifier correctly when offset option is a string into data attribute', () => {

0 commit comments

Comments
 (0)