#32576 closed feature request (fixed)
Add Menu management to the Customizer
Reported by: | celloexpressions | Owned by: | |
---|---|---|---|
Milestone: | 4.3 | Priority: | normal |
Severity: | normal | Version: | 4.3 |
Component: | Customize | Keywords: | |
Focuses: | javascript | Cc: |
Description
Merge the Menu Customizer plugin, per the proposal and tentative approval in the 6/3 dev chat.
Until merge, issues are being tracked on GitHub. Everything on the 1.0 milestone should be fixed before merge. Everything on the "Core Merge" milestone should be fixed in the core merge patch. Unfortunately we've realized that the remaining larger issues require core patches, which @westonrtuer and @adamsilverstein are working on.
In the meantime, here's a tentative merge plan for the code:
class-wp-customize-menus.php
->wp-includes/class-wp-customize-menus.php
customize-menus-preview.js
-> Not sure, @westonruter?customize-menus-preview.css
-> Not sure, @westonruter? Might want to name it as justcustomize-preview.css
to more logically add stuff there in the future, since there's so little here.menu-customize-controls.php
-> split intowp-includes/class-wp-customize-control|section|panel.php
, adding the custom menu objects after all of the existing main objects and subclasses.menu-customizer.css
->wp-admin/css/customize-menus.css
menu-customizer.js
->wp-admin/js/customize-menus.js
menu-customizer.php
-> add menus bootstrap towp-includes/class-wp-customize-manager.php::__construct()
The actual file-moving and patching should wait until the last minute and be merged with the required core patches for settings refactoring, drag & drop, Twenty Fifteen compatibility, and link updating. We'll do a hard freeze on the GitHub repo once we're ready to patch-ify the plugin, likely mid-week this week.
Also cc @valendesigns, @voldemortensen, @designsimply, @folletto, @ocean90 since many of you may not follow this component.
Attachments (10)
Change History (46)
#1
@
10 years ago
The easiest of the four non-plugin-merging core patches required is 32576.link-updates.diff, which handles the admin bar and "Manage in Customize" link on nav-menus.php
. Note that all of the "Appearance" links in the admin bar now deep-link to the Customizer. I suggest condensing those to a single top-level "Customize" link in a future release (probably 4.4), and putting something more useful in this valuable place.
Additional needed patches will be:
- Twenty Fifteen compatibility fix
- Drag & Drop submenus core patch
- Settings refactoring core patch (adds a filter I believe)
- Unit tests, if not included in the main plugin-merging part
- Plugin-merging patch, which does what I described above in terms of merging the files in
Eventually these will all be combined into one massive patch for committing, but I'd like to keep them as distinct as we can until we're ready to prepare that.
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
10 years ago
#3
@
10 years ago
See also https://core.trac.wordpress.org/ticket/28138#comment:3 for something we should fix before/on merge. Could use a new patch that takes exactly what Menu Customizer does, as I believe it changed somewhat.
#8
@
10 years ago
Opened #32631 with patch to ensure that wp_get_nav_menu_items
filter will apply even when the menu has no initial items.
#9
@
10 years ago
Opened #32632 with patch to update wp_setup_nav_menu_item()
so that it will not clobber supplied $menu_item
properties that are empty.
#10
@
10 years ago
Opened #32633 with patch to allow wpNavMenu
to have its sortable items selector to be configured.
@
10 years ago
Additional Core changes needed for Menu Customizer from https://github.com/xwp/wordpress-develop/pull/91/files
#17
follow-up:
↓ 20
@
10 years ago
With one of my themes that has a hidden menu by default, the menu is opened and messes up the entire design with the change (screenshot attached). This seems to happen because of the addition of the following code (added via JS?):
<div id="partial-refresh-menu-container-1" class="partial-refresh-menu-container" data-instance-number="1">
I've already figured out how to correct this on my end, but I imagine there'll be other themes with similar issues. Injecting an additional <div>
has potential to cause some unintended consequences with theme designs.
#18
@
10 years ago
That code is added by WP_Customize_Menus::filter_wp_nav_menu()
under certain conditions when a menu can be partially refreshed. If you want to hack it so it works without partial refresh you could set echo
to false
when you use wp_nav_menu
and then echo the menu manually.
#20
in reply to:
↑ 17
;
follow-up:
↓ 22
@
10 years ago
Replying to greenshady:
With one of my themes that has a hidden menu by default, the menu is opened and messes up the entire design with the change (screenshot attached). This seems to happen because of the addition of the following code (added via JS?):
<div id="partial-refresh-menu-container-1" class="partial-refresh-menu-container" data-instance-number="1">I've already figured out how to correct this on my end, but I imagine there'll be other themes with similar issues. Injecting an additional
<div>
has potential to cause some unintended consequences with theme designs.
Instead of wrapping the menu with a div
when wp_nav_menu
is called, we could maybe add those attributes directly onto the root element that would be output.
#22
in reply to:
↑ 20
@
10 years ago
Replying to westonruter:
Replying to greenshady:
With one of my themes that has a hidden menu by default, the menu is opened and messes up the entire design with the change (screenshot attached). This seems to happen because of the addition of the following code (added via JS?):
<div id="partial-refresh-menu-container-1" class="partial-refresh-menu-container" data-instance-number="1">I've already figured out how to correct this on my end, but I imagine there'll be other themes with similar issues. Injecting an additional
<div>
has potential to cause some unintended consequences with theme designs.
Instead of wrapping the menu with a
div
whenwp_nav_menu
is called, we could maybe add those attributes directly onto the root element that would be output.
I tested by live-editing the HTML on my test install. That idea seems to fix the issue for me.
#24
@
10 years ago
@ocean90 @westonruter 32576.link-updates.diff should be ready for commit now.
Since this is a tracking ticket, let's make new tickets for new issues, and try to keep this ticket focused on any remaining merge-specific things (with commits and new tickets referencing this as well).
#31
@
10 years ago
- Focuses javascript added
- Resolution set to fixed
- Status changed from new to closed
Things are going pretty smoothly here, with tons and tons of community-generated tickets and patches being reviewed and committed since the initial merge. Going to close this ticket out for now as it looks like we've sorted out all of the merge-specific issues or moved them to distinct tickets.
Point admin bar link to menus in the Customizer, add a manage in Customizer link to nav-menus.php.