WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#14518 closed defect (bug) (fixed)

The class 'current-category-ancestor' shows on the current category menu item - reproducible

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

Description

Description: If you create a new menu and add a category and add a child category menu item under it then go to the parent category menu item, you will see a class 'current-category-ancestor' when it should be 'current-category' or 'current-category-item'

To reproduce
1) use the TwentyTen theme
2) add two categories: CAT-PARENT and CAT-CHILD
3) add two pages: PAGE-PARENT and PAGE-CHILD
4) add a menu, add the two categories and two pages and adjust them so the children are under the parents
5) select the CAT-PARENT and look at the source
6) select the PAGE-PARENT and look at the source
Note that when the CAT-PARENT is selected it gets a class of 'current-category-ancestor' when if is the current category. This class implies that there is a sub-category that is the current category. The same thing does not happen for the pages.

Here is the HTML that is generated.
Case 1 - menu item CAT-PARENT selected
<div class="menu-header">

<ul id="menu-test" class="menu">

<li id="menu-item-1408" class="menu-item menu-item-type-taxonomy current-menu-item current-category-ancestor"><a href="http://192.168.48.99/wp300/?cat=3">CAT-PARENT</a>

<ul class="sub-menu">

<li id="menu-item-1409" class="menu-item menu-item-type-taxonomy"><a href="http://192.168.48.99/wp300/?cat=8">CAT-CHILD</a></li>

</ul>

</li>
<li id="menu-item-1413" class="menu-item menu-item-type-post_type"><a href="http://192.168.48.99/wp300/?page_id=3">PAGE-PARENT</a>

<ul class="sub-menu">

<li id="menu-item-1411" class="menu-item menu-item-type-post_type"><a href="http://192.168.48.99/wp300/?page_id=7">PAGE-CHILD</a></li>

</ul>

</li>

</ul>

</div>


case 2 - page parent selected
<div class="menu-header">

<ul id="menu-test" class="menu">

<li id="menu-item-1408" class="menu-item menu-item-type-taxonomy"><a href="http://192.168.48.99/wp300/?cat=3">CAT-PARENT</a>

<ul class="sub-menu">

<li id="menu-item-1409" class="menu-item menu-item-type-taxonomy"><a href="http://192.168.48.99/wp300/?cat=8">CAT-CHILD</a></li>

</ul>

</li>
<li id="menu-item-1413" class="menu-item menu-item-type-post_type current-menu-item page_item page-item-3 current_page_item"><a href="http://192.168.48.99/wp300/?page_id=3">PAGE-PARENT</a>

<ul class="sub-menu">

<li id="menu-item-1411" class="menu-item menu-item-type-post_type"><a href="http://192.168.48.99/wp300/?page_id=7">PAGE-CHILD</a></li>

</ul>

</li>

</ul>

</div>

Attachments (1)

objects-not-own-ancestors.14518.diff (1015 bytes) - added by filosofo 4 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 nacin4 years ago

Can you also confirm this in 3.0.1?

comment:2 filosofo4 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.0.2
  • Owner set to filosofo
  • Status changed from new to assigned

I can confirm. objects-not-own-ancestors.14518.diff should fix, by checking that potential ancestors are not the currently-queried objects.

comment:3 follow-ups: juggledad4 years ago

While this patch does remove the 'current-category-ancestor' it now has no class to indicate that this is a category at all.

comment:4 in reply to: ↑ 3 juggledad4 years ago

Replying to juggledad:

While this patch does remove the 'current-category-ancestor' it now has no class to indicate that this is a category at all.

The testing of the patch was with 3.0.1

comment:5 in reply to: ↑ 3 ; follow-up: filosofo4 years ago

Replying to juggledad:

While this patch does remove the 'current-category-ancestor' it now has no class to indicate that this is a category at all.

That sounds like a separate feature request, unrelated to the reported bug in this ticket.

comment:6 in reply to: ↑ 5 juggledad4 years ago

The patch, while fixing one issue, now causes NO category class to show up on a menu item that is a category. Therefore, this patch does not solve it reported bug (which is that the current category does not have a class of 'current-category' or 'current-category-item'.)
Replying to filosofo:

Replying to juggledad:

While this patch does remove the 'current-category-ancestor' it now has no class to indicate that this is a category at all.

That sounds like a separate feature request, unrelated to the reported bug in this ticket.

comment:7 filosofo4 years ago

There are two issues you're discussing:

  • Defect: an incorrect class indicating a menu item is an ancestor, when it isn't
  • Feature Request: a class name to indicate when a menu item corresponds to a category

The first warrants being included in a bugfix release, like 3.0.2, because it's a bug.

The second probably should wait until the next major release, 3.1, if done at all.

comment:8 juggledad4 years ago

Defect: an incorrect class assigned to a menu item - it should be 'current-category' or 'current-category-item' when it is showing as 'current-category-ancestor'

all depends on how you look at it. If I select the CAT-CHILD, with this patch, the CAT-CHILD classes don't indicate that it is a category at all, BUT now the CAT-PARENT shows a class 'current-category-ancestor'. having a menu item showing that it is an 'current-category-ancestor' but the child doesn't specify a class that shows it is a category seems to be a defect to me.

comment:9 filosofo4 years ago

Well, the point is that for the current menu item it's less important to know what kind of item it's associated with. Basically all conceivable styling issues can already be handled. So, for example, if you wanted to style---in particular---current menu items that are categories, the following CSS selector would work:

body.category current-menu-item

In contrast, because non-current menu items can have an indefinite number of relationships to the currently-queried page (category ancestor, post ancestor, etc.), it's more helpful to specify those more particularly.

Anyways, I'm not saying we shouldn't have current-category class; I'm just suggesting that it might be better if they were on separate tickets.

comment:10 juggledad4 years ago

ok, that could work. as long as there is some way to address it. thanks

comment:11 filosofo4 years ago

Patch is still fresh and relevant.

comment:12 nacin4 years ago

  • Keywords commit added
  • Milestone changed from 3.0.3 to 3.1

comment:13 nacin4 years ago

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

(In [16731]) Check that ancestors are not the currently queried objects in nav menu classes. props filosofo, fixes #14518.

Note: See TracTickets for help on using tickets.