Add action hook in class-walker-nav-menu-edit.php

[timersys]

The class-walker-nav-menu-edit.php is lack for a simple action hook that let authors add their own fields to the admin menu.
In the last week I found 4 different users complaining their plugins/themes are not compatible ( Ubermenu, Geotargeting Pro, Salient theme and another theme I can't remember) because every of them are replacing the whole walker.
The whole menu ecosystem could simple work by adding a hook ( patch added ) and makes totally sense.

We already have filters/actions for settings wp_setup_nav_menu_item , wp_update_nav_menu_item so I don't see a reason of why not having a hook to add the gui of this fields without modifying the whole walker.

Related to https://core.trac.wordpress.org/ticket/18584 but could solve lot of issues until that ticket from 5 years got solved and merged.

[welcher]

Thanks for the patch!

There was a change on line 73 in the diff that seems out of place with the intent of the patch.

I think we may need a more descriptive name for the action. add_menu_settings is too generic. Perhaps something like, admin_nav_menu_item_below_settings?.

Patch updated to address the above, some documentation standards and to generate from the root of the repo instead of /src

[welcher]

I took another look at the original patch and there were a fair amount of markup changes that had nothing to do with adding the action. I've reverted those and uploaded a new patch containing just the new action.

@timersys can you review it and be sure it's still what you were wanting to do?

[timersys]

@welcher it works perfectly. Not sure what I did with the original patch that got messed. Glad you catch it!
We just need to instruct plugin authors to use the hook instead of replacing the entire class now :)

[welcher]

Thanks for checking! We just need someone to commit this now :)

[adamsilverstein]

Related: #27280, #14414, #18889 and #18584.

[adamsilverstein]

@timersys & @welcher - thanks for the bug report and patch.

My concern with the current patch is it is so narrowly focused & application/need specific. While it solves the reported use case, what if plugins want to add something to a submenu or alter the classes added to menus? We might need many hooks to address every use case. I think this is the reason the general approach of replacing the walker has been used, and was the resolution for #13342.

A more generic filter applied to the return of start_el might be worth considering (as suggested in #27280). This would still let you add menu items (although it would require a bit more care to find the correct insertion point) without replacing Walker_Nav_Menu_Edit entirely.

What do you think @timersys, would that still be useful?

[timersys]

HI @adamsilverstein , thanks for looking into it. Adding the filter at the end of the generated HTML looks ugly and dirty imo.
I would rather prefer to add filters and hooks everywhere to cover as much use cases as possible. Is not what wordpress does anyway :)

Adding just that hook will make lot of people happier. And I don't mean just developers. I found lots of complaints from users because their plugins are not compatible between each other because they simply override the walker.

[adamsilverstein]

Replying to timersys:

just that hook will make lot of people happier. And I don't mean just developers. I found lots of complaints from users because their plugins are not compatible between each other because they simply override the walker.

This is a primary reason hooks are preferable! What other actions do you think would be worth considering as part of this change?

[timersys]

38904.3

[timersys]

I added a couple of filters that should cover most cases. But I guess once developers actually start using this, with more feedback we will be able to tune it up.

[crstnio]

I want to build multiple small plugins to extend menu items. But currently, this is not possible.

We need hooks inside Walker_Nav_Menu_Edit. The more, the better.

[MikeSchinkel]

Hi @adamsilverstein - I am running into this very same problem with the [If Menu plugin](​https://wordpress.org/plugins/if-menu/) a client started using on their site.

What is the reason this stalled 2 years ago, and is there anything we can do to put onto the radar for next release?

I would question if the hook names in the patch are consistent with the rest of the hook names in nav-menu.php of if they should be updated. And if someone needs to have a go at updating this patch I can certainly do so, assuming there is some reasonable chance we can get in a near future version of WP.

[adamsilverstein]

@MikeSchinkel Thanks for the ping, I will review this for the 5.3 cycle. Do you feel the latest patch is sufficient, or would additional hooks help? In terms of the naming, what would you suggest for the hook names?

[MikeSchinkel]

@adamsilverstein Thanks for the reply.

After reviewing this and the ticket that this is somewhat a duplicate of I created a new ticket #47056 with a single goal of just getting one hook added that 3rd party plugins are already modifying the Walker class the patch suggested in #18584 and the one my new ticket asks for an up or down vote to include.

IOW, rather than get stalled on the perfect set of hooks I am hoping we can move forward with one we know has been used "in the wild" because there is a know need for it, and then deal with the others as edge cases when they come up?

[SergeyBiryukov]

In 47190:

Menus: Introduce wp_nav_menu_item_custom_fields action that fires just before the move buttons of a nav menu item in the menu editor.

Props MikeSchinkel, birgire, sebastian.pisula, desrosj, helgatheviking.
Fixes #47056. See #38904, #18584.

[adamsilverstein]

@welcher or @timersys - did the changes committed in https://core.trac.wordpress.org/changeset/47190 resolve this issue for you, or do still need an additional action fired here?