Make WordPress Core

Opened 14 years ago

Closed 8 years ago

#13265 closed enhancement (fixed)

Filter results of get_page_templates()

Reported by: nathanrice's profile nathanrice Owned by: nathanrice's profile nathanrice
Milestone: 4.4 Priority: normal
Severity: normal Version: 3.0
Component: Themes Keywords: has-patch
Focuses: template Cc:

Description

It doesn't make sense to not have a filter on the returned value of this function. I can think of several instances where this would be useful.

I also inserted a "pre_" filter at the beginning of the function so plugins/themes can short-circuit the function as well.

I'm hoping this is small enough to get in for 3.0.

Attachments (9)

theme.php.diff (653 bytes) - added by nathanrice 14 years ago.
13265.patch (834 bytes) - added by wpsmith 11 years ago.
Filter moved to WP_Theme
core-page-templates.zip (1.7 KB) - added by wpsmith 11 years ago.
Sample plugin per Nacin's request
core-page-templates.2.zip (1.7 KB) - added by wpsmith 11 years ago.
Sample plugin per Nacin's request
plugin.php (2.0 KB) - added by wpsmith 11 years ago.
test.php (540 bytes) - added by wpsmith 11 years ago.
template file for plugin.php
core-remove-page-templates.php (1.2 KB) - added by downstairsdev 10 years ago.
A plugin test case for the page_templates filter. Removes a page template that is used in the TwentyFourteen theme.
core-add-page-templates.zip (3.1 KB) - added by downstairsdev 10 years ago.
Uses the proposed page_templates filter to inject a new template.
13265.diff (4.2 KB) - added by nacin 10 years ago.
Add post context to the filter.

Download all attachments as: .zip

Change History (71)

#1 follow-up: @nacin
14 years ago

  • Milestone changed from 3.0 to Future Release

There's a typo in the patch. I don't like the idea of adding filters after the feature freeze and especially once we're deep in beta because we generally don't have the time to develop the idea out to the point where we are satisfied with implementation, as we need to then account for filters in the future. Maintaining back compat is difficult, and this just makes it more so.

In this case, two tickets jump out at me for why we should not be rushing this: #12253 and #10959. Just because we return page templates doesn't mean they won't then work.

#2 follow-up: @nacin
14 years ago

  • Cc nacin westi ryan removed

CC should be used so people can subscribe themselves, not so others can subscribe them (unless requested). We look at all of the tickets, we don't need the extra emails.

#3 in reply to: ↑ 1 @nathanrice
14 years ago

Replying to nacin:

There's a typo in the patch.

Indeed there was. Fixed and re-uploaded.

I don't like the idea of adding filters after the feature freeze and especially once we're deep in beta

Fair enough. I knew it was a long-shot for 3.0. Just hoped it was a small enough change to make it in. You guys know more about the larger implications than I do, so I defer to your judgment on that one.

we need to then account for filters in the future. Maintaining back compat is difficult, and this just makes it more so.

Is it just a matter of naming conventions? I've got no problem renaming the filter to something more specific like "get_page_templates" to match the function name, if that helps.

In this case, two tickets jump out at me for why we should not be rushing this: #12253 and #10959. Just because we return page templates doesn't mean they won't then work.

I don't see the relevance in #12253, but I do see some issue with #10959. But given the fact that a plugin/theme could filter, and thus control, the entire array (keys and values), assuming they do it correctly, the new page template would show up in the dropdown with the correct file location as the value (folder/file.php or folder/folder/file.php).

#4 in reply to: ↑ 2 @nathanrice
14 years ago

Replying to nacin:

CC should be used so people can subscribe themselves, not so others can subscribe them (unless requested). We look at all of the tickets, we don't need the extra emails.

Sorry, probably bad timing. I just recall westi mentioning that this was the best way to get him to take a look at a patch:
http://lists.automattic.com/pipermail/wp-hackers/2009-February/024952.html (his last sentence).

If that does still apply, I can understand why that might not necessarily be true at the moment, with 3.0 lurking.

