WordPress.org

Make WordPress Core

Opened 9 years ago

Last modified 4 months ago

#13998 reviewing enhancement

Inconsistency in arguments when wp_nav_menu falls back to wp_page_menu

Reported by: Utkarsh Owned by: chriscct7
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Menus Keywords: has-patch
Focuses: Cc:
PR Number:

Description

wp_nav_menu's 'menu_class' parameter applies the class to the <ul>.

The default fallback function wp_page_menu however, applies 'menu_class' to the <div> enclosing the <ul>.

This can cause inconsistent styles if the style is applied to ul.<menu class>

Attachments (4)

13998_wp_page_menu_incosistence.patch (1.3 KB) - added by Sanjo 9 years ago.
wp_page_menu() has now a container_class and a menu_class argument that are identical to wp_nav_menu.
13998_wp_page_menu_incosistence_2.patch (2.0 KB) - added by Sanjo 9 years ago.
Added documentation
13998.patch (5.3 KB) - added by Frank Klein 6 years ago.
13998.2.patch (647 bytes) - added by Frank Klein 4 years ago.

Download all attachments as: .zip

Change History (15)

#1 @nacin
9 years ago

Related #13979

#2 @nacin
9 years ago

  • Milestone changed from Awaiting Review to 3.1

#3 @nacin
9 years ago

  • Keywords needs-patch added

Needs patch or punt.

@Sanjo
9 years ago

wp_page_menu() has now a container_class and a menu_class argument that are identical to wp_nav_menu.

#4 @Sanjo
9 years ago

  • Keywords has-patch added; needs-patch removed

@Sanjo
9 years ago

Added documentation

#5 @nacin
9 years ago

  • Keywords needs-patch added; dev-feedback has-patch removed

We can't change how wp_page_menu() works. We'll need to add a ul_class argument to wp_page_menu and re-map menu_class to ul_class in wp_nav_menu before passing it to wp_page_menu.

#6 @nacin
9 years ago

  • Milestone changed from 3.1 to Future Release
  • Type changed from defect (bug) to enhancement

@Frank Klein
6 years ago

#7 @Frank Klein
6 years ago

  • Cc contact@… added
  • Keywords has-patch added; needs-patch removed

The updated patch with contains the following elements:

  • Add a ul_class argument to wp_page_menu.
  • Remap the menu_class from wp_nav_menu to ul_class before passing it to wp_page_menu.
  • Updated inline documentation.
  • Some whitespace fixes and other code standards adaptations.

I think that there are other possible enhancements that can be added to the wp_page_menu function, specifically some of the arguments used by wp_nav_menu such as:

  • container
  • container_id
  • menu_id
  • before
  • after
  • items_wrap
  • depth

As far as I see, it should be possible to implement these arguments in wp_page_menu as well without breaking existing themes. I think especially the depth argument might be a worthwhile addition, what do you think?

#8 @yarco
5 years ago

I get the problem in v3.9... menu_class will be always assigned to the container...
So what is the current resolution?

#9 @chriscct7
4 years ago

  • Keywords needs-refresh added

@Frank Klein
4 years ago

#10 @Frank Klein
4 years ago

  • Keywords needs-refresh removed

The refreshed patch only deals with adding the menu_class to the <ul> in case of a fallback.

I think this would make sense to include in 4.4, since it's related to #11095.

#11 @chriscct7
4 years ago

  • Owner set to chriscct7
  • Status changed from new to reviewing

4.4's enhancement window is closed. It can however be submitted for 4.5

Note: See TracTickets for help on using tickets.