bootstrap icon indicating copy to clipboard operation
bootstrap copied to clipboard

Trying to expand a dropdown after expanding another dropdown breaks the focus

Open dibsyjr1 opened this issue 6 months ago • 3 comments

Prerequisites

Describe the issue

We've found a bug when updating to version 5.3.6, I think it's specifically related to the following commit: https://github.com/twbs/bootstrap/pull/41365

Essentially if you have more than one dropdown, expanding one and then another will break the focus as the fix above returns the focus to the previous trigger after the focus is applied to the current trigger.

In the included codepen click one of the buttons/triggers and then the other, the focus will be applied to the opposite button/trigger. I've made the focus a big red glow so that it's as clear as possible.

Reduced test cases

https://codepen.io/team/bootstrap/pen/qBamdLj

What operating system(s) are you seeing the problem on?

Windows

What browser(s) are you seeing the problem on?

Chrome

What version of Bootstrap are you using?

5.3.6

dibsyjr1 avatar Jun 03 '25 10:06 dibsyjr1

Your CodePen link doesn't have anything in it about dropdowns—please double check the link.

mdo avatar Jun 03 '25 20:06 mdo

Here's a working codepen --- https://codepen.io/nathan-muir/pen/XJbNWXW

Here's a video of the new broken focus behaviour (highlights the focused element in blue):

https://github.com/user-attachments/assets/7cb74283-6578-4e9c-9808-7cbfcddc0589


It's clear the change from #41365 will just take focus whenever completeHide is called (usually after the closing animation ends). Instead, it could check if anything has focus (eg, document.activeElement? ).

nathan-muir avatar Jun 04 '25 06:06 nathan-muir

Thanks for reporting this issue @dibsyjr1 I can reproduce directly in the documentation. I can confirm that the issue comes from 24305e7b187875c524a3c9c97ae53e47c1cc5b03.

Image

There's another issue with tab navigation from one dropdown to another:

Image


I did a quick local test with the following approach. It seemed to improve things, but also felt overly complex. There’s likely a cleaner or more efficient solution.

diff --git a/js/src/dropdown.js b/js/src/dropdown.js
index 88af932f7..7e608998e 100644
--- a/js/src/dropdown.js
+++ b/js/src/dropdown.js
@@ -208,8 +208,17 @@ class Dropdown extends BaseComponent {
     Manipulator.removeDataAttribute(this._menu, 'popper')
     EventHandler.trigger(this._element, EVENT_HIDDEN, relatedTarget)
 
-    // Explicitly return focus to the trigger element
-    this._element.focus()
+    // Return focus to trigger element if:
+    // 1. The dropdown was closed via keyboard (Escape key)
+    // 2. The dropdown was closed by clicking a dropdown item
+    // 3. The dropdown was closed by clicking outside (but not on a focusable element)
+    // 4. The dropdown was closed by tabbing to another dropdown (let the browser handle focus)
+    const isTabNavigation = relatedTarget.clickEvent?.type === 'keyup' && relatedTarget.clickEvent?.key === TAB_KEY
+    if (!isTabNavigation && (!relatedTarget.clickEvent ||
+        (relatedTarget.clickEvent && this._menu.contains(relatedTarget.clickEvent.target)) ||
+        (relatedTarget.clickEvent && !SelectorEngine.focusableChildren(document.body).includes(relatedTarget.clickEvent.target)))) {
+      this._element.focus()
+    }
   }
 
   _getConfig(config) {
@@ -379,14 +388,14 @@ class Dropdown extends BaseComponent {
         continue
       }
 
-      // Tab navigation through the dropdown menu or events from contained inputs shouldn't close the menu
+      // Don't close the dropdown if clicking on a focusable element inside the menu
       if (context._menu.contains(event.target) && ((event.type === 'keyup' && event.key === TAB_KEY) || /input|select|option|textarea|form/i.test(event.target.tagName))) {
         continue
       }
 
       const relatedTarget = { relatedTarget: context._element }
 
-      if (event.type === 'click') {
+      if (event.type === 'click' || event.type === 'keyup') {
         relatedTarget.clickEvent = event
       }

julien-deramond avatar Jun 06 '25 18:06 julien-deramond

I also stumbled across this issue, see comment in #35793 and before reading this issue I was guessing that just preventing the focus on parent if you click inside the dropdown would be enough.

Now I see that there is maybe more about this ...

Some general thoughts: #35793 was about A11y and "returning focus to parent element if dropdown is closed".

I suppose this is in someways about the definition of "closing". By default (from the autoClose option default setting):

"the dropdown will be closed by clicking outside or inside the dropdown menu"

The dropdown will be closed on any click basically. If this is the definition for "closing", returning the focus to the parent element is correct in terms of the A11y requirement "returning focus to parent if dropdown is closed".

For the example given on clicking an input outside the dropdown: In theory it might also be an "accident" clicking an input element. Just clicking outside the dropdown (maybe to close it) and hitting an input element (in the example given with the bootstrap documentation, this seems rather silly. But I can imagine a dropdown that covers lot of the screen and maybe there is just part of an input showing behind the dropdown. In such a case it might not be that unlikely to accidentally hit the input trying to close the dropdown by clicking outside of it).

So while I see the points made here, I think in someways there is a mixup between two actions (close and click). And it cannot be determined reliably in any case if the click means just closing the dropdown (and returning focus to parent) or if it means closing the dropdown and doing some other action (that implies not returning the focus to parent). That said, I suppose that "close" means "close" and thus means returning focus to parent. One can use the autoClose option to further control "closing".

stefan-korn avatar Jul 31 '25 10:07 stefan-korn

Hello!

Is the reported bug also the cause of this?

https://github.com/user-attachments/assets/ad4ad52f-5d6f-4158-b359-6134c157c091

My initial solution was to add data-bs-auto-close="inside" but it's not the way I wanted

lucas-davi avatar Aug 01 '25 21:08 lucas-davi

Reverted #41365 for v5.3.8.

mdo avatar Aug 25 '25 16:08 mdo