#5 @nacin
14 years ago

  • Cc westi added

Sorry, probably bad timing. I just recall westi mentioning that this was the best way to get him to take a look at a patch: http://lists.automattic.com/pipermail/wp-hackers/2009-February/024952.html (his last sentence).

Fair enough. I'll re-CC westi especially since he was heavily involved in #10959, which I think could pose problems here if we ever revisit #11216. (I meant that ticket, 12253 is actually [12253], the commit in #10959.)

#6 follow-up: @westi
14 years ago

This definitely needs exploration before popping a set of filters in as we are quite particular in what does work.

It would be interesting to see how you intend to use the filter from a plugin too :-)

#7 in reply to: ↑ 6 @nathanrice
14 years ago

  • Cc nathanrice added

Replying to westi:

It would be interesting to see how you intend to use the filter from a plugin too :-)

Plugins, maybe not so much :-) Though, it is theoretically possible, assuming I had access to the page templates array. Would be fun :-)

#8 @scribu
14 years ago

I think #14310 covers this.

#9 @scribu
14 years ago

Nevermind, I confused it with get_page_template().

#10 @scribu
13 years ago

  • Keywords has-patch added

#11 @nathanrice
13 years ago

  • Keywords page templates filter removed

#12 @pross
13 years ago

  • Cc pross@… added

#13 @arpowers
13 years ago

I'd also like to submit a request that something is done with this.. Adding static files for every page template is not ideal.

A filter here would allow page templates to be added dynamically, and would allow page template names to be changed on the fly.

#14 follow-ups: @nacin
13 years ago

This won't be enough to make this work. You'd also have to filter get_page_template() (via the {$type}_template filter in get_query_template(), with $type being 'page'), because the validate_file() check in get_page_template() will fail.

Selecting a special template doesn't help much when the template loader can't do anything with it.

In order to back a patch like this, it would be great seeing a plugin uploaded here that then implements something using it. That way we know we're doing it correctly. If that happens soon this is something that can make it into trunk after some review.

#15 @arpowers
13 years ago

Ya, basically having this functionality is the only thing preventing database driven template handing.

In PageLines' framework, templates are more dynamic. Basically, it allows users to configure their own templates using the 'drag and drop' interface.

Ideally we'd be able to allow them to create and remove templates at will, then set those templates up.
Unfortunately, because of this issue; we'd have to resort to generating static files, which leads to other problems.

We will look in to doing the patch/plugin for you if needed.

#16 @mikeschinkel
13 years ago

  • Cc mikeschinkel@… added

#17 in reply to: ↑ 14 @johnjamesjacoby
12 years ago

Replying to nacin:

This won't be enough to make this work. You'd also have to filter get_page_template() (via the {$type}_template filter in get_query_template(), with $type being 'page'), because the validate_file() check in get_page_template() will fail.

Selecting a special template doesn't help much when the template loader can't do anything with it.

bbPress 2.0/2.1 both have functions to do everything except filter get_page_templates(). I stopped short of that, because it simply was not possible to do.

It's neigh impossible to accurately guess the mark-up needed to match a header/body/footer in a cookie-cutter page template; twentyeleven and twentyten are different enough where the same 1 page template will not work with both themes. As it stands, I'm not convinced that putting a filter on get_page_template() actually will solve the problems outlined here; maybe for parent/child themes, or for creating some kind of "theme compatibility pack" for each specific theme where functionality can be almost guaranteed.

In order to back a patch like this, it would be great seeing a plugin uploaded here that then implements something using it. That way we know we're doing it correctly. If that happens soon this is something that can make it into trunk after some review.

See: bbp-template-functions.php for the functions needed to bypass WordPress limitations.

See: bbp-template-loader.php to see how bbPress mirrors WordPress's template loader.

See: bbp-theme-compatibility.php to see how bbPress shoehorns itself into existing themes.

