Skip to content

Commit

Permalink
Allow data-toggle="dropdown" and form click events to bubble
Browse files Browse the repository at this point in the history
* remove stopPropagation from button click event

* test for delegated click events

* ensure button children can open menu

* test to ensure clicking button opens the menu

* check current element and parents

* allow dropdown form click events to bubble
  • Loading branch information
caseyjhol authored and XhmikosR committed Mar 23, 2021
1 parent 9667438 commit 16bc47d
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 14 deletions.
31 changes: 19 additions & 12 deletions js/src/dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ const CLASS_NAME_DROPSTART = 'dropstart'
const CLASS_NAME_NAVBAR = 'navbar'

const SELECTOR_DATA_TOGGLE = '[data-bs-toggle="dropdown"]'
const SELECTOR_FORM_CHILD = '.dropdown form'
const SELECTOR_MENU = '.dropdown-menu'
const SELECTOR_NAVBAR_NAV = '.navbar-nav'
const SELECTOR_VISIBLE_ITEMS = '.dropdown-menu .dropdown-item:not(.disabled):not(:disabled)'
Expand Down Expand Up @@ -253,7 +252,6 @@ class Dropdown extends BaseComponent {
_addEventListeners() {
EventHandler.on(this._element, EVENT_CLICK, event => {
event.preventDefault()
event.stopPropagation()
this.toggle()
})
}
Expand Down Expand Up @@ -377,8 +375,14 @@ class Dropdown extends BaseComponent {
}

static clearMenus(event) {
if (event && (event.button === RIGHT_MOUSE_BUTTON || (event.type === 'keyup' && event.key !== TAB_KEY))) {
return
if (event) {
if (event.button === RIGHT_MOUSE_BUTTON || (event.type === 'keyup' && event.key !== TAB_KEY)) {
return
}

if (/input|select|textarea|form/i.test(event.target.tagName)) {
return
}
}

const toggles = SelectorEngine.find(SELECTOR_DATA_TOGGLE)
Expand All @@ -402,11 +406,16 @@ class Dropdown extends BaseComponent {
continue
}

if (event && ((event.type === 'click' &&
/input|textarea/i.test(event.target.tagName)) ||
(event.type === 'keyup' && event.key === TAB_KEY)) &&
dropdownMenu.contains(event.target)) {
continue
if (event) {
// Don't close the menu if the clicked element or one of its parents is the dropdown button
if ([context._element].some(element => event.composedPath().includes(element))) {
continue
}

// Tab navigation through the dropdown menu shouldn't close the menu
if (event.type === 'keyup' && event.key === TAB_KEY && dropdownMenu.contains(event.target)) {
continue
}
}

const hideEvent = EventHandler.trigger(toggles[i], EVENT_HIDE, relatedTarget)
Expand Down Expand Up @@ -519,10 +528,8 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, Dropdown.clearMenus)
EventHandler.on(document, EVENT_KEYUP_DATA_API, Dropdown.clearMenus)
EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (event) {
event.preventDefault()
event.stopPropagation()
Dropdown.dropdownInterface(this, 'toggle')
Dropdown.dropdownInterface(this)
})
EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_FORM_CHILD, e => e.stopPropagation())

/**
* ------------------------------------------------------------------------
Expand Down
51 changes: 49 additions & 2 deletions js/tests/unit/dropdown.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1020,13 +1020,13 @@ describe('Dropdown', () => {
showEventTriggered = true
})

btnDropdown.addEventListener('shown.bs.dropdown', e => {
btnDropdown.addEventListener('shown.bs.dropdown', e => setTimeout(() => {
expect(btnDropdown.classList.contains('show')).toEqual(true)
expect(btnDropdown.getAttribute('aria-expanded')).toEqual('true')
expect(showEventTriggered).toEqual(true)
expect(e.relatedTarget).toEqual(btnDropdown)
document.body.click()
})
}))

btnDropdown.addEventListener('hide.bs.dropdown', () => {
hideEventTriggered = true
Expand Down Expand Up @@ -1783,4 +1783,51 @@ describe('Dropdown', () => {

triggerDropdown.dispatchEvent(keydown)
})

it('should allow `data-bs-toggle="dropdown"` click events to bubble up', () => {
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 btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]')
const clickListener = jasmine.createSpy('clickListener')
const delegatedClickListener = jasmine.createSpy('delegatedClickListener')

btnDropdown.addEventListener('click', clickListener)
document.addEventListener('click', delegatedClickListener)

btnDropdown.click()

expect(clickListener).toHaveBeenCalled()
expect(delegatedClickListener).toHaveBeenCalled()
})

it('should open the dropdown when clicking the child element inside `data-bs-toggle="dropdown"`', done => {
fixtureEl.innerHTML = [
'<div class="container">',
' <div class="dropdown">',
' <button class="btn dropdown-toggle" data-bs-toggle="dropdown"><span id="childElement">Dropdown</span></button>',
' <div class="dropdown-menu">',
' <a class="dropdown-item" href="#subMenu">Sub menu</a>',
' </div>',
' </div>',
'</div>'
].join('')

const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]')
const childElement = fixtureEl.querySelector('#childElement')

btnDropdown.addEventListener('shown.bs.dropdown', () => setTimeout(() => {
expect(btnDropdown.classList.contains('show')).toEqual(true)
expect(btnDropdown.getAttribute('aria-expanded')).toEqual('true')
done()
}))

childElement.click()
})
})

0 comments on commit 16bc47d

Please sign in to comment.