WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 4 weeks ago

Last modified 4 weeks ago

#44286 closed enhancement (fixed)

Feature Request: Provide Menu Settings on create new menu

Reported by: garrett-eclipse Owned by: audrasjb
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Menus Keywords: has-screenshots has-patch commit
Focuses: administration Cc:

Description

Hello,

I wanted to request an improvement to the Menu creation workflow to reduce the workflow. Currently you create a menu with name and then once created can set the Menu Settings for 'Auto add pages' and 'Display location'. It would be nice to have those options available on the create menu screen thus removing a step in the process if you're not adding any menu items.

Thanks

Attachments (7)

Screen Shot 2018-05-31 at 4.33.29 PM.png (23.3 KB) - added by garrett-eclipse 2 years ago.
Create Menu
Screen Shot 2018-05-31 at 4.33.16 PM.png (57.9 KB) - added by garrett-eclipse 2 years ago.
Menu Settings
44286.patch (2.3 KB) - added by ianjvr 17 months ago.
Screen Shot 2019-02-27 at 9.57.14 PM.png (106.0 KB) - added by garrett-eclipse 17 months ago.
After a failed menu creation if any menu locations were selected they become default.
44286.2.diff (4.2 KB) - added by garrett-eclipse 3 months ago.
Refreshed patch and addressed setting 0 to nav_menu_locations theme mod which caused a default location to be checked when creating a menu.
44286.3.diff (4.1 KB) - added by audrasjb 2 months ago.
patch refreshed against trunk
Capture d’écran 2020-05-11 à 22.51.38.png (100.6 KB) - added by audrasjb 2 months ago.
Patch looks good

Download all attachments as: .zip

Change History (19)

#1 @pratikthink
2 years ago

  • Keywords 2nd-opinion has-screenshots added

#2 @pento
18 months ago

  • Version trunk deleted

#3 @audrasjb
18 months ago

  • Keywords needs-patch good-first-bug added; 2nd-opinion removed

Hi,

Looks like a nice little enhancement. Adding good-first-bug keyword.

@ianjvr
17 months ago

#4 @ianjvr
17 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

#5 @audrasjb
17 months ago

  • Owner set to audrasjb
  • Status changed from new to accepted

@garrett-eclipse
17 months ago

After a failed menu creation if any menu locations were selected they become default.

#6 @garrett-eclipse
17 months ago

  • Keywords needs-refresh added; good-first-bug has-patch needs-testing removed

Thanks for the patch @ianjvr

I successfully applied against trunk and testing everything seemed to be working until I tried to break it... and by that I mean I tried to save a second menu with the same title as an existing one. In this one circumstance, an issue presents itself in that the failed save updated the database option for theme_mods_twentynineteen to set the nav_menu_locations I'd had selected 'social' against an id of 0;
a:2:{s:18:"custom_css_post_id";i:-1;s:18:"nav_menu_locations";a:2:{s:6:"menu-1";i:4;s:6:"social";i:0;}}
With this set against id 0 when you go to create an additional menu we are presented with pre-selected menu location(s).
https://core.trac.wordpress.org/raw-attachment/ticket/44286/Screen%20Shot%202019-02-27%20at%209.57.14%20PM.png

I believe the issue here lies in this code which occurs before the menu title is checked for error.
https://github.com/WordPress/WordPress/blob/master/wp-admin/nav-menus.php#L295-L309

The other tests I did worked nicely they entailed;

  1. Creating a menu.
  2. Creating a menu w/ auto-add.
  3. Creating a menu w/ location(s)
  4. Creating a menu w/ both auto-add and location(s)

*Between these tests I reloaded and attempted the location tests with different ones individually and multiples as well.

The only other pause I had while testing was after creating my first menu and going to create my second I wanted to go back but in this case where there's only one existing menu the dropdown select doesn't appear so one either needed to reload or use the sidebar navigation. Due to this I opened (#46367) to suggest adding a Cancel to the Create Menu screen.

Nice work on the patch, and let me know if I can clarify or assist in any way.

Cheers

Last edited 17 months ago by garrett-eclipse (previous) (diff)

@garrett-eclipse
3 months ago

Refreshed patch and addressed setting 0 to nav_menu_locations theme mod which caused a default location to be checked when creating a menu.

#7 @garrett-eclipse
3 months ago

  • Keywords has-patch needs-testing added; needs-refresh removed
  • Milestone changed from Awaiting Review to 5.5

In 44286.2.diff I've refreshed to patch to apply cleanly as well as address the issue I raised during testing. This was done by moving a set_theme_mod of the cleaned up menu_locations before to only apply for existing menus.

This feels like a good candidate for 5.5, @audrasjb could you take a moment to review/test.
Thank you

#8 @audrasjb
3 months ago

It's on my list, thanks for the heads up @garrett-eclipse

@audrasjb
2 months ago

patch refreshed against trunk

#9 @audrasjb
2 months ago

  • Keywords needs-testing removed

Hi,

I tested the previous patch and it looks good to me. Works fine, thanks 👍

44286.3.diff is a small refresh against trunk.

#10 @garrett-eclipse
2 months ago

  • Keywords commit added

Thanks for testing @audrasjb let's get a committer review here and I think that's a wrap.

#11 @whyisjake
4 weeks ago

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

This was fixed in [48051], so marking as closed. The committer failed to add the relevant line to auto-close the issue.

#12 @whyisjake
4 weeks ago

In 48052:

Code Standards: Cleanup some code spacing.

See: #37826 and #44286.

Note: See TracTickets for help on using tickets.