WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#27878 closed defect (bug) (fixed)

Custom Menu widget can't be previewed without changing a field

Reported by: kucrut Owned by: markjaquith
Milestone: 3.9.1 Priority: normal
Severity: normal Version: 3.9
Component: Widgets Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

Steps to reproduce

  1. Go to Appearance > Themes
  2. Click on the Customize button of the Twenty Fourteen theme
  3. Expand any widget area (let's use "Widgets: Primary Sidebar")
  4. Click on Add Widget button
  5. Select "Custom Menu" widget
  6. The widget is not added to the widget area

I believe adding "--Select--" as the first option on the Menus dropdown should solve this, because the user will be 'forced' to make an interaction to the widget form.

Attachments (2)

27878.patch (816 bytes) - added by westonruter 6 years ago.
https://github.com/x-team/wordpress-develop/commit/af253983e00ae602fc952c25ffda6ebde7012a4c
27878.diff (849 bytes) - added by markjaquith 6 years ago.

Download all attachments as: .zip

Change History (17)

#1 @westonruter
6 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 3.9.1

Yes. This is a side-effect of eliminating the Submit/Update button, and not being able to auto-submit a widget's form when it is first added, because it could result in the widget form showing an error message. So either the Custom Menu widget needs to update its rendering method to use the same default menu as is pre-selected when the form is displayed, or the menu select in the form needs to start out with an empty value (as kucrut suggests, which is probably the best approach).

#2 @westonruter
6 years ago

  • Keywords has-patch added; needs-patch removed

27878.patch takes kucrut's suggested approach.

#3 @seanchayes
6 years ago

I tested this, using the steps provided, and found the following:

Pre-patch, when adding the Custom menu widget the menu does not appear in the Sidebar and, if you only have one Custom Menu, it will only appear if you add a title to it. If you have more than one menu available and you want the first one listed then you have to switch to another menu and then switch back to the menu you actually want.

Post-patch, indeed the addition of the --Select-- option does force the user to select the menu and that triggers the action / process to refresh the preview of the theme. If, you add a title but still leave the menu option as --Select-- no menu is added (it doesn't know which one of course).

Forcing the choice seems to work logically and practically in this circumstance.

This ticket was mentioned in IRC in #wordpress-dev by nacin1. View the logs.


6 years ago

#5 @nacin
6 years ago

  • Owner set to markjaquith
  • Status changed from new to assigned

#6 @markjaquith
6 years ago

  • Keywords needs-patch added; has-patch removed

--Select-- isn't translated, but -- Select -- is, so we should use that.

#7 @markjaquith
6 years ago

It also should have a value of 0, to match nav-menus.php.

src/wp-admin/nav-menus.php:623: <option value="0" selected="selected"><?php _e( '-- Select --' ); ?></option>

Repatching.

@markjaquith
6 years ago

#8 follow-up: @markjaquith
6 years ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

I like 27878.diff. Good?

#9 in reply to: ↑ 8 @kucrut
6 years ago

Replying to markjaquith:

I like 27878.diff. Good?

+1

#10 @markjaquith
6 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 28197:

Force users to choose a nav menu in the custom nav menu widget, for a better customizer UX

Before, they had to make a dummy change to get it to render. Now they
are made to choose a nav menu from the dropdown, which feels more
natural.

fixes #27878 for trunk. props westonruter

#11 @markjaquith
6 years ago

In 28198:

Force users to choose a nav menu in the custom nav menu widget, for a better customizer UX

Before, they had to make a dummy change to get it to render. Now they
are made to choose a nav menu from the dropdown, which feels more
natural.

Merges [28197] to the 3.9 branch.

fixes #27878. props westonruter

#12 follow-up: @nacin
6 years ago

Existing string: "&mdash; Select &mdash;"

What's the esc_html() for?

#13 in reply to: ↑ 12 @westonruter
6 years ago

Replying to nacin:

What's the esc_html() for?

In esc_html( $menu->name )? Just for good measure, to prevent any possibility of the value introducing malformed markup.

#14 @markjaquith
6 years ago

In 28205:

Use '&mdash; Select &mdash;' instead of '-- Select --' for nav menus. Looks nicer.

see #27878

#15 @markjaquith
6 years ago

In 28206:

Use '&mdash; Select &mdash;' instead of '-- Select --' for nav menus widget. Looks nicer.

Partially merges [28205] to the 3.9 branch.

see #27878

Note: See TracTickets for help on using tickets.