WordPress.org

Make WordPress Core

Opened 9 years ago

Last modified 4 months ago

#17077 assigned defect (bug)

Page parent li elements in nav menus not always given current-page-parent class

Reported by: andymacb Owned by:
Milestone: Priority: normal
Severity: minor Version: 3.1
Component: Menus Keywords: needs-patch
Focuses: Cc:
PR Number:

Description

Using a fresh WP3.1.1, 2010 theme, Theme Unit Test data, adding a navigation menu with Parent Page as an item without any child items and then navigating in the browser to Child Page 1.

The <li> for the parent menu item doesn't get assigned the current-page-parent class.

It does get the current-page-ancestor class, and none of my themes demand the current-page-parent class specifically so I've tagged this as trivial.

Attachments (1)

17077.patch (1.2 KB) - added by codeclarified 6 years ago.
Checks if $queried_object parent matches $parent_item ID

Download all attachments as: .zip

Change History (13)

#1 @andymacb
9 years ago

  • Cc am@… added

@codeclarified
6 years ago

Checks if $queried_object parent matches $parent_item ID

#2 @codeclarified
6 years ago

Patch adds check inside _wp_menu_item_classes_by_context() to see if $queried_object->post_parent matches $parent_item->object_id

Also updated the conditional check for applying the classes for backward compatibility. Instead of checking for current-menu-* now checking for current-page-*, seems more correct, however I'm not sure if the -menu- classes were selected for a legitimate reason

#3 @codeclarified
6 years ago

  • Keywords has-patch added

#4 @ruud@…
6 years ago

Hi, I tested the patch in version WordPress 3.7-beta1-25639-src, here are my results:

(only showing relevant classes)

parent page of selected child page:
before patch: current-menu-ancestor current-menu-parent current_page_parent current_page_ancestor
after patch: current-menu-ancestor current-menu-parent current_page_parent current-page-parent

parent page of selected child page after making it is actual parent page (in hierarchy):
before patch: current-menu-ancestor current-menu-parent current_page_parent current_page_ancestor current-page-ancestor
after patch: current-menu-ancestor current-menu-parent current_page_parent current_page_ancestor current-page-ancestor current-page-parent

parent page of selected child page after making it is actual parent page (in hierarchy sense) and after re-saving menu:
before patch: current-menu-ancestor current-menu-parent current_page_parent current_page_ancestor current-page-ancestor current-page-parent
after patch: current-menu-ancestor current-menu-parent current_page_parent current_page_ancestor current-page-ancestor current-page-parent

As far as I can see this patch fixes the problem that the menu had to get saved again to add the 'current-page-parent' class.

However 2 more things are not right in my opinion for the situation where the parent page when their only relation is in the menu:
1) having the 'current-page-parent' class in the li for a page which is acually no parent of any other page seems not appropriate
2) the code in the patch doesn't look for further ancestry.

Thanks!

#5 @ruud@…
6 years ago

  • Cc ruud@… added
  • Component changed from General to Menus
  • Keywords needs-patch added; has-patch removed
  • Severity changed from trivial to minor

OK, I looked into this way further together with @DH-Shredder and @Obenland today at the WCEU contributor day and this is what we found:

  • The initial problem is an effect of the menu not getting updated when a posttype relation is set or un-set.

This latter point is relevant because with this patch only the adding of a posttype relation is represented OK in the menu, but un-setting a relation still reports a parent relation in the menu after un-setting it (if that makes sense).

We concluded that getting rid of this bug, we should try to update any relevant menu's after setting or un-setting a posttype relation.

The second half of the patch (setting the backwards compatible classes based on page-parent instead of menu-parent) is probabbly not a good solution, because the former underscore classes don't reflect the posttype relation, but the menu relation... so checking the menu relation makes sense.

Thanks!

This ticket was mentioned in Slack in #core by jorbin. View the logs.


4 years ago

#7 @jorbin
4 years ago

  • Milestone changed from Awaiting Review to 4.3

The description above still seems accurate. Our docs say that:

.current-{object}-parent
This class is added to menu items that correspond to the hierachical parent of the currently rendered object, where {object} corresponds to the the value used for .menu-item-object-{object}

We should follow our docs.

#8 @obenland
4 years ago

  • Owner set to jorbin
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core by jorbin. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by jorbin. View the logs.


4 years ago

#11 @jorbin
4 years ago

  • Milestone changed from 4.3 to Future Release

Punting due to lack of activity. This needs a patch.

#12 @jorbin
4 years ago

  • Owner jorbin deleted
Note: See TracTickets for help on using tickets.