WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 5 months ago

#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 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 2 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 2 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 2 years ago.
Merging class-wp-*
32576.2.diff (195.2 KB) - added by ocean90 2 years ago.
Merge styles/scripts
32576.3.diff (194.0 KB) - added by ocean90 2 years ago.
Fix JS/CSS
32576.4.diff (195.8 KB) - added by obenland 2 years ago.
32576.5.diff (249.6 KB) - added by ocean90 2 years ago.
Tests
32576.6.diff (251.1 KB) - added by ocean90 2 years ago.
menu-customizer-saga.png (298.3 KB) - added by greenshady 2 years ago.
32576.7.diff (1.6 KB) - added by DrewAPicture 2 years ago.
Rename WP_New_Menu_Customize_Control

Download all attachments as: .zip

Change History (46)

@celloexpressions
2 years ago

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

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


2 years ago

#3 @celloexpressions
2 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
2 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
2 years ago

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

#6 @westonruter
2 years ago

Opened #32629 for introduction of pre_get_term filter.

#7 @westonruter
2 years ago

Opened #32630 with patch for has_nav_menu filter.

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

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

@westonruter
2 years ago

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

#11 @azaozz
2 years ago

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

#12 @ocean90
2 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
2 years ago

Merging class-wp-*

@ocean90
2 years ago

Merge styles/scripts

@ocean90
2 years ago

Fix JS/CSS

@obenland
2 years ago

@ocean90
2 years ago

Tests

@ocean90
2 years ago

#13 @ocean90
2 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
2 years ago

Awesome! :)

#15 @ocean90
2 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
2 years ago

  • Component changed from Menus to Customize

#17 follow-up: @greenshady
2 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
2 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
2 years ago

In 32808:

Fix a typo in [32806].

see #32576.

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

That would probably be better.

#22 in reply to: ↑ 20 @greenshady
2 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
2 years ago

In 32815:

JSHint after [32806]

See #32576.

#24 @celloexpressions
2 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
2 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
2 years ago

In 32887:

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

see #32576.

#27 @ocean90
2 years ago

In 32888:

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

see #32576.

#28 @ocean90
2 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
2 years ago

In 32893:

Customizer: Escape original title of menu items.

see #32576.

#30 @ocean90
2 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
2 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
2 years ago

In 33163:

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

see #32576.

#33 @ocean90
2 years ago

In 33164:

Customizer: Merge two strings.

see #32576.

#34 @ocean90
2 years ago

In 33167:

Customizer: Remove unused string.

see #32576.

@DrewAPicture
2 years ago

Rename WP_New_Menu_Customize_Control

#35 @SergeyBiryukov
2 years ago

In 33859:

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

see #32576.

#36 @westonruter
5 months 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.