WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 2 months ago

#18842 reopened defect (bug)

wp_nav_menu confuses new developers when it falls through to page listing

Reported by: dd32 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.2
Component: Menus Keywords: needs-patch
Focuses: Cc:

Description

It appears that when wp_nav_menu() falls through to a page listing, many menu-specific args are not passed to the page listing, which ultimately confuses new developers.

I seem to answer this at least weekly in #wordpress

One example is the 'container_class' arg, if it falls through to the fallback_cb, the container_class is not applied.

Ideally, template-related arguements should be passed to the fallback (And with pages as the default callback, it should handle these) or wp_nav_menu() should output any extra wrapping divs if appropriate.

Change History (8)

#2 @coffee2code
6 years ago

  • Keywords needs-patch added
  • Version set to 3.2.1

#3 @alex-ye
5 years ago

  • Cc nashwan.doaqan@… added

#5 @chriscct7
2 years ago

  • Version changed from 3.2.1 to 3.2

#6 @joyously
2 months ago

  • Resolution set to invalid
  • Status changed from new to closed

I was attempting to fix this for a theme, by using a filter on 'page_menu_args', but wp_page_menu not only ignores the args, but has different meanings for some and sets others itself.
So this code almost gets it, but not handling the case where the menu_id is empty.

function my_page_menu_args( $args )  {
  list( $before, $after ) = explode('%3$s', $args['items_wrap'] );
  $wanted = array(
    'menu_id'    => $args['container_id'],
    'menu_class' => $args['container_class'],
    'before'     => sprintf( $before, esc_attr( $args['menu_id'] ), esc_attr( $args['menu_class'] ) ),
    'after'      => $after,
  );
  unset( $args['fallback_cb'] );  // to bypass wp_page_menu setting args for us
  return wp_parse_args( $wanted, $args );
}

One of the problems is that wp_page_menu checks to see if it's a fallback, and then sets the args a certain way. But it always returns a container, even though wp_nav_menu checks if container is empty .

The sequence in the code at this time is:

wp_nav_menu sets default args
wp_nav_menu combines defaults with passed args
wp_nav_menu filters the args
if no menu, return fallback passing filtered args

  wp_page_menu sets default args
  wp_page_menu combines defaults with passed args
  wp_page_menu filters the args
  wp_page_menu sets the 'container' to a div if it's empty
  wp_page_menu sets 'before' and 'after' to plain `ul` if it's fallback
  wp_page_menu wraps the menu with 'before' and 'after'
  wp_page_menu wraps the menu with 'container' using 'menu_id' and 'menu_class'
  wp_page_menu filters the entire menu before returning it

// else the menu exists
wp_nav_menu uses 'container_id' and 'container_class' for 'container'
wp_nav_menu adjusts menu for child classes
wp_nav_menu filters the menu objects and resulting menu several ways
wp_nav_menu wraps the menu with 'items_wrap' using built 'menu_id' and 'menu_class'
wp_nav_menu wraps the menu with built 'container'
wp_nav_menu filters the entire menu before returning it

What I would suggest is a small change to wp_page_menu to handle the args like wp_nav_menu when it checks for fallback.
@SergeyBiryukov, can you take a look at this?

Other tickets that are all pretty much the same thing:
#16679: wp_nav_menu treats menu_class differently for fallback
#14614: Can't remove container in wp_nav_menu
#18232: wp_nav_menu - Setting walker parameter affects fallback_cb

#7 @joyously
2 months ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

This ticket was mentioned in Slack in #core by joyously. View the logs.


2 months ago

Note: See TracTickets for help on using tickets.