-
-
Notifications
You must be signed in to change notification settings - Fork 78.8k
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
Nav Pills with open dropdown don't reflect the nav-pills active state… #22513
Conversation
scss/_nav.scss
Outdated
color: $nav-pills-active-link-color; | ||
background-color: $nav-pills-active-link-bg; | ||
} | ||
.nav-link[aria-expanded="true"], |
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 don't put aria roles in our selectors. This should likely be removed.
scss/_nav.scss
Outdated
} | ||
.nav-link[aria-expanded="true"], | ||
.nav-link.active, | ||
.nav-item.show .nav-link { |
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.
Consider removing the .nav-item
from .nav-item.show .nav-link
so it's a slightly less specific selector?
3a786c4
to
a5ab2b5
Compare
I removed the aria role selector and made |
…. The active state should also be reflected when using collapse plugin inside nav-pills.
a5ab2b5
to
d71f3c7
Compare
This PR will close : #23482 |
The active state should also be reflected when using collapse plugin inside nav-pills.
Latest v4-dev branch doesn't work like the current docs site http://v4-alpha.getbootstrap.com/components/navs/#pills-with-dropdowns. I think it was introduced at 91b6294.
Fixes #23482.