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

Missing classes #83

Closed
designlobby opened this issue Aug 1, 2024 · 1 comment
Closed

Missing classes #83

designlobby opened this issue Aug 1, 2024 · 1 comment
Assignees

Comments

@designlobby
Copy link

I'm new to navi v3 and trying to familiarise myself - I'm getting an issue where the default menu classes aren't generating apart from (when on the relevant page):

  • current page classes
  • current page parent classes
  • custom classes

I'm looking in MenuBuilder.php and think this may be expected behaviour(?).

I can work with the @props classes, but I'm also not seeing the menu-item-has-children class on parent items, and it doesn't look to be in the $disallowedClasses list. Should we be adding our own custom classes for this now?

Thank you!

@Log1x
Copy link
Owner

Log1x commented Aug 1, 2024

Checking current default behavior for ->classes, it is actually wrong and classes such as:

menu-item-type-taxonomy menu-item-object-category current-menu-item

should be getting trimmed off with this originally being a Str::contains() check before rewriting Navi for non-Acorn WordPress installs.

I hadn't noticed as I never use or depend on ->classes unless I'm expecting a custom menu item class and instead will just check ->active and other conditions and add my Tailwind classes accordingly.

By design, Navi is supposed to remove all default classes so you entirely control the markup through and through.

That being said, this definitely deserves proper tests, a major release "fixing" the current default classes not getting trimmed off as originally intended (also fixing #78), and a chain-able method on the Navi instance so you can customize/control the default class disallow behavior for edge-cases or preference.

I will try to do all this when I get time. 😅

@Log1x Log1x self-assigned this Aug 1, 2024
@Log1x Log1x mentioned this issue Oct 1, 2024
@Log1x Log1x closed this as completed in 76c39df Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants