Make WordPress Core

Opened 15 years ago

Closed 8 years ago

#10676 closed enhancement (fixed)

current-cat-ancestor in wp_list_categories

Reported by: spathon's profile spathon Owned by: boonebgorges's profile boonebgorges
Milestone: 4.5 Priority: normal
Severity: normal Version: 2.8.5
Component: Taxonomy Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

The wp_list_categories should apply current-cat-ancestor like wp_list_pages not just current-cat-parent to the closest parent

Attachments (5)

category-template-10676.diff (752 bytes) - added by ardathksheyna 11 years ago.
10676.diff (799 bytes) - added by wonderboymusic 11 years ago.
10676_short_circuit.diff (822 bytes) - added by jrchamp 10 years ago.
10676_wordpress_4_4.diff (894 bytes) - added by jrchamp 8 years ago.
10676.2.diff (2.2 KB) - added by swissspidy 8 years ago.

Download all attachments as: .zip

Change History (22)

#1 @spathon
14 years ago

  • Milestone changed from Future Release to 2.9
  • Version changed from 2.8.4 to 2.8.5

#2 @azaozz
14 years ago

  • Milestone changed from 2.9 to Future Release

No patch.

#3 @ardathksheyna
11 years ago

  • Cc jgreen@… added

#4 @ardathksheyna
11 years ago

  • Keywords has-patch needs-testing added

#5 @DrewAPicture
11 years ago

  • Component changed from General to Taxonomy
  • Keywords class categories removed

#6 @wonderboymusic
11 years ago

  • Milestone changed from Future Release to 3.7

10676.diff​ rehabs the whitespace

#7 @nacin
10 years ago

Two considerations here:

  • Are the ancestor queries OK and worth it?
  • Does the class name we're going with make sense?

#8 @nacin
10 years ago

  • Type changed from feature request to enhancement

#9 @nacin
10 years ago

  • Keywords 2nd-opinion added
  • Milestone changed from 3.7 to Future Release

Punting pending answers to the two questions.

#10 @jrchamp
10 years ago

The class name makes sense. It's the logical conclusion given that I reimplemented it before finding this ticket.

I dislike the use of get_ancestors() because it has extra overhead, we already have the first term and only need to check the terms until the point that it matches. I'm attaching a modified version of this patch that doesn't waste time array_reversing and leverages the short circuit opportunities to often avoid the get_ancestors() call altogether.

Really, given the inherent static response given by the get_ancestors() call for the provided current_category, if the Walker_Category object initialized a private member variable with the get_ancestors() array the first time it was needed, you could easily use a simple isset on the member variable on all subsequent calls. Additionally, if this path were pursued I would recommend an array_flip to allow use of the isset language construct on the array of ancestor IDs itself rather than the much slower in_array() function.

#11 @SergeyBiryukov
9 years ago

#30635 was marked as a duplicate.

#13 @swissspidy
8 years ago

  • Keywords needs-refresh needs-unit-tests added; needs-testing removed

#14 @jrchamp
8 years ago

  • Keywords needs-refresh removed

Refreshed for 4.4

#15 @boonebgorges
8 years ago

  • Keywords 2nd-opinion removed

The technique in 10676_wordpress_4_4.diff looks good to me. Improved cache support for get_term() in 4.4 should minimize the overhead of walking the ancestor tree like this.

Can we get tests for this? At least one, which would show the class being applied to multiple levels of ancestor. (We test these classes via wp_list_categories(): https://core.trac.wordpress.org/browser/tags/4.4/tests/phpunit/tests/category/wpListCategories.php

@swissspidy
8 years ago

#16 @swissspidy
8 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed
  • Milestone changed from Future Release to 4.5

10676.2.diff is a new patch containing a unit test for the new class.

#17 @boonebgorges
8 years ago

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

In 36008:

Add current-cat-ancestor class to ancestor items in wp_list_categories().

Pairs nicely with current-cat-parent.

Props jrchamp, swisssipdy, ardathksheyna, wonderboymusic.
Fixes #10676.

Note: See TracTickets for help on using tickets.