Make WordPress Core

Opened 14 years ago

Closed 14 years ago

#13378 closed defect (bug) (fixed)

Associate nav menus with particular menu locations in a theme

Reported by: ryan's profile ryan Owned by: nacin's profile nacin
Milestone: 3.0 Priority: normal
Severity: blocker Version:
Component: Menus Keywords: ux-feedback
Focuses: Cc:

Description (last modified by ryan)

There is currently no way to associate a particular menu with a given call to wp_nav_menu() in a theme. Themes cannot call wp_nav_menu() multiple times and have sane assignment of menus without hard coding particular menu names/ids/slugs (which is not portable across themes and languages).

This can be solved by allowing themes to register how many menus they use and give those menus a handle. A dropdown in the menus admin would show these handles and allow associating a menu with a given handle. Upon switching themes this association would be cleared, although a set of standard slot names could be used and preserved when switching themes (Main, Secondary, ...). Registering "slots" decouples assignment from menu naming, allowing users to name their menus as they see fit in any language instead of being tied to hard-coded strings coming from the theme.

Attachments (12)

13378.diff (1.7 KB) - added by ryan 14 years ago.
Beginning of register_nav_menus()
13378.2.diff (3.7 KB) - added by ryan 14 years ago.
13378.3.diff (3.7 KB) - added by ryan 14 years ago.
nav menu locations.png (48.4 KB) - added by TECannon 14 years ago.
nav-menu-locations mockup
13378.truncate.long.names.1.patch (793 bytes) - added by koopersmith 14 years ago.
14720.patch (2.9 KB) - added by jorbin 14 years ago.
inline docs for some functions added/changed
13378.nonce.diff (2.0 KB) - added by koopersmith 14 years ago.
13378.4.diff (688 bytes) - added by ryan 14 years ago.
Explanatory text for Theme Locations box.
13378.add.menu.save.diff (3.5 KB) - added by koopersmith 14 years ago.
13378.location.wording.diff (848 bytes) - added by johnonolan 14 years ago.
Revised wording for selecting a location or locations based on my comment above.
13378.twentyten.menulocation.label.diff (515 bytes) - added by johnonolan 14 years ago.
Revised menu location label for TwentyTen.
fix.underlines.diff (829 bytes) - added by koopersmith 14 years ago.

Download all attachments as: .zip

Change History (52)

#1 @ryan
14 years ago

  • Description modified (diff)

@ryan
14 years ago

Beginning of register_nav_menus()

#2 @ryan
14 years ago

Themes do this in functions.php:

register_nav_menus( array('main' => __('Main Menu'), 'footer' => __('Footer Menu') ) );

And this in templates:

wp_nav_menu( array('slot' => 'main') );
wp_nav_menu( array('slot' => 'footer') );

In the backend, an array is set in options that is keyed by slot with the values being menu ids.

update_option( 'nav_menu_slots', array( 'main' => 1234, 'secondary' => 5678 ) );

@ryan
14 years ago

#3 @ryan
14 years ago

During dev meeting it was decided to store the slots per theme.

@ryan
14 years ago

#4 @wpmuguru
14 years ago

(In [14611]) add menu slots/theme_menus, props ryan, see #13378

#5 @ryan
14 years ago

Todo:

  • Actually save the slot mapping
  • Allow a slot mapping of none in the dropdown
  • Make sure two (or more) menus don't map to the same slot
  • Update stale mappings when switching back to a theme
  • Assign default slot mappings when switching to a theme for the first time
  • Find a better term than slot
  • UX/UI review, cause this is confusing and needs love

@TECannon
14 years ago

nav-menu-locations mockup

#7 @nacin
14 years ago

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

In progress.

#8 @nacin
14 years ago

(In [14620]) First pass on 'Theme Locations' for navigation menus. Themes need to specify a location when calling wp_nav_menu and register locations in functions.php. Users then map menus to locations in the nav menu admin. Subject to review. see #13378.

#9 @nacin
14 years ago

I wanted to get the basic structure committed first so others can see what we are confusing each other about.

This still needs some love.

UI - I imagine we will want to visually separate this meta box from the others. Maybe even lock it to the top (and allow it to be collapsible, but kick it out of screen options), and shade it like a widget box (see mock from Tracy above).

JS - Currently it works non-JS. So now we just need to add progressive enhancements. I am thinking about this in two stages. One, the "Save" button needs to save them via AJAX. In fact, the button (when JS) should probably be disabled unless the meta box is dirty, simply because it's an asynchronous setting any way you look at it.

Two, the "Save Menu" button needs to copy over these fields and post them with the form, otherwise they will not be saved if the "Save" button was not pressed. That ensures everything being seamless.

