Log1x / navi

A developer-friendly alternative to the WordPress NavWalker.
https://github.com/Log1x/navi
MIT License
311 stars 29 forks source link

Custom classes excluded #41

Closed mike-sheppard closed 3 years ago

mike-sheppard commented 3 years ago

Hey @Log1x! πŸ‘‹πŸ»

It seems custom classes added to nav items aren't being added to $item->classes.

Screenshot 2021-04-21 at 17 08 00


Had a quick scan through the code and noticed this is excluding everything except what's in the exclusion list? Unless I'm misreading it.

https://github.com/Log1x/navi/blob/master/src/MenuBuilder.php#L90-L95

$classes = array_filter($item->classes, function ($class) {
    return array_key_exists(
        $class,
        array_flip($this->classes)
    );
});

Excluded classes (however they're being allowed). Not sure if this is intentional and simply a typo in the comment: https://github.com/Log1x/navi/blob/master/src/MenuBuilder.php#L37-L50

/**
  * Blacklisted Classes
  *
  * @var array
  */
protected $classes = [
    'current-menu',
    'current_page',
    'sub-menu',
    'menu-item',
    'menu_item',
    'page-item',
    'page_item',
];

I'll try debugging a bit later on and pop a PR over if I can fix, just thought I'd submit this issue in case you know of a solution off the top of your head.

Cheers!

jellycode commented 3 years ago

@mike-sheppard I have a pull request in for this issue ( #39 ) already. I couldn't quite figure out what the filter was doing so I just removed it, and everything seems to be working as usual again, but perhaps that isn't the best approach. It got me where I needed to be for the time being at least.

mike-sheppard commented 3 years ago

Thanks @jellycode I did see that PR, nice one! I think the idea behind it is to exclude a bunch of the default nav items class bloat, which I'm all for! I'll pop a PR over shortly that preserves this + custom classes :)

kamilgrzegorczyk commented 3 years ago

@Log1x thank you for your effort!

Is there any plan for releasing that anytime soon?

Twansparant commented 3 years ago

First of all, thanks for another great package! I would like to know the same about the classes filter, right now all my custom classes are excluded in the output. Thanks!

kamilgrzegorczyk commented 3 years ago

Hey @Twansparant - for the time being I switched my projects to install navi as dev-master Its an ugly solution but does the job for the time being.

Twansparant commented 3 years ago

Hi @kamilgrzegorczyk, thanks that's what I ended up doing too!

Log1x commented 3 years ago

release pushed – sorry for the delay.

kamilgrzegorczyk commented 3 years ago

Thank you very much @Log1x !