Setting active class on nav-pills > li for about, faqs, tos

It looks like there’s a small bug in setting the active class on the nav-pills for about, faqs, tos, etc. The active class is being properly set for the child link <a />, but being added to the element twice and lacking on the parent <li />.

Adding the second active class to the li would fix this for styling in my theme.

1 Like

I am not sure we ever supported an “active” class on the LI elements on nav-pills. I am not against adding this support for themes cause it seems like it could be useful. That said a bunch of stuff would need refactoring.

@techAPJ I think you moved this around, is there a reason for this “active” class? Seems like an error to me?

https://github.com/discourse/discourse/blob/d8412f409a9b98e82b269ed058bf05f88e34e886/app/assets/javascripts/discourse/templates/about.hbs#L6-L6

As to the bigger discussion of adding “active” class to LIs based on route, will leave it to @eviltrout and @awesomerobot. At the moment we use “link-to” which only gives us automatic support for route based classes on the A element. We would likely need a new component here.

4 Likes

At one point we did have an active state on some of the li elements across all the nav-pills, and in some other places we had active on the a elements…

As I recall I wanted to make things more consistent so I moved the active class to a everywhere? Putting it on the li instead is totally fine, but I think @sam is right and I went with putting it on a because there wasn’t a simple way to put it on all the lis instad…

6 Likes

Is there a way to select the parent (li) of the a with CSS? I wasn’t able to find a way to style the parent based on the child element.

The li on the topic list are classed active, so I figured that was the intention in the user navigation that was missed potentially.

No, CSS doesn’t have a parent selector. We’d need to implement a new component as mentioned above to get what you’re looking for.

5 Likes

Ok thanks @awesomerobot!