Fix some JS todos and warnings (#41998)

* Fix some JS todos and warnings

* Undo some

Author: mdo

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
     }
 

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)
 

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

js/src/scrollspy.js

@@ -39,15 +39,13 @@ 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,
   threshold: [0.1, 0.5, 1]
 }
 
 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))
     }

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

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()

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', () => {