Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#32576 closed feature request (fixed)

Add Menu management to the Customizer

Reported by: celloexpressions's profile 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 just customize-preview.css to more logically add stuff there in the future, since there's so little here.
  • menu-customize-controls.php -> split into wp-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 to wp-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)

32576.link-updates.diff (2.3 KB) - added by celloexpressions 10 years ago.
Point admin bar link to menus in the Customizer, add a manage in Customizer link to nav-menus.php.
32576.menu-customizer-core-misc.diff (3.2 KB) - added by westonruter 10 years ago.
Additional Core changes needed for Menu Customizer from https://github.com/xwp/wordpress-develop/pull/91/files
32576.diff (81.4 KB) - added by obenland 10 years ago.
Merging class-wp-*
32576.2.diff (195.2 KB) - added by ocean90 10 years ago.
Merge styles/scripts
32576.3.diff (194.0 KB) - added by ocean90 10 years ago.
Fix JS/CSS
32576.4.diff (195.8 KB) - added by obenland 10 years ago.
32576.5.diff (249.6 KB) - added by ocean90 10 years ago.
Tests
32576.6.diff (251.1 KB) - added by ocean90 10 years ago.
menu-customizer-saga.png (298.3 KB) - added by greenshady 10 years ago.
32576.7.diff (1.6 KB) - added by DrewAPicture 9 years ago.
Rename WP_New_Menu_Customize_Control

Download all attachments as: .zip

Change History (46)

@celloexpressions
10 years ago

Point admin bar link to menus in the Customizer, add a manage in Customizer link to nav-menus.php.

#1 @celloexpressions
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 @celloexpressions
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.

#4 @westonruter
10 years ago

In 32744:

Customizer: Allow sections and panels to be exported to JS.

Also fix param docs for customize_dynamic_setting_class filter, and use require_once for class-wp-customize-manager.php in bootstrap function _wp_customize_include().

See #30737, #32576.

#5 @westonruter
10 years ago

Opened #32628 for the change needed to wp.ajax.post.

#6 @westonruter
10 years ago

Opened #32629 for introduction of pre_get_term filter.

#7 @westonruter
10 years ago

Opened #32630 with patch for has_nav_menu filter.

#8 @westonruter
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 @westonruter
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 @westonruter
10 years ago

Opened #32633 with patch to allow wpNavMenu to have its sortable items selector to be configured.

@westonruter
10 years ago

Additional Core changes needed for Menu Customizer from https://github.com/xwp/wordpress-develop/pull/91/files

#11 @azaozz
10 years ago

32576.menu-customizer-core-misc.diff looks good.

#12 @ocean90
10 years ago

In 32763:

Nav menus: Reset the fallback_cb default argument in wp_nav_menu in case of a Customizer preview.

props westonruter.
see #32576.

@obenland
10 years ago

Merging class-wp-*

@ocean90
10 years ago

Merge styles/scripts

@ocean90
10 years ago

Fix JS/CSS

@obenland
10 years ago

@ocean90
10 years ago

Tests

@ocean90
10 years ago

#13 @ocean90
10 years ago

In 32806:

Add menu management to the Customizer.

This brings in the Menu Customizer plugin: https://wordpress.org/plugins/menu-customizer/.

props celloexpressions, westonruter, valendesigns, voldemortensen, ocean90, adamsilverstein, kucrut, jorbin, designsimply, afercia, davidakennedy, obenland.
see #32576.

#14 @folletto
10 years ago

Awesome! :)

#15 @ocean90
10 years ago

In 32807:

Twenty Fifteen: Wrap navigation helpers into a function so it can be called on a refresh event of the Customize Preview.

props westonruter.
see #32576.

#16 @ocean90
10 years ago

  • Component changed from Menus to Customize

#17 follow-up: @greenshady
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 @valendesigns
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.

#19 @ocean90
10 years ago

In 32808:

Fix a typo in [32806].

see #32576.

#20 in reply to: ↑ 17 ; follow-up: @westonruter
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.

#21 @valendesigns
10 years ago

That would probably be better.

#22 in reply to: ↑ 20 @greenshady
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 when wp_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.

#23 @iseulde
10 years ago

In 32815:

JSHint after [32806]

See #32576.

#24 @celloexpressions
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).

#25 @obenland
10 years ago

In 32855:

Keep WP_Customize_Nav_Menus_Panel with other panels rather than controls.

Accidentally merged into the wrong file in [32806].
H/t celloexpressions for noticing.

See https://wordpress.slack.com/archives/core-customize/p1434696254000258.
See #32576.

#26 @ocean90
10 years ago

In 32887:

Customizer: Replace wrong usage of _x() with translator comments.

see #32576.

#27 @ocean90
10 years ago

In 32888:

Customizer: Make labels of the menu item reorder buttons translatable.

see #32576.

#28 @ocean90
10 years ago

In 32892:

Customizer: Improve handling of posts with no title.

Use the #%d (no title) string as a fallback which is already used on the nav menus screen.
Add a translator comment to all occurrences of the #%d (no title) string.

see #32576.

#29 @ocean90
10 years ago

In 32893:

Customizer: Escape original title of menu items.

see #32576.

#30 @ocean90
10 years ago

In 32895:

Customizer: Fix live preview for menu item titles.

Show also a default label for menu items without a label which are assigned to a menu. This is currently only supported in the Customizer, see #24146 for nav menus screen.

see #32576.

#31 @celloexpressions
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.

#32 @ocean90
10 years ago

In 33163:

Customizer: Replace non-visible error messages for nav menus with error codes.

see #32576.

#33 @ocean90
10 years ago

In 33164:

Customizer: Merge two strings.

see #32576.

#34 @ocean90
10 years ago

In 33167:

Customizer: Remove unused string.

see #32576.

@DrewAPicture
9 years ago

Rename WP_New_Menu_Customize_Control

#35 @SergeyBiryukov
9 years ago

In 33859:

Set svn:eol-style for the files added in [32806].

see #32576.

#36 @westonruter
8 years ago

In 41001:

Customize: Fix logic to re-expand a newly inserted nav menu section after placeholder section is replaced.

Fixes todo from Menu Customizer feature plugin: https://github.com/voldemortensen/menu-customizer/commit/0f4ea4e#diff-daa55fade2253f26ccbe02f71058841cR2330
Amends [32806].
Props greuben.
See #32576.
Fixes #40997.

Note: See TracTickets for help on using tickets.