Make Walker_Nav_Menu::start_lvl more flexible by introducing filters for class name(s) at least

[ruslik]

It seems that most of the posibilities to "tune" menus output through filters in Walker_Nav_Menu::start_el, have limited use cases without the ability to alter Walker_Nav_Menu::start_lvl output?

[welcher]

Thanks for the report!

Can you be a bit more specific as to the request, perhaps adding a patch or some pseudo code?

[ruslik]

Sure.

So currently if user/theme developer want to customize Navigation output, she/he has two options (as I'm aware of):

1. Extend Walker_Nav_Menu class and overwrite its methods (which in some cases maybe an overkill).
2. Hook into the following filters which unfortunately only defined for nav item markup output.
   • nav_menu_item_args
   • nav_menu_css_class
   • nav_menu_item_id
   • nav_menu_link_attributes
   • nav_menu_item_title
   • walker_nav_menu_start_el

So we have a bunch of hooks that allow us to customize item markup, but zero for level markup. I think it would be helpful to introduce similar filter hooks for start_lvl method, which responsible for ul markup.

Hope it clears things a little bit, and make sense. Thanks.

[peterwilsoncc]

Replying to ruslik:

Thanks for clarifying. I agree, adding a filter the sub-menu class would be a good thing. Naming the filter nav_menu_submenu_css_class and mirroring the arguments for the filter nav_menu_css_class.

Filtering the full output might be overkill, as end_lvl() would also need to be filtered at which point a custom walker isn't overkill.

[ruslik]

That's good.
What about nav_menu_submenu_id for id attribute as well, just in case anyone need additional flexibility?

EDIT: Although I see that it's not as trivial to implement it as nav_menu_submenu_css_class hook.

[csloisel]

Adding submenu css class filter in start_lvl()

[darthaud]

Add css filter and id filter to walker nav menu Class

[ruslik]

Replying to darthaud:

I think there also should be a way to generate unique IDs for sub-level ULs to make output valid HTML (with current patch I don't see a way to do it). Maybe introduce static class property with index or something like that and then add it to filter args.

[peterwilsoncc]

I'd like to limit any changes on this ticket to the class, rather than adding an ID too.

Adding an ID can be discussed in another ticket if needs be, as @ruslik mentions there's no way of ensuring the ID is unique in the current patch. A more complicated approach would need to be taken should it be decided to add them to core.

[raisonon]

In 36163.2.diff​

• Removed the ID completely from the code.
• Introduced late escaping to the class name variable.

[peterwilsoncc]

In 40537:

Menus: Add filter to sub-menu class in nav menus.

Add new filter nav_menu_submenu_css_class to the Walker_Nav_Menu::start_el() method, allowing themers to modify the sub menu classes output by wp_nav_menu().

Props: csloisel, darthaud, raisonon.
Fixes: #36163.

[Kopepasah]

@peterwilsoncc I think it might be better to make an empty array to not append and empty class= attribute to the <ul> element.

See: ​https://github.com/xwp/wordpress-develop/pull/228

[Kopepasah]

36163.3.diff is from ​https://patch-diff.githubusercontent.com/raw/xwp/wordpress-develop/pull/228.diff

[peterwilsoncc]

Replying to Kopepasah:

@peterwilsoncc I think it might be better to make an empty array to not append and empty class= attribute to the <ul> element.

See: ​https://github.com/xwp/wordpress-develop/pull/228

I did consider this but ultimately decided escaping the attribute as $output was populated was the most appropriate course of action. The empty attribute is benign in the case a developer delete the default class rather than replacing.

[Kopepasah]

Replying to peterwilsoncc:

Replying to Kopepasah:

@peterwilsoncc I think it might be better to make an empty array to not append and empty class= attribute to the <ul> element.

See: ​https://github.com/xwp/wordpress-develop/pull/228

I did consider this but ultimately decided escaping the attribute as $output was populated was the most appropriate course of action. The empty attribute is benign in the case a developer delete the default class rather than replacing.

Yeah, I do agree, but was following the way the menu item li was implemented. Seems awkward to implement the same solution two different ways in the same class.

[pbiron]

I like this and don't want to throw a wrench into this making it into the imminent 4.8 release, but it would be good to add it's equivalent to both the Walker_Page and Walker_Category classes as well (of course, adding those in 4.8.1 would be acceptable to me).

Related: #40359, #40666

[peterwilsoncc]

Replying to Kopepasah:

Yeah, I do agree, but was following the way the menu item li was implemented. Seems awkward to implement the same solution two different ways in the same class.

I've been sitting on the a few days pondering it, you've convinced me & I'll put it in this weekend.

[peterwilsoncc]

Replying to pbiron:

I like this and don't want to throw a wrench into this making it into the imminent 4.8 release, but it would be good to add it's equivalent to both the Walker_Page and Walker_Category classes as well (of course, adding those in 4.8.1 would be acceptable to me).

Related: #40359, #40666

If the don't already exist, could you please open separate tickets for the other walkers & send me a direct message in slack with the numbers?

[peterwilsoncc]

In 40665:

Menus: Prevent empty class attribute following [40537].

Prevents an empty class attribute, class="", from appearing in the HTML if a developer removes all classes using the nav_menu_submenu_css_class filter.

Props Kopepasah.
Fixes #36163.

[miklb]

Am I to understand this is only for the li element and there still isn't a way to filter the <a> class?

Sorry if this is the wrong place to ask the question but I've been trying to dig into custom menus lately and that has been my sticking point.

[Kopepasah]

Replying to miklb:

Am I to understand this is only for the li element and there still isn't a way to filter the <a> class?

You can add/modify the nav menu <a> element class by the nav_menu_link_attributes filter. A full list of filters can be found in wp-includes/class-walker-nav-menu.php.

[miklb]

Replying to Kopepasah:

Replying to miklb:

Am I to understand this is only for the li element and there still isn't a way to filter the <a> class?

You can add/modify the nav menu <a> element class by the nav_menu_link_attributes filter. A full list of filters can be found in wp-includes/class-walker-nav-menu.php.

ah, I read through that but didn't see a @type string $class so just assumed it wasn't covered. Now I realize I need to add it myself. Thanks.