Skip hidden dropdowns while focusing (twbs#29523)

jeremyvii · XhmikosR · commit c1ee395f80c0 · 2019-10-17T18:01:44.000+03:00
diff --git a/js/src/dropdown.js b/js/src/dropdown.js
@@ -9,6 +9,7 @@ import {
   getjQuery,
   getElementFromSelector,
   isElement,
+  isVisible,
   makeArray,
   noop,
   typeCheckConfig
@@ -478,6 +479,7 @@ class Dropdown {
     }
 
     const items = makeArray(SelectorEngine.find(Selector.VISIBLE_ITEMS, parent))
+      .filter(isVisible)
 
     if (!items.length) {
       return
diff --git a/js/src/util/index.js b/js/src/util/index.js
@@ -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
diff --git a/js/tests/unit/dropdown.spec.js b/js/tests/unit/dropdown.spec.js
@@ -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)
+
+        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">',

Original file line number	Diff line number	Diff line change
`@@ -138,9 +138,12 @@ const isVisible = element => {`
`138`	`138`	`}`
`139`	`139`
`140`	`140`	`if (element.style && element.parentNode && element.parentNode.style) {`
`141`		`- return element.style.display !== 'none' &&`
`142`		`- element.parentNode.style.display !== 'none' &&`
`143`		`- element.style.visibility !== 'hidden'`
	`141`	`+ const elementStyle = getComputedStyle(element)`
	`142`	`+ const parentNodeStyle = getComputedStyle(element.parentNode)`
	`143`	`+`
	`144`	`+ return elementStyle.display !== 'none' &&`
	`145`	`+ parentNodeStyle.display !== 'none' &&`
	`146`	`+ elementStyle.visibility !== 'hidden'`
`144`	`147`	`}`
`145`	`148`
`146`	`149`	`return false`