WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 14 months 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 4 years ago.
wp_list_pages() css patch for custom post types
17590.patch (612 bytes) - added by SergeyBiryukov 3 years ago.
17590.2.patch (913 bytes) - added by SergeyBiryukov 3 years ago.
17590.3.patch (884 bytes) - added by Clorith 2 years ago.
Updated .patch to reflect recent changes to post-template.php
17590.4.diff (631 bytes) - added by obenland 22 months ago.
17590.diff (860 bytes) - added by nacin 16 months ago.

Download all attachments as: .zip

Change History (27)

@tobiasn4 years ago

wp_list_pages() css patch for custom post types

comment:1 follow-up: @husobj4 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 );

comment:2 @SergeyBiryukov4 years ago

  • Description modified (diff)

Corrected markup.

comment:3 in reply to: ↑ 1 @tobiasn4 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 );

comment:4 @depi3 years ago

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

comment:5 @daveleeone3 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

comment:6 @daveleeone3 years ago

  • Type changed from feature request to defect (bug)

comment:7 @tobiasn3 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?

comment:8 follow-up: @daveleeone3 years ago

Hi Tobias, thanks for reply.
I have not seen, unfortunately, you've written back to me.

here is my code in the template:

function file:

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 );

template file:

<?php
// create a ID of parent page to use the wp_list_page as submenu
			if ($post->post_parent)	{
				$ancestors=get_post_ancestors($post->ID);
				$root=count($ancestors)-1;
				$parent = $ancestors[$root];
			} else {
				$parent = $post->ID;
			}

?>	

<ul class="submenu">
<?php 
// create a Submenu for CPT's 

	$args = array(
	'post_type'	=>	'event',
	'child_of'	=>	$parent,
	'title_li'	=>	__('')
);
wp_list_pages( $args ); 
?> 
</ul>

jquery file

 $(document).ready(function(){
	$('.submenu li:has(li.current_page_item)').addClass('parent_page_item');	

});

I hope it helps.
if you make it I'll pay you a beer :)

Version 0, edited 3 years ago by daveleeone (next)

comment:9 in reply to: ↑ 8 @tobiasn3 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.

comment:10 @thomas.mery3 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

comment:11 @SergeyBiryukov3 years ago

Closed #21525 as a duplicate.

@SergeyBiryukov3 years ago

comment:12 @SergeyBiryukov3 years ago

  • Milestone changed from Awaiting Review to 3.5

@SergeyBiryukov3 years ago

comment:13 @SergeyBiryukov3 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.

comment:14 @nacin3 years ago

  • Milestone changed from 3.5 to Future Release

get('post_type') could be an array.

@Clorith2 years ago

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

@obenland22 months ago

comment:15 @obenland22 months ago

Updated patch based on SergeyBiryukov's initial pass.

comment:16 follow-up: @billerickson16 months ago

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

comment:17 in reply to: ↑ 16 @tobiasn16 months 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.

comment:18 @obenland16 months ago

  • Milestone changed from Future Release to 3.9

comment:19 @nacin16 months 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.

@nacin16 months ago

comment:20 @nacin16 months 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.

comment:21 @wonderboymusic14 months 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.