-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Dynamic tabs: use buttons rather than links #32630
Conversation
- change docs - tweak styles to neutralise border and background
set to draft for now while we wait for #32631 - once that is in place (removing the forced only minor concerns:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should include some docs mentions here for why we use <button>
elements here.
- replace links with buttons - make one specific test that uses links instead of buttons, as we still want to support it despite it being non-semantically appropriate - remove tests involving dropdown tabs, as they're discouraged (though still silently supported)
1547c5b
to
34f38ce
Compare
added a short sentence to explain. also took opportunity to update the visual and unit tests |
5010e23
to
1866c27
Compare
1866c27
to
b4a3db8
Compare
🚀 |
Has this been published with Using the exact code from the docs with
Btw. would love to use |
@MickL if you're using the exact same code as in the docs, but it's looking different from https://getbootstrap.com/docs/5.0/components/navs-tabs/#javascript-behavior, then it's likely some other modification/customisation you're doing that's affecting this? if not, please submit a fresh issue with a reduced test case, in case we've missed some nuance of how styles interact with each other in specific conditions |
@patrickhlauke obviously it is exactly the same BUT i changed The issue I opened has been closed in favor of this PR: #32945 |
@MickL Our current docs example (linked by Patrick) is working fine with If you still see any issue related to this in beta 21, please open a new issue—and stop spamming an already merged PR that we cannot re-open. |
this is more semantically correct. note that currently, the forced
:focus
in reboot styles is making this look rather ugly - see separate PR #32631 to remove that cargo culted (?) style from reboot.Preview: https://deploy-preview-32630--twbs-bootstrap.netlify.app/docs/5.0/components/navs-tabs/#javascript-behavior