search

roryashfordbentley / Wordpress-Bem-Menu

Better menu implementation for Wordpress using BEM syntax
139 stars 23 forks source link

Classes on links? #11

Closed nocean closed 8 years ago

nocean commented 8 years ago

First -- this is excellent, thanks for this! I'm looking to BEM-ify my themes, and this was a huge help.

Just wondering if you've any plans for adding BEM classes to the menu links. I realize .menu__item a works just fine, but had thought using classes over elements was a better practice, and so something like .menu__link would be more appropriate.

Either way, thanks again. Super helpful.

roryashfordbentley commented 8 years ago

Thanks for the kind words!

I love seeing how people use this and to be honest I hadn't considered adding classes to the links. I originally didn't add classes to links to keep the output light but it wouldn't add too much bloat and the change wouldn't affect existing installs either.

Once I find some time I will create a PR and update this so it can be tested.

roryashfordbentley commented 8 years ago

Should be added now: 5b4e1b3

nocean commented 8 years ago

Hey Rory, I just gave this a shot. I see the new classes on the sub-menu links, but the top-level links just have a blank class="".

Included the source functions verbatim, and don't believe I have anything going on that would modify that. Will dig a little deeper into the source and see, in case it's a local problem to my theme.

roryashfordbentley commented 8 years ago

I see the issue, It seems I was a bit hasty with the last PR. I'll reopen the issue and get this fixed :D

roryashfordbentley commented 8 years ago

@nocean I have updated the code to hopefully rectify any further issues with link classes. Give it a try and let me know if there are any further issues.

67602cf

nocean commented 8 years ago

@roikles -- just tested and it works like a charm!