Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip hidden dropdowns while focusing #29523

Merged
merged 9 commits into from
Oct 17, 2019
2 changes: 2 additions & 0 deletions js/src/dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
getjQuery,
getElementFromSelector,
isElement,
isVisible,
makeArray,
noop,
typeCheckConfig
Expand Down Expand Up @@ -478,6 +479,7 @@ class Dropdown {
}

const items = makeArray(SelectorEngine.find(Selector.VISIBLE_ITEMS, parent))
.filter(isVisible)

if (!items.length) {
return
Expand Down
9 changes: 6 additions & 3 deletions js/src/util/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,12 @@ const isVisible = element => {
}

if (element.style && element.parentNode && element.parentNode.style) {
return element.style.display !== 'none' &&
element.parentNode.style.display !== 'none' &&
element.style.visibility !== 'hidden'
const elementStyle = getComputedStyle(element)
const parentNodeStyle = getComputedStyle(element.parentNode)

return elementStyle.display !== 'none' &&
parentNodeStyle.display !== 'none' &&
elementStyle.visibility !== 'hidden'
}

return false
Expand Down
37 changes: 37 additions & 0 deletions js/tests/unit/dropdown.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1292,6 +1292,43 @@ describe('Dropdown', () => {
triggerDropdown.click()
})

it('should skip hidden element when using keyboard navigation', done => {
fixtureEl.innerHTML = [
'<style>',
' .d-none {',
' display: none;',
' }',
'</style>',
'<div class="dropdown">',
' <button href="#" class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
' <div class="dropdown-menu">',
' <button class="dropdown-item d-none" type="button">Hidden button by class</button>',
' <a class="dropdown-item" href="#sub1" style="display: none">Hidden link</a>',
' <a class="dropdown-item" href="#sub1" style="visibility: hidden">Hidden link</a>',
' <a id="item1" class="dropdown-item" href="#">Another link</a>',
' </div>',
'</div>'
].join('')

const triggerDropdown = fixtureEl.querySelector('[data-toggle="dropdown"]')
const dropdown = fixtureEl.querySelector('.dropdown')

dropdown.addEventListener('shown.bs.dropdown', () => {
const keyDown = createEvent('keydown')
keyDown.which = 40

triggerDropdown.dispatchEvent(keyDown)
triggerDropdown.dispatchEvent(keyDown)
XhmikosR marked this conversation as resolved.
Show resolved Hide resolved

expect(document.activeElement.classList.contains('d-none')).toEqual(false, '.d-none not focused')
expect(document.activeElement.style.display === 'none').toEqual(false, '"display: none" not focused')
expect(document.activeElement.style.visibility === 'hidden').toEqual(false, '"visibility: hidden" not focused')
done()
})

triggerDropdown.click()
})

it('should focus next/previous element when using keyboard navigation', done => {
fixtureEl.innerHTML = [
'<div class="dropdown">',
Expand Down