-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
add_filter( 'theme_mod_nav_menu_locations', array( $this, 'filter_theme_mod_nav_menu_locations' ) ); | ||
add_filter( 'wp_get_nav_menus', array( $this, 'filter_wp_get_nav_menus' ) ); | ||
add_filter( 'wp_get_nav_menu_items', array( $this, 'filter_wp_get_nav_menu_items' ), 10, 3 ); | ||
add_filter( 'wp_get_nav_menu_object', array( $this, 'filter_wp_get_nav_menu_object' ), 10, 2 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we not be able to re-use the methods in WP_Customize_Nav_Menus
? https://github.com/xwp/wordpress-develop/blob/57d2c46591e6593c8d6bfcb2100b6538e80f9dce/src/wp-includes/customize/class-wp-customize-nav-menu-setting.php#L231-L234
That is, use a similar approach to re-using methods from WP_Customize_Widgets
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually while debugging I saw that these filters are already used via Customize_Snapshot_Manager::preview()
.
https://github.com/xwp/wp-customize-snapshots/blob/master/php/class-customize-snapshot-manager.php#L862-L887
If the $setting
is a menu setting, the filters are set by WP_Customize_Nav_Menu_Setting::preview()
but they are not making a difference right now for the Menus section. They do work in case of widgets, e.g. the menus created via snapshot are displayed in the selection list when adding a Custom Menu
widget.
I'm right now testing the filters used in WP_Customize_Nav_Menus
as suggested to see if there's still something missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: the issue is that these filters are used only settings-specifically but the menus are initially loaded at this moment in WP_Customize_Nav_Menus
:
https://github.com/xwp/wordpress-develop/blob/master/src/wp-includes/class-wp-customize-nav-menus.php#L509
Loading the Customize_Nav_Menu_Setting
by itself to use the filters would require being related to existing setting IDs which are unknown before loading the Snapshot data. Any ideas how to still use these instead of creating new filters? Otherwise it seems that new filters are needed.
There's a second issue as well. The following function doesn't return a correct value when used with negative menu ID-s:
https://github.com/xwp/wordpress-develop/blob/57d2c46591e6593c8d6bfcb2100b6538e80f9dce/src/wp-includes/customize/class-wp-customize-nav-menu-section.php#L37-L42
E.g. nav_menu[-123]
returns 0.
I tested an existing menu I had on a site. I edited the placement/order of items and sub-items in this menu. I also added items to the menu. This menu wasn't currently set to a location. I set the menu to a registered menu location. All of these actions resulted in the information being saved in the snapshot correctly. I even went into the snapshot and made edits and saw these changes reflected |
Ok, now we need to add back the lost coverage and test with the REST API. |
Closing in favor of #55 which would merge into |
Fixes #2