Even with WordPress core filters, the functions bbPress uses still need to exist; instead they would filter existing WordPress core functions that currently lack filters. The filters you mention above would make it easier for plugins to ship working templates or template parts with their new functionality without needing to write custom template loaders.

If anyone is looking to incorporate a patch for WordPress core and is unsure where to start, I'm happy to provide some direction and support.

#18 @anointed
12 years ago

  • Cc anointed added

#19 @anmari
12 years ago

  • Cc anmari@… added

#20 @wpsmith
12 years ago

  • Cc travis@… added

This would be a good addition, especially for plugins who want to add a page template for certain themes.

#21 @ciobi
12 years ago

  • Cc alex.ciobica@… added
  • Resolution set to fixed
  • Status changed from new to closed

#22 @ciobi
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

sorry about that :)

#23 @jontro
12 years ago

I think this would be a very nice addition as well. We are using wordpress as a pure cms and being able to override the template list would help us greatly.

#24 in reply to: ↑ 14 @F J Kaiser
12 years ago

Replying to nacin:

In order to back a patch like this, it would be great seeing a plugin uploaded here that then implements something using it. That way we know we're doing it correctly. If that happens soon this is something that can make it into trunk after some review.

Normally, the relevance of a plugin is meassured by its downloads. BUT: Every update equals a download. Conclusion: Deliver tons of minor updates & make a plugin become relevant. (As I've seen with my own, every update: downloads += 70%).

If you want to know how relevant such an issue is, then you'll have to search the support channels: Tons of Qs about it.

Last edited 12 years ago by F J Kaiser (previous) (diff)

#25 follow-up: @nacin
12 years ago

I didn't say anything about relevance. I wanted to see a sample plugin that implemented the patch.

#26 in reply to: ↑ 25 ; follow-up: @F J Kaiser
12 years ago

Replying to nacin:

I didn't say anything about relevance. I wanted to see a sample plugin that implemented the patch.

Just that I get this right: You want to know what the possible use case might be?

#27 in reply to: ↑ 26 ; follow-up: @nacin
12 years ago

Replying to F J Kaiser:

Just that I get this right: You want to know what the possible use case might be?

Not even that. I want to see a plugin that successfully implements the hooks in this patch and shows the selected template on the frontend.

#28 in reply to: ↑ 27 @F J Kaiser
12 years ago

Replying to nacin:

Not even that. I want to see a plugin that successfully implements the hooks in this patch and shows the selected template on the frontend.

I know that the current patch isn't working. And I also don't know if it worked for 2.9(? - 12 month ago). The only thing I want is to not let this ticket die, as people seem to need it.

The only real problem is that you can't add templates to the meta box drop down, but as I can't find a ticket for that, I'm adding to this one. If there's a ticket for the drop-down: We don't need the current ticket.

#29 @helenyhou
11 years ago

#22566 was marked as a duplicate.

#30 @dway
11 years ago

It seems this feature request is not getting attention anymore, but it's a really good one. What about its integration to core ?

#31 @pbaylies
11 years ago

  • Cc pbaylies added

#32 @mordauk
11 years ago

  • Cc pippin@… added

#33 @ethitter
11 years ago

  • Cc erick@… added

@wpsmith
11 years ago

Filter moved to WP_Theme

@wpsmith
11 years ago

Sample plugin per Nacin's request

@wpsmith
11 years ago

Sample plugin per Nacin's request

@wpsmith
11 years ago

@wpsmith
11 years ago

template file for plugin.php

#34 @mercime
11 years ago

  • Cc mercijavier@… added

#35 @SergeyBiryukov
11 years ago

@wpsmith: uploading attachments doesn't trigger any email notifications, so whenever you've uploaded something, please leave a comment.

#36 @andyadams
11 years ago

  • Cc andyadamscp@… added

#37 @stephenharris
11 years ago

  • Cc stephenharris added

I don't think the filter should be inside the WP_Theme & parent themes.

Adding the filter in the class method means a call to get_page_templates() will trigger that filter twice (potentially): once for the parent templates - and once for the child & parent templates.

It would make sense to just filter the parent & child templates in the function get_page_templates() instead.

#38 @toscho
11 years ago

  • Cc info@… added

#40 @SergeyBiryukov
11 years ago

#24333 was marked as a duplicate.

#41 @c3mdigital
11 years ago

#21891 was marked as a duplicate.

#42 follow-up: @GaryJ
10 years ago

  • Focuses template added

One small use case that doesn't appear to have been mentioned yet - having a page templates filter would allow a child theme (or plugin) to hide a template included with the parent theme, that the developer doesn't want made available to clients in the Page Templates dropdown list.

#43 in reply to: ↑ 42 @downstairsdev
10 years ago

Replying to GaryJ:

That's the use case I have with a project. I'd like to make certain page templates in the theme available only if portfolio post types are active on the site. I'm working around the lack of filter with javascript for the moment: http://wptheming.com/?p=14753

Below I attached a plugin that uses the page_templates filter to remove a template from TwentyFourteen (page-templates/contributors.php): https://core.trac.wordpress.org/attachment/ticket/13265/core-remove-page-templates.php

If that specific template has been selected for the page the #page-template select box will default to "Default Template"- but the actual template loaded on the page does not update until the post is re-saved.

I also tested @wpsmith's plugin. I was able to select the "test" page template successfully from the admin- however, the template did not appear to load "test.php" template on the front end. Perhaps a little more work is needed? I'll see if I can find a solution.

Last edited 10 years ago by downstairsdev (previous) (diff)

@downstairsdev
10 years ago

A plugin test case for the page_templates filter. Removes a page template that is used in the TwentyFourteen theme.

#44 @MikeSchinkel
10 years ago

I've got the same use-case on my current project too. We have the ability to "skin" a section of the site, and in that section only templates of the selected skin should be valid.

#45 @nathanrice
10 years ago

having a page templates filter would allow a child theme (or plugin) to hide a template included with the parent theme, that the developer doesn't want made available to clients in the Page Templates dropdown list.

I hadn't thought of that. Would be VERY useful.

Do we need an updated patch? What's the next step to getting this ticket reconsidered?

@downstairsdev
10 years ago

Uses the proposed page_templates filter to inject a new template.

#46 @downstairsdev
10 years ago

I modified the code by @wpsmith that adds a template via a plugin. It loads the correct template on the front-end now. See: https://core.trac.wordpress.org/attachment/ticket/13265/core-add-page-templates.zip

The latest patch by @wpsmith still applies fine, just offset a couple lines:
https://core.trac.wordpress.org/attachment/ticket/13265/13265.patch

#47 @nacin
10 years ago

In 27297:

Add a filter to remove or rename page templates for a theme. This does not yet handle adding page templates. see #13265.

#48 follow-up: @nacin
10 years ago

  • Milestone changed from Future Release to 3.9

[27297] handles two common use cases:

  • Removing a page template you don't want to appear.
  • Renaming a page template (this runs post-translation).

It deliberately prevents new templates, for the moment. I didn't want this ticket to keep getting stuck on that without giving theme authors tools that are otherwise straightforward. (One use case highlighted would be a child theme removing a parent theme's template, for example.)

