WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#14053 closed enhancement (fixed)

Weird, and seemingly incorrect, classes on menu items

Reported by: nathanrice Owned by: filosofo
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.0
Component: Menus Keywords: has-patch dev-feedback
Focuses: Cc:

Description

Forgive me if this has been reported already, but I couldn't find anything. After looking at the source output of a nav menu (in various contexts), I see some inconsistency in the class output for individual menu items.

(BTW, using the latest trunk as of 5:30pm Eastern)

For instance, when I put a Home menu item in a menu, then view the homepage, that menu item has the following classes applied:

class="menu-item menu-item-type-custom current-menu-item menu-item-home current_page_item"

However, when I click over to another page, the Home menu item class output changes to this:

class="menu-item menu-item-type-custom"

That's all fine, except I would assume that the 'menu-item-home' should stay on that menu item, since it is telling us that this is the "home menu item". I wouldn't think that class should change.

Also, when I put a page (let's say an "About Me" page menu item in the menu, the class output is something like this:

class="menu-item menu-item-type-post_type"

Shouldn't that second class be "menu-item-type-page" instead? The 'post_type' output seems like a bug.

Finally, when a menu item has children (or grandchildren, etc.), why isn't there something in the class string to indicate this? Like 'menu-item-parent' or 'menu-item-ancestor' ???

#12812 was a good start, but I can conceive a scenario where I'd like to do something special to pages that have submenu items, even if the page itself (or its subpages) don't happen to be the current active menu item.

I don't have a patch. But I feel like this might at least get the conversation going.

Attachments (2)

fix-menu-item-home-class.14053.diff (796 bytes) - added by filosofo 5 years ago.
menu-item-object-class.14053.diff (664 bytes) - added by filosofo 5 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 follow-up: @filosofo5 years ago

fix-menu-item-home-class.14053.diff should fix the home-link class issue. Good catch.

I think the idea behind using menu-item-type-post_type is that now it's increasingly likely for there to be unexpected post types; hence the more general class can apply styling rules to unanticipated menu items.

comment:2 @filosofo5 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Unassigned to 3.0.1
  • Version set to 3.0

comment:3 @nacin5 years ago

Patch looks good.

comment:4 @nacin5 years ago

(In [15302]) Assign menu-item-home also when menu item isn't current. props filosofo, see #14053 for trunk.

comment:5 @nacin5 years ago

(In [15303]) Assign menu-item-home also when menu item isn't current. props filosofo, see #14053 for 3.0.

comment:6 in reply to: ↑ 1 ; follow-up: @nathanrice5 years ago

Replying to filosofo:

fix-menu-item-home-class.14053.diff should fix the home-link class issue. Good catch.

I think the idea behind using menu-item-type-post_type is that now it's increasingly likely for there to be unexpected post types; hence the more general class can apply styling rules to unanticipated menu items.

I agree, if this was an unknown post-type. But this is a page. Shouldn't it be menu-item-type-page?

Also, if I need to create other tickets for the issues other than the home class, let me know. I didn't really think that through when I created this ticket.

comment:7 in reply to: ↑ 6 @filosofo5 years ago

  • Type changed from defect (bug) to enhancement

Replying to nathanrice:

I agree, if this was an unknown post-type. But this is a page. Shouldn't it be menu-item-type-page?

In menu item jargon, "type" is the family of objects such as "taxonomy" or "post_type"; the "object" is the specific kind within that family, such as "page."

menu-item-object-class.14053.diff adds the class 'menu-item-object-page' to pages, mutatis mutandis for the other object kinds.

comment:8 follow-up: @nacin5 years ago

Can we split this ticket up into class improvement for 3.1 and things we need to fix in 3.0.1? (In that case, are we done on 3.0.1?)

comment:9 @filosofo5 years ago

  • Milestone changed from 3.0.1 to 3.1

I think that's the right approach.

comment:10 in reply to: ↑ 8 @nathanrice5 years ago

Replying to nacin:

Can we split this ticket up into class improvement for 3.1 and things we need to fix in 3.0.1? (In that case, are we done on 3.0.1?)

Yeah, good point. The only bug reported in this ticket has been fixed. Thanks nacin and folosofo!!!

comment:11 @filosofo5 years ago

  • Milestone changed from Awaiting Triage to 3.1
  • Owner set to filosofo
  • Status changed from new to assigned

comment:12 @filosofo4 years ago

  • Keywords dev-feedback added

What about getting in menu-item-object-class.14053.diff? The patch applies cleanly and it's trivial.

comment:13 @nacin4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [16095]) Add menu-item-object-$object class. props filosofo, fixes #14053.

Note: See TracTickets for help on using tickets.