WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 8 months ago

#13265 closed enhancement (fixed)

Filter results of get_page_templates()

Reported by: nathanrice Owned by: 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 6 years ago.
13265.patch (834 bytes) - added by wpsmith 3 years ago.
Filter moved to WP_Theme
core-page-templates.zip (1.7 KB) - added by wpsmith 3 years ago.
Sample plugin per Nacin's request
core-page-templates.2.zip (1.7 KB) - added by wpsmith 3 years ago.
Sample plugin per Nacin's request
plugin.php (2.0 KB) - added by wpsmith 3 years ago.
test.php (540 bytes) - added by wpsmith 3 years ago.
template file for plugin.php
core-remove-page-templates.php (1.2 KB) - added by downstairsdev 2 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 2 years ago.
Uses the proposed page_templates filter to inject a new template.
13265.diff (4.2 KB) - added by nacin 2 years ago.
Add post context to the filter.

Download all attachments as: .zip

Change History (71)

#1 follow-up: @nacin
6 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
6 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.

@nathanrice
6 years ago

#3 in reply to: ↑ 1 @nathanrice
6 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
6 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
6 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
6 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
6 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
6 years ago

I think #14310 covers this.

#9 @scribu
6 years ago

Nevermind, I confused it with get_page_template().

#10 @scribu
5 years ago

  • Keywords has-patch added

#11 @nathanrice
5 years ago

  • Keywords page templates filter removed

#12 @pross
5 years ago

  • Cc pross@… added

#13 @arpowers
5 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
5 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
5 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
5 years ago

  • Cc mikeschinkel@… added

#17 in reply to: ↑ 14 @johnjamesjacoby
4 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
4 years ago

  • Cc anointed added

#19 @anmari
4 years ago

  • Cc anmari@… added

#20 @wpsmith
4 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
4 years ago

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

#22 @ciobi
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

sorry about that :)

#23 @jontro
4 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
4 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 4 years ago by F J Kaiser (previous) (diff)

#25 follow-up: @nacin
4 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
4 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
4 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
4 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
4 years ago

#22566 was marked as a duplicate.

#30 @dway
4 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
3 years ago

  • Cc pbaylies added

#32 @mordauk
3 years ago

  • Cc pippin@… added

#33 @ethitter
3 years ago

  • Cc erick@… added

@wpsmith
3 years ago

Filter moved to WP_Theme

@wpsmith
3 years ago

Sample plugin per Nacin's request

@wpsmith
3 years ago

Sample plugin per Nacin's request

@wpsmith
3 years ago

@wpsmith
3 years ago

template file for plugin.php

#34 @mercime
3 years ago

  • Cc mercijavier@… added

#35 @SergeyBiryukov
3 years ago

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

#36 @andyadams
3 years ago

  • Cc andyadamscp@… added

#37 @stephenharris
3 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
3 years ago

  • Cc info@… added

#40 @SergeyBiryukov
3 years ago

#24333 was marked as a duplicate.

#41 @c3mdigital
3 years ago

#21891 was marked as a duplicate.

#42 follow-up: @GaryJ
2 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
2 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 2 years ago by downstairsdev (previous) (diff)

@downstairsdev
2 years ago

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

#44 @MikeSchinkel
2 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
2 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
2 years ago

Uses the proposed page_templates filter to inject a new template.

#46 @downstairsdev
2 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
2 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
2 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
2 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
2 years ago

Add post context to the filter.

#50 follow-up: @nacin
2 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
2 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 2 years ago by MikeSchinkel (previous) (diff)

#52 follow-up: @downstairsdev
2 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
2 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
2 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
2 years ago

In 27471:

I am having a bad day. see #13265.

#56 @nacin
2 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
2 years ago

#27967 was marked as a duplicate.

#58 @varun21
23 months 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
10 months ago

#33309 was marked as a duplicate.

#60 @prionkor
10 months 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 months ago

  • Milestone changed from Future Release to 4.4

#62 @DrewAPicture
8 months 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.