Navigation Screen: Menu item custom ACF options

[amir2mi]

Advanced Custom Fields (ACF) is a popular plugin which developers use for their themes.
It has the feature to add custom options to each menu item, therefore, users can customize that menu item and the theme will show the menu depends on that settings, these options can be menu item color, icon, and so on.
I found that the new Navigation editor does not support those custom options.

Classic navigation:

New navigation:

[CreativeDive]

@elliotcondon

[elliotcondon]

Hi all 👋.

Elliot here representing the Advanced Custom Fields plugin. We are very interested to know what plans are in place to handle backwards compatibility for the PHP wp_nav_menu_item_custom_fields action introduced in WP 5.4 allowing custom HTML to be added into menu items.

Please note that ACF is not the only plugin using this action 👍.

[getdave]

Thanks again for raising this @amir2mi.

@elliotcondon This is on the project board and we'll be testing this shortly in order to review ways in which we can support the wp_nav_menu_item_custom_fields action. Would you be interested in helping to test any PRs that relate to this feature?

[elliotcondon]

@getdave Absolutely. Where can I sign-up and stay notified on these upcoming changes?

[getdave]

@elliotcondon That's great - thank you. Perhaps it's easiest if we use this Issue. I will ping you for review once we get to resolving this.

Also you might be interested in the Navigation Editor and Navigation Block project boards (but don't feel obliged to dive in there).

Thanks for agreeing to be involved. Much appreciated.

[getdave]

Just performed a quick test to illustrate what I believe we're discussing here.

Screen.Capture.on.2021-05-10.at.12-51-39.mov

Essentially with ACF (and other Plugins) you can register fields (of many types) to appear on the Menus screen. That data is then saved as post meta and rendered as required on the Menus screen or on the Theme front end.

In ACFs case they are making use of it here to render additional form fields for each menu item on the Menus screen.

[getdave]

As part of this discussion, let's also explore the alternatives that the Nav Editor / Gutenberg can expose which will mirror the same functionality.

[spacedmonkey]

At a guess, all these custom fields are stored in post meta on the menu item. So have you considered using the register_meta function? For each field that is registered, register a new meta field. This would allow the existing menu item endpoint to save the data.

As for rendering form inputs, I have a couple of idea.

• For every registered field of post meta, render a div on the page with a unique id, that relates to the meta key. So <div id="field-color">. Then you could use javascript to insert a input field and handle saving.
• For every registered field of post meta, use the type, like boolean or text to render a input and handling saving. So string, would render input type="text" or boolean would render a checkbox. This gets a little complex with fields like color or dropdown.
• Render the form inputs using an iframe or similar in the REST API output them. Similar to how meta boxes and widget output is handled currently.

[getdave]

One thing I'm considering is that wp_nav_menu_item_custom_fields doesn't just cater for form fields. It's open to any HTML.

I wonder whether if (as you suggest) we were to render the HTML within an iframe (like we do for Legacy Widgets) we might be able to provide backwards compatibility in all cases.

Trying to guess whether the developer is rendering a <form> (or similar) might be prone to error.

Also worth noting that Gutenberg provides APIs to add fields to blocks out of the box and as a result developers can/will be able to add fields to all the blocks that are available in the Nav Editor. I'm not saying that's good enough as a solution, but we should start to signpost developers towards these new APIs early in order that they can provide their users with the best possible experience.

Perhaps it would be good to get input from folks who were heavily involved in the Widgets project - @noisysocks @draganescu?

[lgladdy]

Hey @spacedmonkey @getdave. I've been following this thread for the ACF team.

Switching to register_meta would be a significant undertaking for us, given the amount of flexibility users have with creating and removing fields, plus our backwards compatibility requirements.

We only had to make a small change for an issue with verifying nonces (as we don't have access to $_POST) with the new widget block editor which maintains backwards compatibility like @getdave said, so that route feels like the best if it's viable.

[noisysocks]

Hm, very tricky!

I wonder whether if (as you suggest) we were to render the HTML within an iframe (like we do for Legacy Widgets) we might be able to provide backwards compatibility in all cases.

Yes I think this is all that we can realistically do. Perhaps display the output of wp_nav_menu_item_cusotm_fields inside a <form> inside an <iframe>¹ inside a popover that you open from the Navigation Link's block toolbar or block settings.

The form would have to contain all of the same hidden <input> fields that exist in core. Similar to what Legacy Widget does here.

Something sensible that results in backend hooks firing would have to happen on form submit². Perhaps the action="" can be a PHP file that exists somewhere. This part I'm not too sure about.

This approach wouldn't be 100% backwards compatible³ but should hopefully make transition easier as @lgladdy says.

Trying to guess whether the developer is rendering a <form> (or similar) might be prone to error.

We shouldn't need to deduct anything from the output of wp_nav_menu_item_custom_fields. We just have to invoke wp_nav_menu_item_custom_fields with the same or similar conditions as it is invoked in core. Specifically, within a (fake) <form>.

---

¹ An iframe might not be necessary. Legacy Widget doesn't use one for the form.
² How Custom Fields work in the post editor could serve as inspiration here, too.
³ Nothing is 100% backwards compatible when, in WordPress, a hook or filter can access the entire application's state.

[spacedmonkey]

Switching to register_meta would be a significant undertaking for us

I strongly recommend you do this in general, not only for menu items, but for all data that you save in meta ( user / post / comment etc). I believe it would map pretty easily and would add some massive benefits to compatibility with core and your users. @lgladdy @elliotcondon , do feel free to reach out privately via slack or email, I can create a proof of concept PR for you, if you so wish. I understand this is a long term goal and isn't a solution this direct problem, just something I believe you should do. For context, I have worked a lot on register meta in core, so will be able to help a lot here.

Trying to guess whether the developer is rendering a

(or similar) might be prone to error.

We don't need to guess this, we know that core wraps it is <form tag. See the action in core, there are buttons and fieldsets line below, it is in a form.

Regarding wp_nav_menu_item_cusotm_fields, we still have to support this, not only because ACF but other plugins that use it. It worth noting that seems like their are lots of reference in the WP plugin repo to it.

I have the following recommendation.

• Add a new field the menu item REST API endpoint, called menu_item_custom_fields ( or similar ). This will wrap a call to wp_nav_menu_item_cusotm_fields in an output buffer and get the field.
• When render the nav editor, render the menu_item_custom_fields in the sidebar in the link settings section ( wrapped in a form).
• Create a new endpoint, called /menu-item/legacy_save, this will handle saving data submitted form.
• On save of menu, loop around all the menu_item_custom_fields forms, submit form data to /menu-item/legacy_save

This is just rough idea for now that has some holes, like how it might work in the customize but a solution like this may work. Thoughts?

[spacedmonkey]

For some extra context, there is a new tracking ticket for backwards compat #35157

[spacedmonkey]

For some extra context, I have prototyped added support for wp_nav_menu_item_custom_fields. This wraps a call to the wp_nav_menu_item_custom_fields action in an output buffer and returns the output as a string, under a field called item_custom_field.render.

Checkout my branch. To test, create some menu items and call await wp.apiFetch({ path: '__experimental/menu-items?context=edit'}) in the gutenberg editor.

Sadly in my testing, my workaround didn't work.

I believe there is some conditional logic in the ACF plugin that means that fields not loading the REST API. I couldn't work it out in my limited tests.

I did however see this PR work, by installing menu-icons

@lgladdy any idea why this fields would not be showing on the rest api if we using the wp_nav_menu_item_custom_fields action?

[lgladdy]

@spacedmonkey Can't see anything obvious that would stop us rendering so long as at least 4 parameters are passed into wp_nav_menu_item_custom_fields.

I'll try and get your branch running and test this further with ACF.

[lgladdy]

@spacedmonkey Because ACF supports different fields based on the menu, we also need to know what menu ID is currently being edited when we render the fields for it. Currently, we do this with this code on a wp_edit_nav_menu_walker filter to store the ID of menu being edited.

Could this be passed through as part of argument 4 ($args) on the wp_nav_menu_item_custom_fields filter so we can read it from there instead?

[spacedmonkey]

@lgladdy I think the data being passed to wp_nav_menu_item_custom_fields is correct. In my limited testing, found that even through the correct data was being passed too this function, wasn't getting an field group back. I believe it is because of the acf_get_field_group_visibility. I believe that the field group is only be loaded conditionally on the menu editor. I am not sure this is the case, it is what I thought it was. If the field group is only conditionally loaded, to get this to work, it will need to be loaded in the rest api. Again, not sure that this is what the happening.

[lgladdy]

@spacedmonkey Yeh - agreed the data is correct, but ACF uses other filters prior to wp_nav_menu_item_custom_fields in order to detect the nav menu ID currently being viewed, and this context isn't available when rendered via the REST API. ACF's currently failing to render any fields because it's trying to check which fields should be displayed in that nav menu, but doesn't know what nav menu is being viewed.

My request to add the menu ID to the $args is a feature request more than anything, but one ACF will need in order to know what fields to render, one which shouldn't have any issues with backwards compatibility as we'd just be adding a new key to on the $args array.

Whilst we can probably work around it not being there in the case of existing menu items by using the term meta lookup to get the menu ID; New menu items added would need to be assigned to a menu before we could detect the fields required, unless we can get the menu ID into that filter.

Edit: Specifically, this code is trying to use the existing loaded context to know what menu ID is being displayed. This isn't set, so ACF never believes there are any fields assigned to that nav menu.

[spacedmonkey]

My request to add the menu ID to the $args is a feature request more than anything

@lgladdy Correct me if I am wrong, but can't you already get the menu id from the wp_nav_menu_item_custom_fields action. The last param on this action is Nav menu ID., which is the menu id. This action, has both menu item id and menu id, which is all the context you should need.

[lgladdy]

@spacedmonkey Weirdly, that $id parameter always seems to be 0 inside this filter on the normal nav menu pages, but it is working correctly in your REST API branch.

Here's a build of ACF 5.10.2 which adds support for using it to know the menu ID, and this is working with your branch now and our fields are rendered in your item_custom_fields[rendered] key.

[Mamaduka]

I'm closing this issue since the navigation screen is no longer actively developed.

Related: #43620.