WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 3 years ago

#17590 closed defect (bug) (fixed)

wp_list_pages() not setting "current_page_item" classes on custom post types

Reported by: tobiasn Owned by: wonderboymusic
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.1
Component: Posts, Post Types Keywords: has-patch commit
Focuses: Cc:

Description (last modified by SergeyBiryukov)

wp_list_pages() don't set the correct css for a hierarchical custom post type.

To reproduce this bug do this:
<?php wp_list_pages("title_li=&post_type=custom_post_type_name"); ?>

and compare the html output to normal pages:
<?php wp_list_pages(); ?>

The problem seems to be this row in wp_list_pages():

if ( is_page() || is_attachment() || $wp_query->is_posts_page )

In the following patch i've changed it to

if ( is_page() || is_attachment() || $wp_query->is_posts_page || ($wp_query->query_vars['post_type'] != 'post' && is_single()) )

(If this is not the intended behavoir please let me know. )

Attachments (6)

post-template.diff (640 bytes) - added by tobiasn 5 years ago.
wp_list_pages() css patch for custom post types
17590.patch (612 bytes) - added by SergeyBiryukov 4 years ago.
17590.2.patch (913 bytes) - added by SergeyBiryukov 4 years ago.
17590.3.patch (884 bytes) - added by Clorith 3 years ago.
Updated .patch to reflect recent changes to post-template.php
17590.4.diff (631 bytes) - added by obenland 3 years ago.
17590.diff (860 bytes) - added by nacin 3 years ago.

Download all attachments as: .zip

Change History (27)

@tobiasn
5 years ago

wp_list_pages() css patch for custom post types

#1 follow-up: @husobj
5 years ago

  • Cc ben@… added

As a temporary measure you can hook into to add selected classes like this:

function my_page_css_class( $css_class, $page ) {
	global $post;
	if ( $post->ID == $page->ID ) {
		$css_class[] = 'current_page_item';
	}
	return $css_class;
}
add_filter( 'page_css_class', 'my_page_css_class', 10, 2 );

#2 @SergeyBiryukov
5 years ago

  • Description modified (diff)

Corrected markup.

#3 in reply to: ↑ 1 @tobiasn
5 years ago

Nice quickfix, however i would prefer pinpointing custom post types.

Replying to husobj:

As a temporary measure you can hook into to add selected classes like this:

function my_page_css_class( $css_class, $page ) {
	global $post;
	if ( $post->ID == $page->ID ) {
		$css_class[] = 'current_page_item';
	}
	return $css_class;
}
add_filter( 'page_css_class', 'my_page_css_class', 10, 2 );

#4 @depi
5 years ago

Thanks for the fix with the hook guys.
However I really wonder why it is still not fixed after 9 months.

#5 @daveleeone
4 years ago

  • Cc daveleeone added
  • Type changed from defect (bug) to feature request

Thanks for the very useful hook, but how can I highlight the parent page? At the moment I use Jquery for this.

Any Ideas?

Thanks, Davelee

#6 @daveleeone
4 years ago

  • Type changed from feature request to defect (bug)

#7 @tobiasn
4 years ago

Hi dave! I haven't noticed the parent page-thing, guess that's a bug in the Walker class. How do I recreate it?

#8 follow-up: @daveleeone
4 years ago

Last edited 4 years ago by daveleeone (previous) (diff)

#9 in reply to: ↑ 8 @tobiasn
4 years ago

Hi Dave! This has nothing to do with the trac bug. Could you please remove the comment here and repost it in the support forum? I'll answer you there.

#10 @thomas.mery
4 years ago

Hi,

I was wondering if this had been looked at ?

it seems like this would be quite useful now that CPT are used so often by the wp community

thanks

#11 @SergeyBiryukov
4 years ago

Closed #21525 as a duplicate.

#12 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 3.5

#13 @SergeyBiryukov
4 years ago

17590.2.patch does a hierarchical check, as suggested during the bug scrub.

is_attachment() was added in [10947] (for #9472), so I've kept it.

#14 @nacin
4 years ago

  • Milestone changed from 3.5 to Future Release

get('post_type') could be an array.

@Clorith
3 years ago

Updated .patch to reflect recent changes to post-template.php

@obenland
3 years ago

#15 @obenland
3 years ago

Updated patch based on SergeyBiryukov's initial pass.

#16 follow-up: @billerickson
3 years ago

Patch looks good to me. Anything I can do to help this get in core? Just ran into this issue myself.

#17 in reply to: ↑ 16 @tobiasn
3 years ago

I think @obenland patch works perfect and addresses the @nacin issue but I don't really know how to get this fix into 3.9. Any suggestions? :)

Replying to billerickson:

Patch looks good to me. Anything I can do to help this get in core? Just ran into this issue myself.

#18 @obenland
3 years ago

  • Milestone changed from Future Release to 3.9

#19 @nacin
3 years ago

  • Component changed from Themes to Post Types
  • Keywords commit added

Looks good. The is_singular() should ensure this is checking a single post type.

@nacin
3 years ago

#20 @nacin
3 years ago

17590.diff is a slightly more defensive approach. get_post_type() simply returns the post type query variable, which could very well have been changed to an array on pre_get_posts even after is_singular was set in parse_query(). Instead, we can use the queried object, which isn't foolproof either but is at least it's consistent for what we're doing here.

#21 @wonderboymusic
3 years ago

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

In 27755:

In wp_list_pages(), add the current_page_item class where applicable when used with a custom post type.

Adds a unit test.

Props nacin.
Fixes #17590.

Note: See TracTickets for help on using tickets.