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

Fix v5 regressions in tab dropdown functionality #33626

Merged
merged 3 commits into from Apr 15, 2021

Conversation

cpsievert
Copy link
Contributor

@cpsievert cpsievert commented Apr 13, 2021

Refs #33625 (fixes the 1st problem, at least)

I'd also be happy to help address the 2nd issue, but I'm not sure whether you'd prefer to just update the docs, or actually fix the Tab logic to support .dropdown-items wrapped in <li>

<ul class="dropdown-menu">
     <li><a class="dropdown-item" href="#">Action</a></li>
</ul>

To do the latter, looks as though we need to modify the 1st part of this logic to do something closer to const dropdownElement = element.parents(SELECTOR_DROPDOWN).find(CLASS_NAME_DROPDOWN_MENU)

bootstrap/js/src/tab.js

Lines 165 to 174 in db32b23

if (element.parentNode && element.parentNode.classList.contains(CLASS_NAME_DROPDOWN_MENU)) {
const dropdownElement = element.closest(SELECTOR_DROPDOWN)
if (dropdownElement) {
SelectorEngine.find(SELECTOR_DROPDOWN_TOGGLE)
.forEach(dropdown => dropdown.classList.add(CLASS_NAME_ACTIVE))
}
element.setAttribute('aria-expanded', true)
}

@cpsievert cpsievert requested a review from a team as a code owner April 13, 2021 21:32
@cpsievert cpsievert mentioned this pull request Apr 13, 2021
2 tasks
@XhmikosR XhmikosR added this to Inbox in v5.0.0 via automation Apr 14, 2021
@GeoSot
Copy link
Member

GeoSot commented Apr 14, 2021

@cpsievert can you add a test too?

@GeoSot GeoSot moved this from Inbox to Review in v5.0.0 Apr 14, 2021
@GeoSot
Copy link
Member

GeoSot commented Apr 14, 2021

Thank you for you contibution @cpsievert
I would suggest to open another issue/PR for the second case, supporting ul > li > .dropdown-item

v5.0.0 automation moved this from Review to Approved Apr 14, 2021
@XhmikosR XhmikosR merged commit 69f5c01 into twbs:main Apr 15, 2021
v5.0.0 automation moved this from Approved to Done Apr 15, 2021
@cpsievert cpsievert deleted the tab-dropdown-fixes branch April 15, 2021 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
v5.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants