WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#18750 closed enhancement (fixed)

specify post ID for is_page_template

Reported by: billerickson Owned by: nacin
Milestone: 3.4 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: Cc:

Description

is_page_template() checks the $wp_query object for the page ID. It would be great if we could override this with our own ID as a separate variable:

is_page_template( $template, $id );

Two instances where I've used this today:

  • In a page template to see if the current page's parent is also using the template
  • Limiting metaboxes to display only if a certain page template is used

Attachments (4)

18750.diff (1.1 KB) - added by billerickson 4 years ago.
18750.2.diff (881 bytes) - added by billerickson 4 years ago.
18750.3.diff (1.7 KB) - added by billerickson 4 years ago.
18750.4.diff (1.5 KB) - added by billerickson 4 years ago.

Download all attachments as: .zip

Change History (28)

@billerickson4 years ago

comment:1 @billerickson4 years ago

  • Cc bill.erickson@… added
  • Keywords has-patch dev-feedback added

comment:2 @duck_4 years ago

I don't think that this would be the best way of doing this. It seems that all you really require is a wrapper around:

get_post_meta( $id, '_wp_page_template', true );

A wrapper function, e.g. get_page_template( $id ), so that you don't have to rely on specifying a private meta key (_wp_page_template).

comment:3 follow-up: @billerickson4 years ago

Ah, I didn't know about get_page_template(). So you recommend creating a patch for that to accept an ID?

comment:4 in reply to: ↑ 3 @duck_4 years ago

Replying to billerickson:

Ah, I didn't know about get_page_template(). So you recommend creating a patch for that to accept an ID?

Well I didn't think of it on purpose. I was actually giving an example name for a new function that would just call get_post_meta( $id, '_wp_page_template', true ); and didn't check if it already existed.

@billerickson4 years ago

comment:5 @billerickson4 years ago

New patch creates new function, get_the_page_template( $id ), which is simply a wrapper for get_post_meta.

get_page_template() returns the full path of the template file using get_query_template(), so it wasn't a good fit for this enhancement.

comment:6 @duck_4 years ago

You re-patched and replied whilst I was writing this, but I'm going to put it in anyway since I wrote it. It looks like you got what I meant, but note that my example is much simpler as it isn't written to work in the loop without an ID.

---

Sorry, didn't really answer you :( Pretty much starting again...

From your two use case bullet points it sounds like you just want to do something like:

if ( get_post_meta( $current_id, '_wp_page_template', true ) == get_post_meta( $parent_id, '_wp_page_template', true ) )
    // do some stuff as the templates are the same

and

if ( 'some_template' == get_post_meta( $id, '_wp_page_template', true ) )
    // add metaboxes as the correct template is set

I then suggested the possibility of a new function which would just be a wrapper for the get_post_meta call so that you don't have to be specifying a core meta key. (Foolishly I managed to suggest a new function that already exists!) So you would end up taking the above examples and replacing get_post_meta( $id, '_wp_page_template', true ) with possible_new_function( $id ). Where:

function possible_new_function( $id ) {
    return get_post_meta( $id, '_wp_page_template', true );
}

or something similar.

Why didn't I think it should be a mod to is_page_template()? Because that is specifically for determining "whether currently in a page template" with the ability to specify a certain template. It's not a more abstract function for querying a page's template.

Hope that makes sense :) P.S. All code is untested.

comment:7 follow-up: @billerickson4 years ago

Thanks for the detailed response and clarification. Initially I thought you were saying get_page_template() already is a wrapper function, but once I looked at it I realized what you meant.

I think the function is slightly more useful with it not requiring an ID, but let me know if you think it's better without and I'll update. Either way it will work for my use case.

My only worry is that it could have a better name since the two function names don't indicate clearly what the difference is.

comment:8 @scribu4 years ago

The naming problem comes from the fact that we don't have a clear name for templates set via _wp_page_template.

The new function should be used in get_page_template() and is_page_template(), which currently uses some clunky code.

comment:9 @Master Jake4 years ago

  • Cc chappellind@… added

I second (or third or fourth) the need for this capability.

comment:10 in reply to: ↑ 7 @mikeschinkel4 years ago

Replying to billerickson:

My only worry is that it could have a better name since the two function names don't indicate clearly what the difference is.

get_page_template_name()?

comment:11 @scribu4 years ago

Here's another suggestion: get_custom_page_template()

To summarize, we went from the initial proposal:

is_page_template( $template, $id )

to

get_custom_page_template( $id ) == $template

comment:12 @billerickson4 years ago

New patch:

  • Renamed function to get_custom_page_template()
  • Made a few minor modifications to it (duplicated how is_page_template() used the global $wp_query to get the ID)
  • Used the function in is_page_template() and get_page_template()

@billerickson4 years ago

comment:13 @scribu4 years ago

This part seems superfluous, since get_post_meta() already returns false if there's no template to be found:

 	1280	        if( !empty( $page_template ) ) 
 	1281	                return $page_template; 
 	1282	        else 
 	1283	                return false;

Also, coding standards: if ( instead of if(.

comment:14 @scribu4 years ago

One more thing: get_queried_object_id() instead of $wp_query->get_queried_object_id().

@billerickson4 years ago

comment:15 @billerickson4 years ago

  • Reduced the code to simply return get_post_meta
  • Removed the $wp_query parts
  • Updated if statement to use WP coding standards

comment:16 @scribu4 years ago

  • Keywords dev-feedback removed
  • Milestone changed from Awaiting Review to 3.4

Looks good to me.

comment:17 @goto104 years ago

  • Cc dromsey@… added

comment:18 @nacin3 years ago

  • Owner set to nacin
  • Status changed from new to accepted

Looks good to me as well. I'll get it in next week.

Let's keep thinking about the function name, though. Here's what's been suggested here:

  • get_the_page_template()
  • wp_get_page_template()
  • get_custom_page_template()
  • get_custom_page_template_name()
  • get_page_template_for_post() (even though it is for pages)
  • get_page_template_name()
  • possible_new_function()

I like get_page_template_name(), myself. "Custom" doesn't have much of a place here.

comment:19 @nacin3 years ago

Though in fairness, it's not the name we're getting back, it's the slug. The name is different.

comment:20 @billerickson3 years ago

How about get_page_template_slug() ?

comment:21 @nacin3 years ago

This function should not use get_queried_object_id() by default, as that is not typical of a post function. Rather, it should default to get_the_ID().

comment:22 @nacin3 years ago

we should return false when no page template is chosen for a page (as in, 'default'). There is some history here.

Primarily, #12653. We somewhat enforce _wp_page_template to be 'default' for pages. It should not be empty even when no special page template is chosen. Also see #18018 and #17458.

comment:23 @nacin3 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In [20075]:

Introduce get_page_template_slug( $id = null ) to return a page's template (like "showcase.php"). Returns false if post ID is not a page, and an empty string for the default page template. Use the function across core. props billerickson for initial patch. fixes #18750.

comment:24 @nacin3 years ago

Since is_page_template() returns false for the default page template, I wanted this function to return a falsey value. But I also understand there could be a reason to differentiate between not-a-page and default page template, hence false and '' for that edge case use.

Note: See TracTickets for help on using tickets.