UX - Finally, we need some UX feedback here. But I think we should bite our tongues a little until the UI and JS stuff gets farther along.

Feel free to run with this farther without me.

#10 @nacin
14 years ago

(In [14621]) Only show Theme Locations meta box if menus exist and if the theme has registered menus. see #13378.

#11 @nacin
14 years ago

(In [14623]) If theme has registered menus, set the first menu created to the first registered location by default. see #13378.

#12 @demetris
14 years ago

  • Cc dkikizas@… added

#13 @ryan
14 years ago

  • Summary changed from Associate nav menus with particular menu "slots" in a theme to Associate nav menus with particular menu locations in a theme

#14 @WraithKenny
14 years ago

  • Cc Ken@… added

#15 @nacin
14 years ago

(In [14671]) Truncate long menu names in the menu locations box. props koopersmith, see #13378.

#16 @ryan
14 years ago

  • Keywords ux-feedback added

#17 follow-up: @nacin
14 years ago

(In [14715]) Save menu locations meta box via ajax. see #13378.

@jorbin
14 years ago

inline docs for some functions added/changed

#18 @jorbin
14 years ago

  • Cc aaron@… added

Some of the functions added / modified lack proper documentation. The above patch fixes that.

#19 @nacin
14 years ago

(In [14721]) Inline docs for menu location functions. props jorbin, see #13378.

#20 in reply to: ↑ 17 @TobiasBg
14 years ago

Replying to nacin:

(In [14715]) Save menu locations meta box via ajax. see #13378.

No nonce check or check_ajax_referer()? Is it not necessary here?

#21 @ryan
14 years ago

There should be a nonce. We need a nonce audit in admin-ajax. I see another place that needs it.

#22 @nacin
14 years ago

Yeah, we need one there.

#24 @ocean90
14 years ago

The saving doesn't work. We send via AJAX

menu-locations	menu-locations%5Bprimary%5D=

and get_nav_menu_locations() returns a string

string(30) "menu-locations%5Bprimary%5D=81"


But it should be an array I think: $menu_locations[$location]

@ryan
14 years ago

Explanatory text for Theme Locations box.

#26 @jorbin
14 years ago

The commit message didn't go through. [14772]

#27 @johnonolan
14 years ago

UX: This looks good so far

#29 @ryan
14 years ago

The patch on that thread would have the affect of using the callback when a theme location is not assigned to a menu even if menus exist and there is only one theme location. Since we automatically assign a location to a menu when the menu is created, the location should only be unassigned due to explicit action by the user. This seems okay and provides a means for the user to explicitly ask for the theme's fallback to be run without having to delete menus.

#30 @ryan
14 years ago

(In [14793]) Don't look for a fallback menu when a specific location is requested. Props tinkerpriest. see #13378

#31 @ryan
14 years ago

What's left?

#32 @ryan
14 years ago

I think this is looking pretty good. If you guys are satisfied, resolve as fixed.

#33 @johnonolan
14 years ago

Couple of things:

UX:
The wording is currently "Your theme supports [1] [menu/menus]. Assign a menu to your theme." - Would it be better as "Your theme supports [1] [menu/menus]. Select where you would like [it/them] to appear." ?

The label above the dropdown is "Primary Menu" - I'm thinking that this label should describe the location rather than the menu. Suggested alternative: "Primary Navigation" or "Main Navigation".

UI:
Could we make the dropdown the same height as the text fields in the custom links metabox? I think it's a couple of pixels out at the moment

#34 @nacin
14 years ago

Aside from UI/UX, there is one bit of functionality remaining. When "Save Menu" is pressed, it also needs to save the theme locations. Obviously, this would only be if JS. Anyone is feel free to pick up this.

@johnonolan
14 years ago

Revised wording for selecting a location or locations based on my comment above.

@johnonolan
14 years ago

Revised menu location label for TwentyTen.

#35 @nacin
14 years ago

(In [14831]) Menu tweaks. Save menu locations when saving the menu. Also, centralize theme support checks and add them to menu.php. Improve some branching. Remove some old JS vars, add a missing semicolon, etc. props koopersmith. see #13378.

#36 @nacin
14 years ago

(In [14832]) Some menu string improvements. props JohnONolan. see #13378.

#37 @nacin
14 years ago

(In [14834]) Styling for menu locations howto text. see #13378.

#38 @automattor
14 years ago

(In [14846]) Better text for locations box. Props filosofo. fixes #13520 see #13378

#39 @ryan
14 years ago

(In [14848]) Fix styling of underlines for menus admin. Props koopersmith. see #13378

#40 @nacin
14 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed
Note: See TracTickets for help on using tickets.