By all means, the discussion for new templates can continue. As downstairsdev can attest, I do follow this ticket, as last night at the WordPress Austin meetup I knew he had posted a patch. :-) I don't really love what happens in plugin.php, does anyone else have a preference?

p.s. I didn't mean to omit props on [27297]; that was a group effort. I'll make sure people end up on the credits page if they don't otherwise.

#49 in reply to: ↑ 48 @MikeSchinkel
10 years ago

Replying to nacin:

[27297] handles two common use cases:

+1 for partial progress.

It deliberately prevents new templates, for the moment.
By all means, the discussion for new templates can continue.

Would you be able to summarize the concerns you see for adding templates and/or potential scenarios where adding might be a problem? I ask so that the 19 people watching this ticket might know what concerns need to be addressed before the ability to add templates could become a reality.

@nacin
10 years ago

Add post context to the filter.

#50 follow-up: @nacin
10 years ago

13265.diff adds post context to the filter. This would be necessary so as not to rely on the global post for things like deprecating a template and only allowing it to be used for posts that already use it.

Would you be able to summarize the concerns you see for adding templates and/or potential scenarios where adding might be a problem?

At the moment, page template values are relative paths to physical template files. I'll be honest, the proposed plugins are cleaner than I was expecting, but the use of the template_include hook has me wanting something a bit better.

