Skip to content
This repository has been archived by the owner on Dec 27, 2022. It is now read-only.

Re-use methods in WP_Customize_Nav_Menus in favor of duplication #56

Merged

Conversation

westonruter
Copy link
Contributor

No description provided.

@miina
Copy link
Contributor

miina commented Jul 5, 2016

@westonruter It's not working for me right now. Created a new menu, saved it in a Snapshot:
screenshot from 2016-07-05 11 08 13

Then went to frontend to Customizer and the menu didn't appear:
screenshot from 2016-07-05 11 07 08

The menu did appear after Publishing the Snapshot.
Also tested with the menu locations and although they are saved in a Snapshot, they are not displayed in the frontend (same issue as before).

I double checked that I am using the code with the new filters and tested in on 2 different WP environments. No errors in console. @valendesigns and @westonruter do you both see the Snapshot menus working properly with the pull from bugfix/saving-menus-issue_3?

@miina
Copy link
Contributor

miina commented Jul 5, 2016

It seems to me that the reused filters handle the menu too late -- it needs to be filtered when loading the menu for the first time in Customizer, here: https://github.com/xwp/wordpress-develop/blob/master/src/wp-includes/class-wp-customize-nav-menus.php#L510

What am I missing that the re-used filters don't work for me?

@westonruter
Copy link
Contributor Author

@miina @valendesigns Reported the issue with the WP_Customize_Nav_Menu_Section not allowing negative IDs: https://core.trac.wordpress.org/ticket/37293

Fixed in https://core.trac.wordpress.org/changeset/37981

This will be part of 4.6-beta2 tomorrow (or so).

@westonruter
Copy link
Contributor Author

@miina @valendesigns fixed another bug in core related to nav_menu_items[...] settings not getting loaded into the Customizer as expected: https://core.trac.wordpress.org/ticket/37294

Again, will be part of 4.6-beta2.

@westonruter
Copy link
Contributor Author

@miina Please update your WP core trunk to at least r37982 and test again with this PR. If it is working for you now, please merge this PR (which will amend the other PR, #55).

@valendesigns
Copy link
Contributor

Works for me

@miina
Copy link
Contributor

miina commented Jul 6, 2016

@valendesigns @westonruter It does work for me but actually it seems that this way the menu filters wouldn't be necessary at all to add to Customize_Snapshot_Manager, they already come with the core from WP_Customize_Nav_Menus.

It seems that the only thing needed would be this: add_action( 'customize_register', array( $this, 'preview_early_nav_menus_in_customizer' ), 9 );.

@westonruter
Copy link
Contributor Author

@miina good question, but adding the filters need to get added for the sake of unauthenticated users (or users who lack edit_theme_options) due to this: https://github.com/xwp/wordpress-develop/blob/946e6ac8de2baac007286dffc8dc5be130cc1c5e/src/wp-includes/class-wp-customize-nav-menus.php#L51-L54

@westonruter westonruter merged commit d5e0bbc into bugfix/saving-menus-issue_2 Jul 6, 2016
@westonruter westonruter deleted the bugfix/saving-menus-issue_3 branch July 27, 2016 21:14
@westonruter westonruter modified the milestone: 0.5.0 Aug 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants