bootstrap
bootstrap copied to clipboard
Tab active class isn't removed correctly when inside drop-down menu (5.2 regression)
Prerequisites
- [X] I have searched for duplicate or closed issues
- [X] I have validated any HTML to avoid common problems
- [X] I have read the contributing guidelines
Describe the issue
In the codepen link below, first click on the "C" tab (inside the dropdown "Menu"), then click on the "A" tab. Content for both the "A" tab and the "C" tab are still shown (the content for the "C" tab should not be shown).
It appears this bug was introduced in 5.2.0 (by changing the CDN links to 5.1.3 in the codepen, the issue goes away).
Reduced test cases
https://codepen.io/cpsievert/pen/OJvoYJb
What operating system(s) are you seeing the problem on?
macOS
What browser(s) are you seeing the problem on?
Chrome
What version of Bootstrap are you using?
v5.2.0
you miss data-bs-target
and aria-selected
attributes
Thank you for believing in me.
On Sunday, August 14, 2022, Anton @.***> wrote:
you miss data-bs-target attribute
— Reply to this email directly, view it on GitHub https://github.com/twbs/bootstrap/issues/36947#issuecomment-1214411900, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHORFCZ5QISJ7JJDFSKKADTVZEOONANCNFSM56MRRAHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>
@GeoSot has this issue actually been fixed (it doesn't appear to be)?
As you didn't respond to @WinterSilence answer after 17 days, I supposed it was closed.
Please feel free to reopen it, if his answer doesn't solve your issue
@WinterSilence's comment does not resolve the issue and I cannot reopen it.
@patrickhlauke when you have the time, you may take a look at this. I don't think it was documented, neither seems a valid behavior to me 😕 Do you believe we should handle it?
The tab JavaScript plugin does not support tabbed interfaces that contain dropdown menus, as these cause both usability and accessibility issues. From a usability perspective, the fact that the currently displayed tab’s trigger element is not immediately visible (as it’s inside the closed dropdown menu) can cause confusion. From an accessibility point of view, there is currently no sensible way to map this sort of construct to a standard WAI ARIA pattern, meaning that it cannot be easily made understandable to users of assistive technologies.
https://getbootstrap.com/docs/5.2/components/navs-tabs/#accessibility
Unless I'm missing a piece of the history, it seems we explicitly didn't include tabs with dropdowns with the 5.0.0 release, then this "regression" was patched back in, despite the documentation already suggesting it shouldn't be done. Docs were then later strengthens to make it clear that it's not something that should be done. In hindsight, the regression patch should not have been merged.
Note how the accessibility tree for the codepen example is borked, since role="tab"
elements must be directly owned by a role="tablist"
element, and you can't have links and lists like that inside a tablist.
Basically, you can either have a proper, compliant ARIA tabbed interface (in which case you can't use unorthodox constructs like tabs with dropdowns), or you have to abandon ARIA tabs altogether. We opted for the former, as it covers the vast majority of use cases. https://www.w3.org/WAI/ARIA/apg/patterns/tabpanel/
Docs were then later strengthens to make it clear that it's not something that should be done.
As a heavy user and follower of Bootstrap, I wasn't aware of this. To make it more clear, the section dedicated to using dropdowns should at least link to that accessibility note https://getbootstrap.com/docs/5.2/components/navs-tabs/#using-dropdowns
Basically, you can either have a proper, compliant ARIA tabbed interface (in which case you can't use unorthodox constructs like tabs with dropdowns), or you have to abandon ARIA tabs altogether. We opted for the former, as it covers the vast majority of use cases. https://www.w3.org/WAI/ARIA/apg/patterns/tabpanel/
I understand wanting to push for ARIA compliance, especially in terms of documentation and examples, but given the precedence for tabs within dropdowns in previous versions of Bootstrap, it doesn't seem like justification for breaking/dropping such a widely used feature. I know, for example, many users of Shiny/R Markdown (both of which builds on Bootstrap) will be surprised/upset to see this feature no longer available to them. Would you be willing to entertain a PR if I send a fix for this particular issue?
Would you be willing to entertain a PR if I send a fix for this particular issue?
I'd prefer seeing this as a 3rd party extension/plugin, not a core feature.
breaking/dropping such a widely used feature
personally, I'd argue it's a horrendous franken-pattern (is it a tab? is it a menu? yes!), even leaving aside accessibility concerns. I've not seen it in wide use in the wild.
I'd prefer seeing this as a 3rd party extension/plugin, not a core feature.
Then Bootstrap should be deprecating it first and intentionally removing it in a future major release. The current state is misleading since that the 1st tab in a dropdown does work, but clicking on any other tab breaks in a surprising way. Also, having a look at the code and #33079, it looks as though this particular bug was in oversight in #33079 and can be fixed with one line change here:
https://github.com/twbs/bootstrap/blob/a1b1e43ddc32e8aba848968023ca0e4e11fb3a55/js/src/tab.js#L235
I'll start a PR shortly with the proposed change and why that particular line of code is wrong (along with unit tests)
Then Bootstrap should be deprecating it first and intentionally removing it in a future major release
it was removed as part of the switch to 5.0.0
it was removed as part of the switch to 5.0.0
Is there any record/documentation of this? Also, in that case, why is there still this logic to toggle active class on dropdown items?
https://github.com/twbs/bootstrap/blob/a1b1e43ddc32e8aba848968023ca0e4e11fb3a55/js/src/tab.js#L235
(cc contributors/reviewers on https://github.com/twbs/bootstrap/pull/33079 @GeoSot @mdo @XhmikosR)
why is there still this logic to toggle active class on dropdown items
because shockingly enough we're only human and that slipped through?