Make WordPress Core

Opened 12 years ago

Last modified 5 years ago

#21070 reopened enhancement

Added a filter to the sub-menu class attribute

Reported by: bjornjohansen's profile bjornjohansen Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.4
Component: Menus Keywords: has-patch dev-feedback
Focuses: administration Cc:

Description

The sub-menus are hard coded with "sub-menu" as the only CSS class. This may cause bad performing CSS with maintainability issues, like when you're targeting the third level sub menu.

I've added a filter, so CSS classes like "sub-menu-level-$depth" can be added.

Attachments (2)

nav-menu-template.diff (596 bytes) - added by bjornjohansen 12 years ago.
Patch
functions.php (223 bytes) - added by bjornjohansen 12 years ago.
Example usage of the filter

Download all attachments as: .zip

Change History (10)

@bjornjohansen
12 years ago

Example usage of the filter

#1 @SergeyBiryukov
12 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

#2 @bjornjohansen
12 years ago

I'm not happy with the conclusions of the two other tickets.

Regarding 19297:
As the $depth variable isn't passed on to the filter nav_menu_css_class and the ul element class is hard coded, there actually isn't a very easy way to do this on your own. For the nav_menu_css_class filter, your function actually have to make a lot of queries up the menu tree for each menu item to find the depth. For the ul element, you have to create your own walker, completely replacing Walker_Nav_Menu::start_lvl().

Regarding 19768:
The proposed solution here causes CSS code which is both very ineffcient for browsers to render (https://developers.google.com/speed/docs/best-practices/rendering#UseEfficientCSSSelectors), and makes hard-to-maintain CSS code.

If you take a look at the Walker_Nav_Menu class, there are actually filter calls all over the place, which gives a lot of flexibility without the need for custom walker classes. The exception to this, is the start_lvl method, which actually doesn't have any filters applied at all.

The patch I provided does not change the default output from WordPress, but simply gives you a filter to optionally change it. Just like everything else in WordPress.

#3 @SergeyBiryukov
12 years ago

  • Milestone set to Awaiting Review
  • Resolution duplicate deleted
  • Status changed from closed to reopened

#4 @SergeyBiryukov
12 years ago

  • Keywords 2nd-opinion added

#5 follow-up: @alexvorn2
12 years ago

need the second opinion? close this! :)

#6 in reply to: ↑ 5 @bjornjohansen
12 years ago

Replying to alexvorn2:

need the second opinion? close this! :)

Could you please elaborate on the reasoning behind this? I believe I have provided reasonable arguments for why the patch should be included.

Yet another argument would be the mere existence of the $depth variable itself. We have it in the method, but it's only use is for providing white-space padding in the HTML source code. The included patch makes use of the variable in another, useful way.

Others than me have clearly seen the use for this, and because it just adds a filter, it is totally unobtrusive.

Before you close this, please read the recommendation from Google (in the link I provided), under the headline: "Use class selectors instead of descendant selectors."

#7 @SergeyBiryukov
12 years ago

  • Keywords dev-feedback added; 2nd-opinion removed

#8 @chriscct7
9 years ago

  • Focuses administration added
Note: See TracTickets for help on using tickets.