WordPress.org

Make WordPress Core

Opened 3 years ago

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

Download all attachments as: .zip

Change History (27)

tobiasn3 years ago

wp_list_pages() css patch for custom post types

comment:1 follow-up: husobj3 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 SergeyBiryukov3 years ago

  • Description modified (diff)

Corrected markup.

comment:3 in reply to: ↑ 1 tobiasn3 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 depi2 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 daveleeone2 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 daveleeone2 years ago

  • Type changed from feature request to defect (bug)

comment:7 tobiasn2 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: daveleeone2 years ago

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

comment:9 in reply to: ↑ 8 tobiasn2 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.mery22 months 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 SergeyBiryukov21 months ago

Closed #21525 as a duplicate.

SergeyBiryukov20 months ago

comment:12 SergeyBiryukov20 months ago

  • Milestone changed from Awaiting Review to 3.5

SergeyBiryukov18 months ago

comment:13 SergeyBiryukov18 months 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 nacin17 months ago

  • Milestone changed from 3.5 to Future Release

get('post_type') could be an array.

Clorith10 months ago

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

obenland8 months ago

comment:15 obenland8 months ago

Updated patch based on SergeyBiryukov's initial pass.

comment:16 follow-up: billerickson3 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 tobiasn3 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 obenland3 months ago

  • Milestone changed from Future Release to 3.9

comment:19 nacin3 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.

nacin2 months ago

comment:20 nacin2 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 wonderboymusic3 weeks 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.