Opened 12 years ago
Last modified 5 years ago
#21070 reopened enhancement
Added a filter to the sub-menu class attribute
Reported by: | 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)
Change History (10)
#1
@
12 years ago
- Milestone Awaiting Review deleted
- Resolution set to duplicate
- Status changed from new to closed
#2
@
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
@
12 years ago
- Milestone set to Awaiting Review
- Resolution duplicate deleted
- Status changed from closed to reopened
#6
in reply to:
↑ 5
@
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."
Patch