WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 7 years ago

Last modified 6 years ago

#23834 closed enhancement (fixed)

Menus: add visual Indicator for menu items with children

Reported by: downstairsdev Owned by: helen
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.6
Component: Menus Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

It's helpful to have a visual indicator when a top level menu item contains a hidden drop down menu.

This patch adds a filter to wp_nav_menu_objects in order to add the class "has-children" to top level menu items that contain children. This allows us to style it with psuedo element, similar to how the menu-toggle is styled for smaller screens.

The filter I used was posted by @chipbennet on the WordPress Theme Reviewers List. The same functionality could also be achieved by a Custom Nav Walker (see http://wptheming.com/2013/03/drop-down-menu-indicator/), but Chip's solution seemed more straightforward.

Neither method works unless a menu has actually been set.

Attachments (6)

dropmenu-visual-indicator-1.diff (1.7 KB) - added by downstairsdev 8 years ago.
Visual Indicator Patch
indicator.png (13.9 KB) - added by downstairsdev 8 years ago.
Screen Shot of Indicator
23834.diff (1.1 KB) - added by hotchkissconsulting 7 years ago.
23834.2.diff (2.3 KB) - added by hotchkissconsulting 7 years ago.
Updated patch which fixes default menu
23834.3.diff (2.3 KB) - added by hotchkissconsulting 7 years ago.
changed from array_key_exists() to isset()
23834.4.diff (2.0 KB) - added by helen 7 years ago.

Download all attachments as: .zip

Change History (22)

@downstairsdev
8 years ago

Visual Indicator Patch

@downstairsdev
8 years ago

Screen Shot of Indicator

#1 follow-ups: @downstairsdev
8 years ago

Alternatively, a "has-children" class could just be added to the menus in WordPress core so that it is available to all themes. I'll look into a patch for that next.

#2 @obenland
8 years ago

  • Component changed from General to Bundled Theme
  • Version set to trunk

#3 in reply to: ↑ 1 @lancewillett
8 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.6

Replying to downstairsdev:

Alternatively, a "has-children" class could just be added to the menus in WordPress core so that it is available to all themes. I'll look into a patch for that next.

That would be much better -- that way *all* themes can take advantage of the class and style it as they wish.

#4 in reply to: ↑ 1 @lancewillett
8 years ago

Replying to downstairsdev:

I'll look into a patch for that next.

Try here first: #15214

#5 @lancewillett
8 years ago

  • Keywords has-patch removed
  • Milestone changed from 3.6 to Future Release
  • Summary changed from Twenty Thirteen: Visual Indicator for Drop Down Menus to Twenty Thirteen: add visual Indicator for menu items with children

Chatted about this a lot today during IRC office hours log, and also got an opinion from Joen.

It's a bit late to get the core change in before beta, so we'll punt this to a future release. In case it's helpful, here is some example CSS from Joen to style (based on proposed class names in #15214):

.menu-item-with-sub-menu,
.page_item_with_children {
  content: ‘\f502′;
  font: 8px/1 Genericons;
  -webkit-font-smoothing: antialiased;
  margin-left: 6px;
}

#6 @lancewillett
7 years ago

  • Component changed from Bundled Theme to Menus
  • Milestone changed from Future Release to 3.7
  • Summary changed from Twenty Thirteen: add visual Indicator for menu items with children to Menus: add visual Indicator for menu items with children

Reminder that this is a core change, not just in this theme.

#7 @hotchkissconsulting
7 years ago

Patch added-- this will add the "menu-item-parent" class to parent menu items

@hotchkissconsulting
7 years ago

Updated patch which fixes default menu

#8 @hotchkissconsulting
7 years ago

  • Keywords has-patch added

#9 @hotchkissconsulting
7 years ago

  • Cc hotchkissconsulting added

#10 @nacin
7 years ago

Nice! At a glance, 23834.2.diff looks good. I will note that if we're going to store our data in the key (for speed and easier reduced duplication), we should use isset() rather than array_key_exists(). The latter is only necessary when were are possibly dealing with a null value. In this case, the values are all booleans.

@hotchkissconsulting
7 years ago

changed from array_key_exists() to isset()

@helen
7 years ago

#11 @helen
7 years ago

23834.4.diff - renamed some variables for clarity and changed the class names to be more semantic: menu-item-has-children and page-item-has-children. Tested with wp_nav_menu() and wp_page_menu(). I'm good with it - happy to drop it in at somebody else's okay.

I think at this point, however, we are dupe-ing #15214. Or rather, this should be closed (as we're not adding a visual indicator) and we should use #15214 instead.

#12 @helen
7 years ago

#15214 was marked as a duplicate.

#13 @helen
7 years ago

  • Owner set to helen
  • Resolution set to fixed
  • Status changed from new to closed

In 25602:

Add classes to menus to indicate that an item has sub-items: .menu-item-has-children for wp_nav_menu() and .page_item_has_children for wp_page_menu(). props hotchkissconsulting. fixes #23834.

#14 @johnbillion
7 years ago

#10597 was marked as a duplicate.

#15 @celloexpressions
6 years ago

#14687 was marked as a duplicate.

#16 @McGuive7
6 years ago

  • Keywords 2nd-opinion added

I realize this is closed, but I'd love to know what factored into the decision to split this out into two classes, one for wp_nav_menu() and one for wp_page_menu(). This requires including, at minimum, two CSS selectors when writing rules for parent pages in menus. Can I ask, why not just go with a global .has-children class? Semantically, the two class method doesn't really add any value, as there are already other classes indicating the type of item being output. I may be missing something, but it seems to me that the single class method is more in line with modular CSS practices, and if nothing else, just eliminates the need to include two selectors when styling. Can someone clarify, and/or is this worth re-opening for future improvements?

Last edited 6 years ago by McGuive7 (previous) (diff)
Note: See TracTickets for help on using tickets.