WordPress.org

Make WordPress Core

Opened 12 months ago

Last modified 12 months ago

#21070 reopened enhancement

Added a filter to the sub-menu class attribute

Reported by: bjornjohansen Owned by:
Priority: normal Milestone: Awaiting Review
Component: Menus Version: 3.4
Severity: normal Keywords: has-patch dev-feedback
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 months ago.
Patch
functions.php (223 bytes) - added by bjornjohansen 12 months ago.
Example usage of the filter

Download all attachments as: .zip

Change History (9)

bjornjohansen12 months ago

Patch

bjornjohansen12 months ago

Example usage of the filter

comment:1 SergeyBiryukov12 months ago

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

comment:2 bjornjohansen12 months 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.

comment:3 SergeyBiryukov12 months ago

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

comment:4 SergeyBiryukov12 months ago

  • Keywords 2nd-opinion added

comment:5 follow-up: alexvorn212 months ago

need the second opinion? close this! :)

comment:6 in reply to: ↑ 5 bjornjohansen12 months 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."

comment:7 SergeyBiryukov12 months ago

  • Keywords dev-feedback added; 2nd-opinion removed
Note: See TracTickets for help on using tickets.