-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
…wp-customize-snapshots into bugfix/saving-menus-issue_2
@miina & @westonruter I'm thinking we may need to handle the transition to publish in this PR before it's ready for merge. Thought? |
@valendesigns transition to publish in what way? |
$manager->snapshot()->is_preview = true; | ||
|
||
$menu_items = array(); | ||
$menu_items[] = $this->manager->value_as_wp_post_nav_menu_item( |
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.
I think we can avoid duplicating the WP_Customize_Nav_Menu_Item_Setting::value_as_wp_post_nav_menu_item()
method here by doing:
$setting_id = sprintf( 'nav_menu_item[%d]', static::MENU_ID );
$nav_menu_item = new WP_Customize_Nav_Menu_Item_Setting( $this->customize_manager, $setting_id, array(
value: array( /* ... */ )
) );
$menu_items[] = $nav_menu_item-> value_as_wp_post_nav_menu_item();
@valendesigns @miina I believe this will reduce the overall amount of code and re-use what is in With those changes merged into this PR, the resulting diff is much smaller: develop...bugfix/saving-menus-issue_3 I tried creating a new nav menu, assigning it to a nav menu location, and adding new nav menu items to it. I created the snapshot and loaded it on the frontend in both an authenticated and incognito window, and the snapshotted menu and nav menu items appeared as expected for me in both cases. Please review. |
… into bugfix/saving-menus-issue_2
… snapshot values will be added properly
Re-use methods in WP_Customize_Nav_Menus in favor of duplication
I think this is finally ready! 😂 |
Fixes #2