Customize: edit shortcuts for custom menu widgets do not work

[celloexpressions]

Custom menu widgets contain associated menu partials, which also get shortcuts. This has a few issues:

• There are two shortcut icons, one for the menu and one for the widget.
• The menu shortcut doesn't work. Menu partials look for the associated menu location control, but menus displayed via widgets are not in a menu location.
• Depending on the theme, if the widget does not have a title the menu shortcut will likely be displayed on top of the widget shortcut. Since the menu shortcut doesn't work and the widget shortcut is essentially hidden, the shortcuts appear broken and essentially do nothing (unless you navigate to the widget shortcut with the keyboard).

We should either prevent the menu within a custom menu widget from getting an edit shortcut, or hide that icon with CSS. The widget shortcut is more appropriate in that the widget is the "location" of the menu, and the widget control links to edit the menu similarly to the menu location controls.

[sanket.parmar]

Hi @celloexpressions ,

What if we don't load an edit icon for custom menu widgets. I mean on https://core.trac.wordpress.org/browser/tags/4.7/src/wp-includes/class-wp-customize-nav-menus.php#L1298 line we add one extra condition to check fallback_cb is not empty ( i.e., ! empty( $args->customize_preview_nav_menus_args['fallback_cb'] )). This way the edit icon will not load for custom menu widgets.

Let me know your thoughts.

[sirbrillig]

Prevent adding edit shortcut for nav menus inside widgets

[sirbrillig]

@sanket.parmar That sounds like it would indeed work! But I think it might also prevent custom menu widgets from using partials at all? (I maybe wrong.)

I attached a javascript patch which should prevent nav menus within widgets from getting their own shortcuts. Please let me know if it works for you!

[celloexpressions]

39101.diff​ looks good and works for me. Might want to add an inline comment explaining that it's disabled because the associated widget partial should take precedent, otherwise should be ready for commit.

[sirbrillig]

Add inline comment

[westonruter]

@sirbrillig @celloexpressions I found the underlying problem was that the showControl logic was only accounting for wp_nav_menu() instances that referenced a given theme_location. The Custom Menu widgets never have a theme_location but rather instead refer to a given menu by ID directly. So the fix is just to add support for menu if theme_location is not set. This will also fix the edit shortcuts for any wp_nav_menu() instances that use menu in places other than the Custom Menu widget. See 39101.3.diff​.

This new patch also fixes an issue where showControl would end up focusing on the last control that it found to be associated with a given setting. In this new patch, it will first sort the controls by their priority to then focus on the one that hast the priority with the smallest number, and thus the controls that appear at the top. For nav menus this ensures that the focus is given to the nav menu name control as opposed to the last checkbox control in the nav menu section.

Please confirm and I'll go ahead and commit.

[sirbrillig]

@westonruter nice sleuthing! Confirmed that that fixes the bug, in that it allows the edit shortcut for menus in widgets to work again.

That said (and your patch should be committed regardless), I wonder which is the better experience for users?

Clicking on the (fixed) menu shortcut brings up the editor for that menu.

Clicking on the widget shortcut brings up the widget settings, which allow changing which menu is displayed there.

It seems to me that both could be what a user might be looking for, but obviously we can only allow one. Since you can click the "Edit Menu" button to get from the widget settings to the menu settings and not the other way around, I suggest we hide the menu shortcut anyway, but I'm open to a dissenting opinion.

[westonruter]

In 39622:

Customize: Fix visible edit shortcuts for wp_nav_menu() instances using the menu arg (such as in the Custom Menu widget) instead of the theme_location arg.

Also fix logic for focus-control-for-setting handler to focus on the first control (lowest priority value) associated with a given setting instead of the last control encountered when iterating over all controls, as this ensures the first control in a nav_menu section is focused rather than the last one.

Props westonruter, sirbrillig.
See #27403.
Fixes #39101.

[westonruter]

Re-opening for 4.7.1

@sirbrillig Good questions you're raising. I don't think it particularly hurts that there is two shortcuts for the two separate entities. On that note, one thing I am noticing is that the edit shortcut isn't appearing for the wp_nav_menu() instance when first adding the Custom Widget to the page. Likewise, the edit shortcut goes away for the wp_nav_menu() after making a change to the Custom Menu widget. So there seems to be an issue with nested partials and edit shortcuts.

[sirbrillig]

Created #39352 for the multiple-shortcuts question and #39353 for the missing shortcut bug.

[dd32]

In 39653:

Customize: Fix visible edit shortcuts for wp_nav_menu() instances using the menu arg (such as in the Custom Menu widget) instead of the theme_location arg.

Also fix logic for focus-control-for-setting handler to focus on the first control (lowest priority value) associated with a given setting instead of the last control encountered when iterating over all controls, as this ensures the first control in a nav_menu section is focused rather than the last one.

Props westonruter, sirbrillig.
See #27403.
Merges [39622] to the 4.7 branch.
Fixes #39101.

[westonruter]

In 40055:

Customize: Ensure edit shortcuts get re-created for nested partials when a parent partial is refreshed.

Fixes issue where the edit shortcut for a nav menu gets dropped when the containing Custom Menu widget is updated.

See #39101.
Fixes #39353.

[dd32]

In 40097:

Customize: Ensure edit shortcuts get re-created for nested partials when a parent partial is refreshed.

Fixes issue where the edit shortcut for a nav menu gets dropped when the containing Custom Menu widget is updated.

Props westonruter.
Merges [40055] to the 4.7 branch.
See #39101.
Fixes #39353.