I suppose this could work:

add_filter( 'page_templates', function( $templates ) {
    $template['my_plugin_template_one'] = 'My Plugin Template';
});

add_filter( 'page_template', function( $template ) {
    if ( is_page_template( 'my_plugin_template_one' ) ) {
        return dirname( __FILE__ ) . '/templates/one.php';
    }
    return $template;
});

Something feels wrong about it. Page templates are fairly rigid, are only available for pages, and are intrinsic to the theme. For nearly all use cases, a plugin can't just call get_header() and get_footer() and pray the markup will work. It makes sense that a child theme or a parent theme needs to register them, at least in their current form. That's more of an indictment of themes, yes, but my point is templates in plugins just don't work well. So should we encourage what is a certain level of "slop"?

#51 in reply to: ↑ 50 @MikeSchinkel
10 years ago

Replying to nacin:

Something feels wrong about it. Page templates are fairly rigid, are only available for pages, and are intrinsic to the theme. For nearly all use cases, a plugin can't just call get_header() and get_footer() and pray the markup will work. It makes sense that a child theme or a parent theme needs to register them, at least in their current form. That's more of an indictment of themes, yes, but my point is templates in plugins just don't work well.

Ok, thanks for taking the effort to clarify and I do indeed see the concern. OTOH let me give you a use-case where adding page templates could be beneficial and yet would not violate your concerns?

Consider a soft drink company with many brands that has a large WordPress website for a company-wide campaign where they allow a representative from each of their brands to publish pages on this shared website. Of course each brand has it's own design standards and each brand hires their own agency to build out their brand's WordPress page templates. The IT group for this company has set up the server so that each brand can update their brand's folder but only their folder (after an exhaustive code review, of course.) The brand folders are located like so:

  • /wp-content/themes/soft-drinks-r-us/brands/{brand}/

Thus either the theme or ideally a must-use plugin would need to be able to add the pages templates for the brand associated with the current logged-in user. In the case of a plugin it would just be providing the logic about where to find the page templates in the theme but even if done in the theme's functions files it would still need to be able to add files using the new 'page_templates' filter.

One way to support this use-case yet still disallow plugins adding their own templates would be to simply disallow any new absolute paths but do allow any new relative paths.

However...

So should we encourage what is a certain level of "slop"?

While the usage you describe would definitely be slop I could envision valid use-cases for having a plugin add a page template of it's own such as when the plugin needs to control the formatting. Image a promotional coupon plugin that allowed the user to publish a coupon with a URL path of /christmas-2014-promo/ or similar. Maybe the plugin developer wants the coupon to be formatted to look like an award certificate; in that case the plugin would rightly need to be in complete control of the formatting, and such a plugin would be a lightweight way for a motivated DIYer to add specialty coupons to their website.

Another use-case could be for a poorly-funded government agency that has a [mandate to provide an API for their data. A plugin could provide one of their non-technical staffers a page template that would format the values they typed into meta fields to be output as a JSON file, maybe at an URL path of /api/crime-stats/2013/.

Of course I'm stretching these use-cases a bit to provide examples that people might want to achieve, but limiting them means they definitely will not be any happy accidents for use-cases that require adding page templates. And it's really hard to know what good things people might dream up.

Conversely I'd ask if the potential downside of allowing it is likely to result in significant negative behavior. First off it's something people will have to learn how to do, it's not a case of them doing by accident. And most people building plugins want people to use their plugin but if they create a bad UX then fewer people will use it so it would seem to have a built-in disincentive to use badly. And disallowing it removes the potential for positive serendipity so why not allow and just document in the hook that it's a bad idea to use it to add page templates that expect their layout to work in an existing theme?

But even so, please at least allow adding page templates with relative paths, i.e. within deeper subdirectories.

Last edited 10 years ago by MikeSchinkel (previous) (diff)

#52 follow-up: @downstairsdev
10 years ago

Thanks for the commit @nacin. This solves my use case of removing page templates: https://github.com/devinsays/portfolio-press/commit/3e5e50f5e25ee5303f26bd15b78eed146bef6ebc

@MikeSchinkel I didn't quite understand your first use case until I tested it. Page templates are available if they are in theme/folder/page-template.php, but not if they are another directory level deep, e.g. theme/folder/folder/page-template.php. I didn't realize that. I had assumed WordPress would find a template anywhere in the theme directory structure.

For a plugin to add a page template I would expect to:

1) Be completely self-sufficient (i.e. not rely on theme templates or styling)
2) Require the use of page data in some way (title, content, meta fields)

I think @MikeSchinkel's examples meet the criteria.

I could see a potential issue if someone wanted to create a basic theme and then offer additional features through a paid add-on plugin. They might be tempted to include the additional templates in the plugin even though it would be much preferable to have them in the theme folder.

However, I don't feel too strongly either way on this topic.

#53 in reply to: ↑ 52 @MikeSchinkel
10 years ago

Replying to downstairsdev:

@MikeSchinkel I didn't quite understand your first use case until I tested it. Page templates are available if they are in theme/folder/page-template.php, but not if they are another directory level deep, e.g. theme/folder/folder/page-template.php. I didn't realize that. I had assumed WordPress would find a template anywhere in the theme directory structure.

Yep, get_page_templates() uses WP_Theme::get_files() to one level deep, not more, probably for performance reasons.

#54 @nacin
10 years ago

In 27470:

Rename the new page_templates filter to theme_page_templates, and pass it a post object for proper context.

see #13265.

#55 @nacin
10 years ago

In 27471:

I am having a bad day. see #13265.

#56 @nacin
10 years ago

  • Milestone changed from 3.9 to Future Release

I opened #27334 to track the removing and renaming of page templates using the theme_page_templates filter. It is now closed as fixed on 3.9, and I'm going to move this ticket back out to Future Release for further discussion on adding page templates.

#57 @SergeyBiryukov
10 years ago

#27967 was marked as a duplicate.

#58 @varun21
10 years ago

Here's a use case for being able to add new templates:

An example would be child themes (like Prose from StudioPress) which may want to allow user's to create custom templates and keep them outside the theme's directory (like under /uploads/prose/mytemplate.php) so that the template isn't deleted when the theme is upgraded.

#59 @dd32
9 years ago

#33309 was marked as a duplicate.

#60 @prionkor
9 years ago

Listing another use case of being able to add (fake) templates on the dropdown list (also check #33309).

I have an MVC structured theme. Where I do not want to create files for page templates. Instead all the templates are generated by a master factory class placed in the index.php. The master class decides and calls view classes depending on the request.

An simplified example if the template file name is view/no-sidebar.php it will call View\No_Sidebar class etc.

So, in order to achieve that I would need to add new items to the dropdown list. Currently I am using blank page template files in order to add items on the list.

I would vote for removing array_intersect_assoc() and have to full control to developers.

WPSE thread

http://wordpress.stackexchange.com/questions/196995/adding-items-to-page-template-dropdown-on-page-edit-screen)

#61 @DrewAPicture
8 years ago

  • Milestone changed from Future Release to 4.4

#62 @DrewAPicture
8 years ago

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

In 34995:

Template: Make it possible to both add and remove items from the page templates list using the theme_page_templates filter.

The theme_page_templates hook was originally added in [27297] as page_templates, and later renamed in [27470]. Previously, it was only possible to remove or rename page templates via this hook.

Fixes #13265. Fixes #25879.

Note: See TracTickets for help on using tickets.