Fix some JS todos and warnings (#41998)

* Fix some JS todos and warnings

* Undo some

diff --git a/js/src/carousel.js b/js/src/carousel.js
index da9c83320221ff4d204667015ecd6a69fc8e58fa..66d5b7c4994e81c6cae8abd82ebd33b0b0f74618 100644 (file)
--- a/js/src/carousel.js
+++ b/js/src/carousel.js
@@ -76,7 +76,7 @@ const Default = {
 }
 
 const DefaultType = {
-  interval: '(number|boolean)', // TODO:v6 remove boolean support
+  interval: 'number',
   keyboard: 'boolean',
   pause: '(string|boolean)',
   ride: '(boolean|string)',
@@ -125,10 +125,9 @@ class Carousel extends BaseComponent {
   }
 
   nextWhenVisible() {
-    // FIXME TODO use `document.visibilityState`
     // Don't call next when the page isn't visible
     // or the carousel or its parent isn't visible
-    if (!document.hidden && isVisible(this._element)) {
+    if (document.visibilityState === 'visible' && isVisible(this._element)) {
       this.next()
     }
   }
@@ -328,7 +327,6 @@ class Carousel extends BaseComponent {
 
     if (!activeElement || !nextElement) {
       // Some weirdness is happening, so we bail
-      // TODO: change tests that use empty divs to avoid this check
       return
     }
 

diff --git a/js/src/dom/event-handler.js b/js/src/dom/event-handler.js
index 5b2d4c1dd8c9ecfe3210c37aa1e0027986faf314..b15b5df21ee49b2f5ab902ddc675afa6ea8cabe9 100644 (file)
--- a/js/src/dom/event-handler.js
+++ b/js/src/dom/event-handler.js
@@ -126,7 +126,6 @@ function findHandler(events, callable, delegationSelector = null) {
 
 function normalizeParameters(originalTypeEvent, handler, delegationFunction) {
   const isDelegated = typeof handler === 'string'
-  // TODO: tooltip passes `false` instead of selector, so we need to check
   const callable = isDelegated ? delegationFunction : (handler || delegationFunction)
   let typeEvent = getTypeEvent(originalTypeEvent)
 

diff --git a/js/src/dom/selector-engine.js b/js/src/dom/selector-engine.js
index a4d81f3b9127e2144d6e9d58141dd8b2f6fbe047..d4cee4d811a181c32d38293909a45c67fdd787af 100644 (file)
--- a/js/src/dom/selector-engine.js
+++ b/js/src/dom/selector-engine.js
@@ -70,6 +70,7 @@ const SelectorEngine = {
 
     return []
   },
+
   // TODO: this is now unused; remove later along with prev()
   next(element, selector) {
     let next = element.nextElementSibling

diff --git a/js/src/scrollspy.js b/js/src/scrollspy.js
index 403e12bc9d7495b4d0c53a7bc5ec99bcd3a5f5e8..0fa98d88efc4d2877e917ebd926583e638f49ab2 100644 (file)
--- a/js/src/scrollspy.js
+++ b/js/src/scrollspy.js
@@ -39,7 +39,6 @@ const SELECTOR_DROPDOWN = '.dropdown'
 const SELECTOR_DROPDOWN_TOGGLE = '.dropdown-toggle'
 
 const Default = {
-  offset: null, // TODO: v6 @deprecated, keep it for backwards compatibility reasons
   rootMargin: '0px 0px -25%',
   smoothScroll: false,
   target: null,
@@ -47,7 +46,6 @@ const Default = {
 }
 
 const DefaultType = {
-  offset: '(number|null)', // TODO v6 @deprecated, keep it for backwards compatibility reasons
   rootMargin: 'string',
   smoothScroll: 'boolean',
   target: 'element',
@@ -111,12 +109,8 @@ class ScrollSpy extends BaseComponent {
 
   // Private
   _configAfterMerge(config) {
-    // TODO: on v6 target should be given explicitly & remove the {target: 'ss-target'} case
     config.target = getElement(config.target) || document.body
 
-    // TODO: v6 Only for backwards compatibility reasons. Use rootMargin only
-    config.rootMargin = config.offset ? `${config.offset}px 0px -30%` : config.rootMargin
-
     if (typeof config.threshold === 'string') {
       config.threshold = config.threshold.split(',').map(value => Number.parseFloat(value))
     }

diff --git a/js/src/tab.js b/js/src/tab.js
index 989a37a657f9a51fdf4078c0af3b1416b517402a..2f8c3433054fc2cb64e5721f9cc28d535b527b2f 100644 (file)
--- a/js/src/tab.js
+++ b/js/src/tab.js
@@ -62,7 +62,7 @@ class Tab extends BaseComponent {
     if (!this._parent) {
       return
       // TODO: should throw exception in v6
-      // throw new TypeError(`${element.outerHTML} has not a valid parent ${SELECTOR_INNER_ELEM}`)
+      // throw new TypeError(`${element.outerHTML} has not a valid parent ${SELECTOR_TAB_PANEL}`)
     }
 
     // Set up initial aria attributes

diff --git a/js/src/tooltip.js b/js/src/tooltip.js
index 531b359ac0348ce94937225c710f0d60c60f247e..b318564385495bd6021cf2f5811b993ac697789c 100644 (file)
--- a/js/src/tooltip.js
+++ b/js/src/tooltip.js
@@ -320,13 +320,7 @@ class Tooltip extends BaseComponent {
   _createTipElement(content) {
     const tip = this._getTemplateFactory(content).toHtml()
 
-    // TODO: remove this check in v6
-    if (!tip) {
-      return null
-    }
-
     tip.classList.remove(CLASS_NAME_FADE, CLASS_NAME_SHOW)
-    // TODO: v6 the following can be achieved with CSS only
     tip.classList.add(`bs-${this.constructor.NAME}-auto`)
 
     const tipId = getUID(this.constructor.NAME).toString()

diff --git a/js/tests/unit/dropdown.spec.js b/js/tests/unit/dropdown.spec.js
index f3429d89a6e44fd5cd70b759e90bac8d96c6e9e2..00462e1068a3dd9bc0592112cc0a2f3916eca649 100644 (file)
--- a/js/tests/unit/dropdown.spec.js
+++ b/js/tests/unit/dropdown.spec.js
@@ -59,60 +59,37 @@ describe('Dropdown', () => {
       expect(dropdownByElement._element).toEqual(btnDropdown)
     })
 
-    it('should work on invalid markup', () => {
-      return new Promise(resolve => {
-        // TODO: REMOVE in v6
-        fixtureEl.innerHTML = [
-          '<div class="dropdown">',
-          '  <div class="dropdown-menu">',
-          '    <a class="dropdown-item" href="#">Link</a>',
-          '  </div>',
-          '</div>'
-        ].join('')
-
-        const dropdownElem = fixtureEl.querySelector('.dropdown-menu')
-        const dropdown = new Dropdown(dropdownElem)
-
-        dropdownElem.addEventListener('shown.bs.dropdown', () => {
-          resolve()
-        })
+    it('should create offset modifier correctly when offset option is a function', async () => {
+      fixtureEl.innerHTML = [
+        '<div class="dropdown">',
+        '  <button class="btn dropdown-toggle" data-bs-toggle="dropdown">Dropdown</button>',
+        '  <div class="dropdown-menu">',
+        '    <a class="dropdown-item" href="#">Secondary link</a>',
+        '  </div>',
+        '</div>'
+      ].join('')
 
-        expect().nothing()
-        dropdown.show()
+      const getOffset = jasmine.createSpy('getOffset').and.returnValue([10, 20])
+      const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]')
+      const dropdown = new Dropdown(btnDropdown, {
+        offset: getOffset
       })
-    })
-
-    it('should create offset modifier correctly when offset option is a function', () => {
-      return new Promise(resolve => {
-        fixtureEl.innerHTML = [
-          '<div class="dropdown">',
-          '  <button class="btn dropdown-toggle" data-bs-toggle="dropdown">Dropdown</button>',
-          '  <div class="dropdown-menu">',
-          '    <a class="dropdown-item" href="#">Secondary link</a>',
-          '  </div>',
-          '</div>'
-        ].join('')
 
-        const getOffset = jasmine.createSpy('getOffset').and.returnValue([10, 20])
-        const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]')
-        const dropdown = new Dropdown(btnDropdown, {
-          offset: getOffset
-        })
+      const offset = dropdown._getOffset()
+      expect(typeof offset).toEqual('function')
 
-        btnDropdown.addEventListener('shown.bs.dropdown', () => {
-          // Floating UI calls offset function asynchronously
-          setTimeout(() => {
-            expect(getOffset).toHaveBeenCalled()
-            resolve()
-          }, 20)
-        })
-
-        const offset = dropdown._getOffset()
+      const shownPromise = new Promise(resolve => {
+        btnDropdown.addEventListener('shown.bs.dropdown', resolve)
+      })
 
-        expect(typeof offset).toEqual('function')
+      dropdown.show()
+      await shownPromise
 
-        dropdown.show()
+      // Floating UI calls offset function asynchronously
+      await new Promise(resolve => {
+        setTimeout(resolve, 20)
       })
+      expect(getOffset).toHaveBeenCalled()
     })
 
     it('should create offset modifier correctly when offset option is a string into data attribute', () => {