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

Nav Pills with open dropdown don't reflect the nav-pills active state… #22513

Merged
merged 1 commit into from
Oct 3, 2017

Conversation

pat270
Copy link
Contributor

@pat270 pat270 commented Apr 24, 2017

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.

scss/_nav.scss Outdated
color: $nav-pills-active-link-color;
background-color: $nav-pills-active-link-bg;
}
.nav-link[aria-expanded="true"],
Copy link
Member

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 {
Copy link
Member

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?

@mdo mdo added this to the v4.0.0-beta.2 milestone May 26, 2017
@pat270 pat270 force-pushed the nav-pills-dropdown-highlight branch from 3a786c4 to a5ab2b5 Compare May 30, 2017 18:37
@pat270
Copy link
Contributor Author

pat270 commented May 30, 2017

I removed the aria role selector and made .nav-item.show .nav-link less specific. I used the selector .nav-item.show .nav-link so the highlighted styles wouldn't bleed to nested .nav-link's e.g. using the collapse plugin inside .nav-pills.

@mdo mdo removed this from the v4.0.0-beta.2 milestone Jun 18, 2017
…. The active state should also be reflected when using collapse plugin inside nav-pills.
@Johann-S
Copy link
Member

This PR will close : #23482

@mdo mdo merged commit 9ee2fbe into twbs:v4-dev Oct 3, 2017
@mdo mdo mentioned this pull request Oct 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants