Simplify retrieval of target elements (with data-target and href)

js/src/alert.js

@@ -67,14 +67,14 @@ const Alert = (($) => {
     close(element) {
       element = element || this._element
 
-      const rootElement = this._getRootElement(element)
-      const customEvent = this._triggerCloseEvent(rootElement)
+      const $rootElement = this._getRootElement(element)
+      const customEvent = this._triggerCloseEvent($rootElement)
 
       if (customEvent.isDefaultPrevented()) {
         return
       }
 
-      this._removeElement(rootElement)
+      this._removeElement($rootElement)
     }
 
     dispose() {
@@ -86,18 +86,8 @@ const Alert = (($) => {
     // private
 
     _getRootElement(element) {
-      const selector = Util.getSelectorFromElement(element)
-      let parent     = false
-
-      if (selector) {
-        parent = $(selector)[0]
-      }
-
-      if (!parent) {
-        parent = $(element).closest(`.${ClassName.ALERT}`)[0]
-      }
-
-      return parent
+      const targets = Util.getTargets(element)
+      return (targets.length ? targets : $(element).closest(`.${ClassName.ALERT}`)).first()
     }
 
     _triggerCloseEvent(element) {
@@ -107,22 +97,22 @@ const Alert = (($) => {
       return closeEvent
     }
 
-    _removeElement(element) {
-      $(element).removeClass(ClassName.SHOW)
+    _removeElement($element) {
+      $element.removeClass(ClassName.SHOW)
 
       if (!Util.supportsTransitionEnd() ||
-          !$(element).hasClass(ClassName.FADE)) {
-        this._destroyElement(element)
+          !$element.hasClass(ClassName.FADE)) {
+        this._destroyElement($element)
         return
       }
 
-      $(element)
-        .one(Util.TRANSITION_END, (event) => this._destroyElement(element, event))
+      $element
+        .one(Util.TRANSITION_END, (event) => this._destroyElement($element, event))
         .emulateTransitionEnd(TRANSITION_DURATION)
     }
 
-    _destroyElement(element) {
-      $(element)
+    _destroyElement($element) {
+      $element
         .detach()
         .trigger(Event.CLOSED)
         .remove()

js/src/carousel.js

@@ -453,29 +453,23 @@ const Carousel = (($) => {
     }
 
     static _dataApiClickHandler(event) {
-      const selector = Util.getSelectorFromElement(this)
+      const $target = Util.getTargets(this).first()
 
-      if (!selector) {
+      if (!$target.hasClass(ClassName.CAROUSEL)) {
         return
       }
 
-      const target = $(selector)[0]
-
-      if (!target || !$(target).hasClass(ClassName.CAROUSEL)) {
-        return
-      }
-
-      const config     = $.extend({}, $(target).data(), $(this).data())
+      const config     = $.extend({}, $target.data(), $(this).data())
       const slideIndex = this.getAttribute('data-slide-to')
 
       if (slideIndex) {
         config.interval = false
       }
 
-      Carousel._jQueryInterface.call($(target), config)
+      Carousel._jQueryInterface.call($target, config)
 
       if (slideIndex) {
-        $(target).data(DATA_KEY).to(slideIndex)
+        $target.data(DATA_KEY).to(slideIndex)
       }
 
       event.preventDefault()

js/src/collapse.js

@@ -278,7 +278,7 @@ const Collapse = (($) => {
 
       $(parent).find(selector).each((i, element) => {
         this._addAriaAndCollapsedClass(
-          Collapse._getTargetFromElement(element),
+          Util.getTargets(element)[0],
           [element]
         )
       })
@@ -301,11 +301,6 @@ const Collapse = (($) => {
 
     // static
 
-    static _getTargetFromElement(element) {
-      const selector = Util.getSelectorFromElement(element)
-      return selector ? $(selector)[0] : null
-    }
-
     static _jQueryInterface(config) {
       return this.each(function () {
         const $this   = $(this)
@@ -349,11 +344,11 @@ const Collapse = (($) => {
       event.preventDefault()
     }
 
-    const target = Collapse._getTargetFromElement(this)
-    const data   = $(target).data(DATA_KEY)
+    const $target = Util.getTargets(this).first()
+    const data   = $target.data(DATA_KEY)
     const config = data ? 'toggle' : $(this).data()
 
-    Collapse._jQueryInterface.call($(target), config)
+    Collapse._jQueryInterface.call($target, config)
   })
 
 

js/src/dropdown.js

@@ -85,8 +85,8 @@ const Dropdown = (($) => {
         return false
       }
 
-      const parent   = Dropdown._getParentFromElement(this)
-      const isActive = $(parent).hasClass(ClassName.SHOW)
+      const $parent   = Dropdown._getParentFromElement(this)
+      const isActive = $parent.hasClass(ClassName.SHOW)
 
       Dropdown._clearMenus()
 
@@ -99,7 +99,7 @@ const Dropdown = (($) => {
       }
       const showEvent     = $.Event(Event.SHOW, relatedTarget)
 
-      $(parent).trigger(showEvent)
+      $parent.trigger(showEvent)
 
       if (showEvent.isDefaultPrevented()) {
         return false
@@ -110,15 +110,15 @@ const Dropdown = (($) => {
       // only needed because of broken event delegation on iOS
       // https://www.quirksmode.org/blog/archives/2014/02/mouse_event_bub.html
       if ('ontouchstart' in document.documentElement &&
-         !$(parent).closest(Selector.NAVBAR_NAV).length) {
+         !$parent.closest(Selector.NAVBAR_NAV).length) {
         $('body').children().on('mouseover', null, $.noop)
       }
 
       this.focus()
       this.setAttribute('aria-expanded', true)
 
-      $(parent).toggleClass(ClassName.SHOW)
-      $(parent).trigger($.Event(Event.SHOWN, relatedTarget))
+      $parent.toggleClass(ClassName.SHOW)
+      $parent.trigger($.Event(Event.SHOWN, relatedTarget))
 
       return false
     }
@@ -166,23 +166,23 @@ const Dropdown = (($) => {
       const toggles = $.makeArray($(Selector.DATA_TOGGLE))
 
       for (let i = 0; i < toggles.length; i++) {
-        const parent        = Dropdown._getParentFromElement(toggles[i])
+        const $parent        = Dropdown._getParentFromElement(toggles[i])
         const relatedTarget = {
           relatedTarget : toggles[i]
         }
 
-        if (!$(parent).hasClass(ClassName.SHOW)) {
+        if (!$parent.hasClass(ClassName.SHOW)) {
           continue
         }
 
         if (event && (event.type === 'click' &&
             /input|textarea/i.test(event.target.tagName) || event.type === 'keyup' && event.which === TAB_KEYCODE)
-            && $.contains(parent, event.target)) {
+            && $.contains($parent[0], event.target)) {
           continue
         }
 
         const hideEvent = $.Event(Event.HIDE, relatedTarget)
-        $(parent).trigger(hideEvent)
+        $parent.trigger(hideEvent)
         if (hideEvent.isDefaultPrevented()) {
           continue
         }
@@ -195,21 +195,15 @@ const Dropdown = (($) => {
 
         toggles[i].setAttribute('aria-expanded', 'false')
 
-        $(parent)
+        $parent
           .removeClass(ClassName.SHOW)
           .trigger($.Event(Event.HIDDEN, relatedTarget))
       }
     }
 
     static _getParentFromElement(element) {
-      let parent
-      const selector = Util.getSelectorFromElement(element)
-
-      if (selector) {
-        parent = $(selector)[0]
-      }
-
-      return parent || element.parentNode
+      const targets = Util.getTargets(element)
+      return targets.length ? targets.first() : $(element.parentNode)
     }
 
     static _dataApiKeydownHandler(event) {
@@ -225,22 +219,22 @@ const Dropdown = (($) => {
         return
       }
 
-      const parent   = Dropdown._getParentFromElement(this)
-      const isActive = $(parent).hasClass(ClassName.SHOW)
+      const $parent   = Dropdown._getParentFromElement(this)
+      const isActive = $parent.hasClass(ClassName.SHOW)
 
       if (!isActive && (event.which !== ESCAPE_KEYCODE || event.which !== SPACE_KEYCODE) ||
            isActive && (event.which === ESCAPE_KEYCODE || event.which === SPACE_KEYCODE)) {
 
         if (event.which === ESCAPE_KEYCODE) {
-          const toggle = $(parent).find(Selector.DATA_TOGGLE)[0]
+          const toggle = $parent.find(Selector.DATA_TOGGLE)[0]
           $(toggle).trigger('focus')
         }
 
         $(this).trigger('click')
         return
       }
 
-      const items = $(parent).find(Selector.VISIBLE_ITEMS).get()
+      const items = $parent.find(Selector.VISIBLE_ITEMS).get()
 
       if (!items.length) {
         return

js/src/modal.js

@@ -527,21 +527,16 @@ const Modal = (($) => {
    */
 
   $(document).on(Event.CLICK_DATA_API, Selector.DATA_TOGGLE, function (event) {
-    let target
-    const selector = Util.getSelectorFromElement(this)
+    const $target = Util.getTargets(this).first()
 
-    if (selector) {
-      target = $(selector)[0]
-    }
-
-    const config = $(target).data(DATA_KEY) ?
-      'toggle' : $.extend({}, $(target).data(), $(this).data())
+    const config = $target.data(DATA_KEY) ?
+      'toggle' : $.extend({}, $target.data(), $(this).data())
 
     if (this.tagName === 'A' || this.tagName === 'AREA') {
       event.preventDefault()
     }
 
-    const $target = $(target).one(Event.SHOW, (showEvent) => {
+    $target.one(Event.SHOW, (showEvent) => {
       if (showEvent.isDefaultPrevented()) {
         // only register focus restorer if modal will actually get shown
         return
@@ -554,7 +549,7 @@ const Modal = (($) => {
       })
     })
 
-    Modal._jQueryInterface.call($(target), config, this)
+    Modal._jQueryInterface.call($target, config, this)
   })
 
 

js/src/scrollspy.js

@@ -124,20 +124,14 @@ const ScrollSpy = (($) => {
 
       targets
         .map((element) => {
-          let target
-          const targetSelector = Util.getSelectorFromElement(element)
-
-          if (targetSelector) {
-            target = $(targetSelector)[0]
-          }
-
+          const target = Util.getTargets(element)[0]
           if (target) {
             const targetBCR = target.getBoundingClientRect()
             if (targetBCR.width || targetBCR.height) {
               // todo (fat): remove sketch reliance on jQuery position/offset
               return [
                 $(target)[offsetMethod]().top + offsetBase,
-                targetSelector
+                `#${$(target).attr('id')}`
               ]
             }
           }

js/src/tab.js

@@ -81,10 +81,8 @@ const Tab = (($) => {
         return
       }
 
-      let target
       let previous
       const listElement = $(this._element).closest(Selector.NAV_LIST_GROUP)[0]
-      const selector    = Util.getSelectorFromElement(this._element)
 
       if (listElement) {
         previous = $.makeArray($(listElement).find(Selector.ACTIVE))
@@ -110,10 +108,6 @@ const Tab = (($) => {
         return
       }
 
-      if (selector) {
-        target = $(selector)[0]
-      }
-
       this._activate(
         this._element,
         listElement
@@ -132,6 +126,8 @@ const Tab = (($) => {
         $(this._element).trigger(shownEvent)
       }
 
+      const target = Util.getTargets(this._element)[0]
+
       if (target) {
         this._activate(target, target.parentNode, complete)
       } else {

js/src/util.js

@@ -110,17 +110,15 @@ const Util = (($) => {
       return prefix
     },
 
-    getSelectorFromElement(element) {
+    getTargets(element) {
       let selector = element.getAttribute('data-target')
-      if (!selector || selector === '#') {
+      if (!selector) {
         selector = element.getAttribute('href') || ''
       }
-
       try {
-        const $selector = $(selector)
-        return $selector.length > 0 ? selector : null
-      } catch (error) {
-        return null
+        return $(document).find(selector)
+      } catch (err) {
+        return $()
       }
     },
 

js/tests/unit/dropdown.js

@@ -63,6 +63,24 @@ $(function () {
     }, 300)
   })
 
+  QUnit.test('should open dropdown if trigger is a link with an href and data-target="#"', function (assert) {
+    assert.expect(1)
+    var dropdownHTML = '<ul class="tabs">'
+        + '<li class="dropdown">'
+        + '<a href="https://getbootstrap.com" data-target="#" class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</a>'
+        + '<ul class="dropdown-menu">'
+        + '<li><a href="#">Secondary link</a></li>'
+        + '<li><a href="#">Something else here</a></li>'
+        + '<li class="divider"/>'
+        + '<li><a href="#">Another link</a></li>'
+        + '</ul>'
+        + '</li>'
+        + '</ul>'
+    var $dropdown = $(dropdownHTML).find('[data-toggle="dropdown"]').bootstrapDropdown().trigger('click')
+
+    assert.ok($dropdown.parent('.dropdown').hasClass('show'), '"show" class not added on click')
+  })
+
   QUnit.test('should set aria-expanded="true" on target when dropdown menu is shown', function (assert) {
     assert.expect(1)
     var done = assert.async()