Opened 13 years ago
Last modified 4 years ago
#18584 new enhancement
Nav menus need more hooks for extensibility (on admin page & in customizer)
Reported by: | Viper007Bond | Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 3.3 |
Component: | Menus | Keywords: | has-patch close |
Focuses: | javascript | Cc: |
Description (last modified by )
I'm trying to add some additional fields to the nav menu blocks but sadly there are no hooks for doing so. Zero.
We should add some and get in a better habit of just doing so when the code is written.
My specific suggestions are before the Remove/Cancel links (i.e. for adding standard stuff), one above the "original" field, and one after those action links (to add more actions).
We need to account for both extending the menus admin page and menus in the customizer.
Depends on: #38072
Attachments (4)
Change History (101)
#4
@
13 years ago
This is the plugin I was working on and my use case:
http://wordpress.org/extend/plugins/add-descendants-as-submenu-items/
#7
@
12 years ago
- Cc helgatheviking@… added
I need this hook for my plugin as well:
http://wordpress.org/extend/plugins/nav-menu-roles/
Right now a user cannot use my plugin in conjunction with @Viper007Bond's because we're both required to use a custom walker instead of simply adding our fields to a hook.
#9
@
12 years ago
- Keywords 3.6-menus added
- Milestone changed from Awaiting Review to 3.6
18584.diff removes the filter (handled in ticket:16738:16738.3.diff and also adds a couple of actions to the overall menu editor.
Lets see if we can't get some of these in, especially in light of the 3.6-menus refresh.
#10
@
12 years ago
Nacin noted that issues like #14134 stand in the way of making adding more fields a really good idea.
#11
@
12 years ago
The menu item link attributes filter proposed in the first patch was added in [23565].
#14
@
12 years ago
The proposed filters are great to add some new custom fields but they don't permit to create fully customized menu items.
What I would like to do is to create a group of menu items, more or less the way done by Viper007Bond in his plugin (one "group" item on backend, several items on frontend). In his case keeping the existing navigation label, title attribute and so on makes sense because the plugin has one and only one top level item. But if we want a plugin beeing able to create a group of several top level items, these fields do not make sense anymore.
So we should be able to completely customize the output of the walker (I know it is already possible by deriving a new class, but as mentionned by helgatheviking it leads to plugin conflicts).
So the easiest way I can propose is to filter the whole output at the end of the start_el function with something like:
$output .= ob_get_clean(); $output = apply_filters('walker_nav_menu_edit', $output, $item, $depth, $args);
#17
@
12 years ago
@Chouby's proposed hooks for back-compat-ish with the locations settings from #23741: ticket:23741:23741.diff
#26
@
11 years ago
- Keywords 3.9-early added
That a bit to late since 3.8 will be released this week. Let's see what we can do for 3.9
#28
@
11 years ago
I would like to add that my plugin would greatly benefit from this: Navception
Right now when you expand a Submenu Menu Item, you see the default menu item fields which aren't helpful for a submenu menu item. As I brought up in #12934, related to my plugin as well as for adding submenu items into core, it would be more useful to show a list of the links contained in that submenu rather than the Menu Label and Title Attribute.
So this is my +1 as well as request for full control over the menu item blocks.
Thanks,
Faison
#29
@
11 years ago
It doesn't look like this is making it into 3.9
Any chance we can mark this for 4.0?
There are some really great menu plugins out there and it would be great if users can use them together :)
Not to mention the possibilities for devs
#31
in reply to:
↑ 30
@
11 years ago
Replying to helgatheviking:
... can we get menus to the forefront for 4.0?
I would definitely be willing to spend much of my free time helping that cause!
#32
in reply to:
↑ 30
@
11 years ago
Replying to helgatheviking:
#14134 is possibly holding this up
Oddly I have never encountered the issue described in that ticket and I have dealt with some mega large menus.
Has anyone tested the most recent patch with their plugins? I'd be happy to test any plugins that utilize the proposed action hooks. I'm working on a site now that has a menu with 39 items. It would be the perfect test environment :). I can even try it out on a popular shared host to see if any issues pop up with saving the menu items.
#33
@
11 years ago
The only issue I've run into with large menus is that the POST size gets too large. That's something that was fairly easy to have happen. We often had to do a workaround of disabling JavaScript and using the manual up/down arrows to move around menu items.
#34
@
11 years ago
I too have run into the $_POST getting huge and failing. I've had menu items completely cut out of the menu as a result. How do we trim/optimize the $_POST size? One of the suggestions was to ajax save every item (à la Widgets) but I'm not sure that is the best approach.
#35
follow-up:
↓ 36
@
11 years ago
Any chance this will be addressed in 4.0? Or is the can being kicked down the road again?
#36
in reply to:
↑ 35
@
11 years ago
Replying to dnavarrojr:
Any chance this will be addressed in 4.0? Or is the can being kicked down the road again?
Nothing's really changed since I noted limitation concerns in 10. That said, it's possible celloexpressions might come up with a way that gets us past the $_POST
problem in his GSOC project.
#37
follow-up:
↓ 44
@
10 years ago
In the meantime, per @Shazdeh's very neat suggestion at:
http://shazdeh.me/2014/06/25/custom-fields-nav-menu-items/
I am modifying the Walker in my plugin to only add the following action hook right after the description input:
<?php // This is the added section do_action( 'wp_nav_menu_item_custom_fields', $item_id, $item, $depth, $args ); // end added section ?>
Then I am adding my custom fields to this hook. If we all do this in our plugins (those of us with plugins that rely on replacing the wp_edit_nav_menu_walker
) then our plugins can play together. People will just have to run the risk of hitting the $_POST limit... a risk they are already taking now. If this hook is ever added to core, the plugins should seamless be forward-compatible.
#42
follow-up:
↓ 43
@
10 years ago
I'm taking @helgatheviking's suggestion. Thanks for the idea! (The link to @Shazdeh's article is currently broken (404) btw.)
I can't believe something this fundamental is such a low priority, especially considering the number of duplicate tickets that have been opened, or the fact this is still outstanding for three years.
#43
in reply to:
↑ 42
@
10 years ago
Replying to zedd45:
I'm taking @helgatheviking's suggestion. Thanks for the idea! (The link to @Shazdeh's article is currently broken (404) btw.)
I can't believe something this fundamental is such a low priority, especially considering the number of duplicate tickets that have been opened, or the fact this is still outstanding for three years.
FYI: you can get to that article still through the wayback machine:
#44
in reply to:
↑ 37
@
10 years ago
Replying to helgatheviking:
In the meantime, per @Shazdeh's very neat suggestion at:
http://shazdeh.me/2014/06/25/custom-fields-nav-menu-items/
I am modifying the Walker in my plugin to only add the following action hook right after the description input:
<?php // This is the added section do_action( 'wp_nav_menu_item_custom_fields', $item_id, $item, $depth, $args ); // end added section ?>Then I am adding my custom fields to this hook. If we all do this in our plugins (those of us with plugins that rely on replacing the
wp_edit_nav_menu_walker
) then our plugins can play together. People will just have to run the risk of hitting the $_POST limit... a risk they are already taking now. If this hook is ever added to core, the plugins should seamless be forward-compatible.
FYI: Original blog is gone but you can still get a copy of this in the wayback machine:
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
9 years ago
This ticket was mentioned in Slack in #core-customize by sheri. View the logs.
9 years ago
#47
@
9 years ago
- Component changed from Menus to Customize
- Description modified (diff)
- Focuses javascript added
- Keywords needs-patch added; has-patch needs-testing 3.9-early removed
- Milestone changed from Future Release to 4.4
- Priority changed from low to normal
- Severity changed from minor to normal
I'm re-scoping this in terms of Menu Customizer which was added in 4.3 but also without all of the necessary hooks to be extensible for plugins to easily add new controls. The latest work there was done in #32832 and #32708.
In any case, adding new menu item types and adding fields to existing menu item types should ideally have the same interface whether the extensions are targeting the menus in the customizer or menus on the admin page. Maybe this isn't realistic, but at least there should be parity between the two. I know this also gets into Fields API territory.
On #32832 (comment 16), I put down some thoughts on what needed to be done for menus in the Customizer:
- Extend
WP_Customize_Nav_Menu_Item_Setting::value()
to allow plugins to amend the normal value with additional properties, where the filter can draw the properties from postmeta.- Allow for custom fields to be inserted into
WP_Customize_Nav_Menu_Item_Control::content_template()
.- Allow custom fields to be linked to the new properties in the
nav_menu_item
setting value, using a similar approach to thelink
method, although again here we're dealing with sub-properties of a setting, not entire setting value itself.- Extend
WP_Customize_Nav_Menu_Item_Setting::value_as_wp_post_nav_menu_item()
to amend the returnedWP_Post
object with the additional properties.- Extend
WP_Customize_Nav_Menu_Item_Control::sanitize()
to allow the amended values to be sanitized.- Extend
WP_Customize_Nav_Menu_Item_Setting::update()
to allow the amended values to be persisted (in postmeta)
I also put forth an assumption that the data source for any menu item custom fields should be sourced from postmeta.
#48
@
9 years ago
- Summary changed from Nav menu UI needs more hooks to Nav menus need more hooks for extensibility (on admin page & in customizer)
This ticket was mentioned in Slack in #core by westonruter. View the logs.
9 years ago
#52
@
9 years ago
This remains blocked for the admin page due to fundamental scaling problems there (#14134). It would need a complete refactoring to be able to support this. Therefore, I'd suggest focusing on the Customizer implementation.
At this point, we need help from anyone who is interested in using these hooks to move forward with adding support for custom fields in nav menu item controls. I would second the assumption that postmeta should be used and that custom fields for nav menu items is the use-case supporting hooks in nav menu item controls, but if there are other opinions, I'd be interested in seeing those.
#54
@
9 years ago
- Milestone changed from 4.4 to Future Release
Based on comment:52, seems the approach is still being determined. Moving off the 4.4 milestone.
#55
follow-up:
↓ 56
@
9 years ago
What kind of help do you need to move forward? I know we're supposed to be learning Javascript deeply this year, but I'm not there yet.
Here's a screenshot of what I am doing in the admin:
So in the Customizer I'd like to do a similar thing, adding the first row of radio buttons. And the radio buttons toggling the display of the checkboxes below as they are only needed if the "logged in" radio is selected. The whole thing is saved as 1 post meta field.
#56
in reply to:
↑ 55
@
9 years ago
Replying to helgatheviking:
What kind of help do you need to move forward? I know we're supposed to be learning Javascript deeply this year, but I'm not there yet.
[…]
So in the Customizer I'd like to do a similar thing, adding the first row of radio buttons. And the radio buttons toggling the display of the checkboxes below as they are only needed if the "logged in" radio is selected. The whole thing is saved as 1 post meta field.
Please share the plugin codebase you're using to extend the nav menu on the admin page. I'll try to prototype what you've implemented there for nav menus in the Customizer.
#57
@
9 years ago
Nav Menu Roles is available on github and in the repo too.
Specifically, I am filtering the admin menu Walker and then providing a duplicate of the default Walker with only a do_action hook added. Finally, I am adding the fields to that hook with this callback and saving the meta on wp_update_nav_menu_item
hook with the following callback
Let me know if I can clarify anything.
This ticket was mentioned in Slack in #core by sebastian.pisula. View the logs.
9 years ago
#61
@
9 years ago
Is that action hook enough to do something functional, though? That's a start but we also need to make sure that there's a way to save values from any inputs added with that hook. The best option for developers would probably be to automatically save things here based on the input name, but that could make sanitization difficult. May be worth waiting for the Fields API, and trying to integrate that here, not sure.
#62
follow-up:
↓ 63
@
9 years ago
For me, I can confirm that 1 hook in the Walker would be enough for my plugin. There is *already* the wp_update_nav_menu_item
action that is fired when the item is saved, which I am using to sanitize and save my meta.
Adding an action hook is the *only* thing that I am doing when I serve up my modified menu Walker. See [here](https://github.com/helgatheviking/Nav-Menu-Roles/blob/master/inc/class.Walker_Nav_Menu_Edit_Roles.php#L198).... served up by filtering wp_edit_nav_menu_walker
.
A handful of plugin authors got tired of waiting for this ticket so we agreed on a hook name, and added our own action hook so that no matter who's Walker gets served our plugins could play together. I don't want to speak for everyone else, but that's what I'm looking for from core.
#63
in reply to:
↑ 62
@
9 years ago
Right on, @helgatheviking. I'm one of the ones who has a plugin using the agreed upon action hook. I would love to see this in core. All the other hooks I need are already present.
#64
follow-up:
↓ 69
@
9 years ago
- Keywords needs-patch added; has-patch removed
How about a filter instead of an action? See 18584.2.diff which adds a wp_nav_menu_item_fields
filter for the fields that are output both in the walker for a specific nav menu item on the nav menus admin page and for the JS template for a nav menu item control in the Customizer. It also introduces a wp_nav_menu_manage_columns
filter for manipulating which fields appear in the advanced menu properties in Screen Options. The patch here builds upon #35273. The facility here allows existing fields to be easily removed as well as to allow new ones to be added.
I believe there are still missing pieces here, like hooking up the display to toggle displaying the fields based on whether the column checkbox is checked for any additional fields added. Also, the fields here would be dummy fields without the corresponding PHP logic (for admin menus page) or JS logic (for Customizer) to actually handle the rendering of the values and persisting of the values.
This patch is a WIP. I didn't closely examine the existing patches, but I wanted to see something that would also facilitate extending the Customizer. Please anyone iterate on it.
#65
follow-up:
↓ 66
@
9 years ago
Hi @westonruter, I don't completely understand the benefit of using a filter here in lieu of an action hook. Could you elaborate a little more on your thinking here?
How important are the screen options and display toggle? If you've added a plugin that is meant to show the meta fields it seems like you'd be unlikely to want to hide them. Just my thought, though if it doesn't hold up getting this resolved I'm certainly not opposed to it.
#66
in reply to:
↑ 65
@
9 years ago
Replying to helgatheviking:
Hi @westonruter, I don't completely understand the benefit of using a filter here in lieu of an action hook. Could you elaborate a little more on your thinking here?
The benefit of a filter is that existing fields can be removed or modified, in addition to new fields being added. With an action, you can only inject new fields at the end. A filter seems to be more flexible.
How important are the screen options and display toggle? If you've added a plugin that is meant to show the meta fields it seems like you'd be unlikely to want to hide them. Just my thought, though if it doesn't hold up getting this resolved I'm certainly not opposed to it.
The thought here is to make the API as complete as possible.
#67
@
9 years ago
Thanks! I was just wondering if there was a need for existing fields to be removed and if that could potentially cause problems.
#68
@
9 years ago
Just revisiting this.
The benefit of a filter is that existing fields can be removed or modified, in addition to new fields being added. With an action, you can only inject new fields at the end. A filter seems to be more flexible.
Is it really a good idea to let someone remove the core fields like menu title? Or is this a case of if you remove the title you deserve what you get? :) I should try out your .diff to see if I can't add my fields via the filters, but it'll probably take me a while to get to it (and other while to remember who to apply diffs).
#69
in reply to:
↑ 64
@
9 years ago
@westonruter
I started testing the trac-18584 branch of your github repository. For me, this makes the admin menu page fully functionnal. I used the new filter 'wp_nav_menu_item_fields' to remove all default WP fields, add my own fields and populate them. This replaces efficiently the js hack that I currently use to obtain the same result. To save the fields, your current patch did not require any change to my 'wp_update_nav_menu_item' filter.
The "dummy" fields correctly appear in the customizer. I am ready for further tests when you'll provide a way to populate the fields and save them.
Replying to helgatheviking:
Is it really a good idea to let someone remove the core fields like menu title?
This is what I already do. Well, in fact, I replace the title and url fields by hidden fields.
#70
@
9 years ago
Do you mean https://github.com/xwp/wordpress-develop/tree/trac-18584 ? Since it is on git it will be 100x easier for me to take a look at and I will try to test it out with my Nav Menu Roles plugin.
#72
@
9 years ago
I would like to see this implemented in the core aswell.
At this point I use the same method as Chouby (js hack).
It would be a huge improvement for consistency and stability if we can use core hooks to alter the menu items.
I think the proposal (#comment:64) of @westonruter for using a filter instead of an action would be a great.
Appart from removing existing fields you can even change the order of fields by using a filter.
This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.
9 years ago
#74
@
9 years ago
Finally getting around to reviewing @westonruter's patch, I think this approach works pretty well (using a filter). As I just noted on github, there will need to be a separate filter for the customizer as the html there is a JS template, which will most likely be different from what you would put for the admin page.
My main concern with completing the patch is that the saving process should be as automatic for developers as possible in both places (and also previewing for the customizer). In addition to the items noted in 64.
#76
@
9 years ago
Looking at WP_Customize_Nav_Menus::available_items_template()
there seems to be no way to create additional menu items. In the admin area I can use add_meta_box()
to define a new menu item. I am working to create a new menu item which is line break and a title.
#78
@
9 years ago
Could we also a specific class for the "Navigation Label"? All of the other sections have something like "field-url". Perhaps we could use <p class="field-title description description-wide">
I built a plugin for a client's website and used the version of the class that @westonruter created.
One other problem that I came across was that I when I created a new field the data was not transferred with the ajax request that WP made when adding a new menu. The reason is that WP has a list of fields that it fetches the data from. https://github.com/WordPress/WordPress/blob/4.5-branch/wp-admin/js/nav-menu.js#L208-L222
I could continue the work that @westonruter started but need a bit of guidance and help with the JS and customizer problems.
This ticket was mentioned in Slack in #core by iamntz. View the logs.
9 years ago
This ticket was mentioned in Slack in #meta-i18n by danieliser. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
8 years ago
#82
@
8 years ago
I've been spending some time this week working on a path to finally facilitating custom fields for nav menu items in the customizer.
I have an alpha-state feature plugin that can be played around with: https://github.com/xwp/wp-customize-nav-menu-item-custom-fields
Note that this plugin has two separate plugins embedded within it: “Customize Nav Menu Item Custom Fields” and “Customize Nav Menu Item Custom Field Examples”. The latter includes an example that adds a basic implementation of @helgatheviking's Nav Menu Roles plugin (allowing a nav menu item's display to be conditional based on whether a user is logged-in, logged-out, or if it should be shown for anyone.) You'll note that, being that the customizer is a JavaScript application, the integration here looks very different than how custom fields might currently be integrated with nav menu items in the nav menus admin page. The integration relies primarily on JavaScript events rather than PHP actions and filters. Example: https://github.com/xwp/wp-customize-nav-menu-item-custom-fields/tree/master/examples/user-visibility
Note that since custom fields for nav menu items are postmeta, this feature plugin depends on Customize Posts which implements the necessary interface to represent postmeta in the customizer. You'll have to use the current develop
branch to test the plugin: https://github.com/xwp/wp-customize-posts
There is one key limitation currently with the plugin: it isn't possible to manage custom fields for newly-added nav menu items that haven't been saved yet. This is due to how we're using placeholder IDs as opposed to actual IDs, and postmeta aren't able to work with such placeholder IDs in core currently. For more on this, see #38072.
This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.
8 years ago
#84
@
8 years ago
- Description modified (diff)
- Milestone changed from Future Release to 4.8
Milestoning for 4.8 since nav menu items closely align with the initiative on blocks: A nav menu item, widget, and editor content block should all be the same underlying thing.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
#87
@
8 years ago
- Milestone changed from 4.8 to 4.8.1
Punting to 4.8.1 per bug scrub earlier this week.
#89
@
7 years ago
I was really disappointed to find that 6 years old ticket is still not resolved, with regularly moved milestone. Recently I was in need to create an image based front page menu with manual cropping. I had to base it on my custom post type, prepare the whole customizer structure and finally can use WP_Customize_Cropped_Image_Control
to allow users manual cropping to required size. And still, have to prepare management of items hierarchies.
It would be highly appreciated to finally allow customizing nav_menu_items with custom metadata, one of which is definitely managing related thumbnail_id with WP_Customize_Cropped_Image_Control
. By the way for consistent user experience cropped image control should be useable also in regural wp-admin pages, not only in customzier.
#90
@
7 years ago
- Milestone changed from 4.9 to Future Release
Punting to future release because menus are going to going to get components built for them in Gutenberg and we can then leverage the extension mechanisms developed for that component in core.
This ticket was mentioned in Slack in #core by helgatheviking. View the logs.
7 years ago
#93
@
6 years ago
I added a related ticket #47056 with a very narrow goal to just get the 'wp_nav_menu_item_custom_fields'
hook added that is first mentioned in this ticket by @helgatheviking.
I did some research to find the following information about existing plugins on wordpress.org:
60
already implement the'wp_nav_menu_item_custom_fields'
hook156
plugins use the'wp_edit_nav_menu_walker'
hook
Given that only one (1) plugin can create a custom walker without conflicting with another plugin that does the same, it seems there is a lot of chaos "in the wild" that has been created by the lack of this hook. Here's hoping we can finally reign in this beast, and even dedicate the fix in memory of the original poster of this ticket?
#95
in reply to:
↑ 94
@
5 years ago
Replying to SergeyBiryukov:
Thank you!
#97
@
4 years ago
- Component changed from Customize to Menus
- Keywords close added
Switching component back to menus because this does apply to both menu management interfaces.
The long-term direction is to replace menus with blocks (as noted in 90). The approach there offers more flexibility at a user level, although it may actually be more difficult for plugins to extend in the way that this ticket discusses. That work is happening in the Gutenberg plugin. In the meantime, a single hook was added in [47190] to address the most common need for extensibility in the current menus system.
Therefore, this ticket could probably be closed when (and if) the current menu management interfaces are replaced with blocks. That has been proposed and in process potentially for 5.8 in wp-admin but not yet started for the customizer.
Related: #18527, #16738, #14414, #16691