#33460 closed enhancement (fixed)
wp_list_categories should display title_li only if the $categories is not empty , or have an option to hide it if $categories is empty
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | normal | Version: | 4.4 |
Component: | Taxonomy | Keywords: | has-patch 2nd-opinion |
Focuses: | Cc: |
Description
This part of wp_list_categories:
if ( $r['title_li'] && 'list' == $r['style'] ) { $output = '<li class="' . esc_attr( $r['class'] ) . '">' . $r['title_li'] . '<ul>'; }
Should be
if ( $r['title_li'] && 'list' == $r['style'] && (!empty($categories))) { $output = '<li class="' . esc_attr( $r['class'] ) . '">' . $r['title_li'] . '<ul>'; }
or is this intended behavior?
Attachments (4)
Change History (16)
#1
follow-up:
↓ 3
@
10 years ago
- Keywords needs-patch good-first-bug added
- Milestone changed from Awaiting Review to Future Release
#3
in reply to:
↑ 1
@
10 years ago
Replying to boonebgorges:
Thanks for the ticket, vilkatis. Welcome to Trac!
I'm not sure whether it's intended behavior, but it's been like this forever, and I think it'd cause problems to change it so that the title doesn't appear when there are no categories to show.
However, having an option to hide the title if
$categories
is empty is not a bad idea. Patches welcome.
Thank you !
I hope I did everything right , I have read the codex about patches.
#4
@
10 years ago
- Milestone changed from Future Release to 4.4
Thanks, vilkatis! Patch looks pretty good. I'm thinking about the best name for this parameter. It seems to me that most of our existing params of this type begin with 'hide_' - 'hide_empty', 'hide_if_empty', etc. So maybe something like 'hide_title_if_no_cats', which would default to false? My only concern is that it feels a little like a double-negative. See 33460.diff, which adds unit tests.
#5
follow-up:
↓ 8
@
10 years ago
- Keywords needs-testing removed
Yeah your code looks great , it really makes more sense naming it that way. I went over the unit tests to understand them and I think I got the idea of how those work.
What's the next step for me to do ?
#6
@
10 years ago
- Owner set to boonebgorges
- Resolution set to fixed
- Status changed from new to closed
In 33764:
#8
in reply to:
↑ 5
@
10 years ago
Replying to vilkatis:
Yeah your code looks great , it really makes more sense naming it that way. I went over the unit tests to understand them and I think I got the idea of how those work.
What's the next step for me to do ?
Nothing. We're done here :) Thanks for the contribution!
#9
@
10 years ago
- Keywords 2nd-opinion added; good-first-bug removed
- Resolution fixed deleted
- Status changed from closed to reopened
I'd like to suggest that we change the name of the new argument to hide_title_if_empty
. This does two things:
- It's gets us away from pigeonholing the argument to categories, especially since this function can be used with other taxonomies
- It follows more closely with the naming scheme of other "empty"-related arguments.
On point #1, if/when we move to taxonomy-agnostic namespacing, this would be one less thing we'd have to account for if/when wp_list_categories()
becomes a wrapper for a wp_list_terms()
function, for instance. Plus I think it's just clearer all around.
Thanks for the ticket, vilkatis. Welcome to Trac!
I'm not sure whether it's intended behavior, but it's been like this forever, and I think it'd cause problems to change it so that the title doesn't appear when there are no categories to show.
However, having an option to hide the title if
$categories
is empty is not a bad idea. Patches welcome.