Add support for WordPress Page templates

[DJPaul]

Our fake WordPress Pages, which we map to the various components to "help" our URL routing, have a consequence that occasionally confuses newcomers: when you Edit the page in wp-admin, you can pick a Template (in the Page Attributes metabox). People expect it to work in the same way that it would for any normal WordPress Page, but it doesn't.

I'm pretty sure this comes up on the forums occasionally, and I helped someone with this confusion in person at a recent contributor day. Hugo and I talked about how exciting having another level of per-component page template control would be. It's not actually too complex; I've attached a patch.

[johnjamesjacoby]

Should $page_template take precedence if it's found, ahead of $primary_templates?

[hnla]

Going to update an install in the morning and test - this will be really useful for users and allow a great deal of additional but easy flexibility for them.

[DJPaul]

Replying to johnjamesjacoby:

Should $page_template take precedence if it's found, ahead of $primary_templates?

Good question. I spoke about this in person with hnla a few weeks ago, and we thought if someone had set this option -- and it didn't appear to work -- they would then hopefully figure out they needed to change the template's actual filename to get BP to use it.

Then if we add this patch so we use the page template selection, we're using *that* instead of the renamed page template. If someone's forgotten that they ever picked an option from the dropdown, 2.2 would appear to "break" their site.

If consensus is that we think this is unlikely, it's trivial to change the patch, of course, but the idea made sense when Hugo and I spoke.

[imath]

Just tested it, and i love it. Great work DJPaul.

I'm just adding a '--no-prefix' version of your patch so that it's easier to test ;)

[hnla]

This is testing pretty well for me too.

For additional clarity as to the purpose of this change there is a use case where non technical users will visit these BP 'pages' and being familiar with how WP handles pages do make the understandable assumption that they can make use of listed templates in the dropdown, now we explain that this doesn't work and to overload / copy files to a new folder etc etc, however this can be a step too far or further than those users want to take or understand. This change thus will serve two purposes it will make it super easy for users to simply modify the BP screens and bring BP further into line with WP functionality, BP is no longer by-passing something expected and provided by WP but now works with template selection.

Yes we do have a minor issue that overloading to /buddypress/buddypress.php and having a template selection other than default selected can cause theme clashes but as Paul suggested and we discussed this is perhaps less of an issue than it seems as if this issue arises it arises as someone has taken a deliberate action to overload templates thus we assume they have a degree of knowledge above a base user level and will figure out and be able to deal with the problem without undue inconvenience.

Suggestion: Rather than have broken templates could we perhaps check whether a template selection other than 'default' has been made and bail out of our $primary_templates - I'm assuming - as I've never really thought about it - that the selection 'Default Template' is in fact a WP built in template type so reliable to test against as the selection in operation.

[boonebgorges]

Just wanted to say that this rules. +1

[hnla]

This isn't necessarily a proper approach but testing quickly reworked things to:

if ( $template_file ) :
 $primary_templates = array();
else:
 $primary_templates = array(
  'plugin-buddypress.php',
  'buddypress.php',
  'community.php',
  'generic.php',
 );
endif;

get_page_template() returns an empty string if the selection is the WP default template.

In my test I have page full width selected for the bp pages under 2014 I also have /buddypress/buddypress.php setup resulting in a clash where WP adds the template name to the body class. I've then simply checked if 'get_page_template_slug' returns a value and set our $primary_templates array to empty (not suggesting that's a correct approach ) I now display the named page.php template until I deselect the template and let BP run the full template set and my buddypress.php is found and used.

[DJPaul]

hnla, I don't understand what you're trying to say. Can you try to restate what steps I need to take to recreate the problem, and what the expected output it, and what the output actually is?

[hnla]

@DjPaul need to play around further, but essentially this issue was having in 2014 /buddypress/buddypress.php created, then in a given component page e.g 'members' having a template selection made 'full width template' when I navigate to members directory 2014 has added the body class for it's full width template and we have a clash of styles breaking the page.

My initial thought was take a view that if a template selection was made, lets prevent the template stack for the BP templates from running.

[DJPaul]

Oh, you're saying that WordPress adds the selected Page Template to the <body class= all the time, even if BuddyPress falls back to using another template in the stack. :)

[hnla]

@DjPaul Yes, however it could be deemed somewhat an edge case and one more likely to arise in a testing dev environment? I thought though, if this could arise i.e a BP template did exist, yet user didn't understand that it did, went and set a named template they would potentially have an absolute car crash of a layout, they might not be able to associate their action to this issue and simply consider their site broken - my initial solution was to say 'if a named template found i.e not wp-default then don't proceed and merge those bp $primary_templates'. I don't think this is ideal though which is why I suggested it wasn't really a tested viable approach to take, if even necessary at all.

[DJPaul]

I can see arguments either way. If we can decide what we think is best over the next week or two, I'll update the patch.

[DJPaul]

After the feedback, and with consideration, I'm going to suggest we be bold and override the BP template stack if a WP page template is set. I'll update the patch and get it in.

[johnjamesjacoby]

Replying to DJPaul:

After the feedback, and with consideration, I'm going to suggest we be bold and override the BP template stack if a WP page template is set. I'll update the patch and get it in.

Can you test this using a theme that already uses page templates for things like full-page layouts?

[djpaul]

In 9306:

If a component's mapped Page's template is set in wp-admin, use that instead of BP's default template stack.

This allows people to override which page template BuddyPress injects its content into.

While this was previously possible by creating a specially-named template file in the theme, the technique was a little obscure. This alternate solution is user friendly and allows different container page templates on a per-component basis.

Fixes #6065

[johnjamesjacoby]

Reopening:

This needs checks to make sure the file that get_page_template_slug() looks for actually exists. If it doesn't, you end up with an empty $templates array and a white-screen.

To reproduce:

• Activate a theme with a page template.
• Set the activity directory to a page template.
• Visit activity directory and it's working as intended.
• Change to a different theme without page templates.
• Visit activity directory and see a white screen.
• Edit the activity directory page, and you cannot unset the page template, because the theme doesn't have any to make the UI visible (likely a WordPress core bug here, too?)

[johnjamesjacoby]

​6065.02.patch takes a slightly different approach, and fixes the above bug:

• Use bp_is_directory() for checking whether or not the foreach() is necessary.
• Introduce bp_theme_compat_page_templates() for handling the page-template look-up.
• Rather than override the template hierarchy, append it to the front so it's the first to be found.
• This allows missing page-templates to fallback to the default template hierarchy.

[johnjamesjacoby]

In 9389:

Introduce bp_theme_compat_page_templates() for determining if a directory page template is available as an override for the theme compatibility template hierarchy.

This fixes a fatal PHP error when the results of get_page_template_slug() do not match an available template for the currently active parent or child themes, causing that directory page to have no fallback template (and consequently template_include to receive an empty array.)

Fixes #6065.

[johnjamesjacoby]

The bp_is_directory() check added in r9389 prevents page templates from working on activation and registration pages.

[johnjamesjacoby]

From dev-chat today, the original intent of this feature was to allow for a "component wrapper template" style approach, where single objects funnel down into the page-template set by their directory page.

I'm afraid of what this means for things like the template hierarchy, where our directory_template_hierarchy() methods behave differently from our single_template_hierarchy() methods, and also work differently than any WordPress core template hierarchy.

This isn't to say it's a bad idea, it's just new, foreign, and not openly discussed in this ticket. It also commits us to using page-templates, which somewhat locks us in to using pages and makes moving to rewrite rules much more difficult.

I think we have a few options:

• Leave it as a filter like it is now, and remove the bp_is_directory() checks. If we go this route, I'd like to not announce it as a feature publicly (knowing we will likely either deprecate it later.) This seems kind of wasteful and half-baked to me.
• Leave the filter unhooked so it's not actually active in core for people to get used to, then we can silently drop support for something we never actively supported in the first place. This seems like we are punishing the people that care enough to look for the hidden gems.
• Revert, and put this into an experimental plugin instead. There's been a fair amount of work put into this so far, so putting it in a plugin allows us to gauge popularity and doesn't commit us to supporting anything until we consider all angles. This seems like the approach that's least likely to cause problems later, but full disclosure -- I don't see much value in this as a feature myself anyways, so I'm less likely to root for it.

Would like to get opinions from the leads before taking any further action.

[imath]

Thanks jjj for listing the different alternatives.

As you said, DJPaul & hnla and you worked hard on this feature, so i find it a bit "frustrating" to not have it in 2.2. On the other hand, the rewrite rules perspective introduces some confusion as you described in your latest comment.

So i think we shouldn't go in the first and second option route as i find it too bad to be "quite" about this feature that can be really expected by some users. I'm not a complete fan of the plugin route, in this case you just need to publish a snippet users can add in their bp-custom.php file. But it made me think to a 4th option, i'd like to suggest.

This is a "theme" feature, so we could put the code into the bp-legacy theme. It seems to me less "set in stone" that way, and just like theme, template packs can be changed...

So i suggest 6065.03.patch.

About the directory/single items pages. I think we should restrict by default to the directory. If you test the feature in twentytwelve with the full page width template, you'll see that the directory is displayed the right way, but not the single item (a blank sidebar at the right). So i guess something is wrong somewhere with single items (probably the body class for twentytwelve). So i've introduced a filter to let people eventually allow page template for single items..

[mercime]

I think this is a great idea, it provides new users a simple but limited control over the layout of BP pages per directory. Preference is that this new feature should not override the default BP template hierarchy which is in place and already used by more advanced users/theme developers.

Edit - Didn't see your post, imath. Serves me right for taking too long to write :)

[hnla]

Yes given the extended discussion and issues/points raised it seems we can't in all honesty consider this for 2.2, I feel I want to spend more time exploring aspects of it.

I agree options one and two do not sit well, I'm with imath on option three not being a great fan of plugin route, but the proposed 4th option is interesting, I like supporting something in bp-functions.php although I can see possible contra arguments to this aspect not being handled in core.

This process always was to simply find a way to confuse novice non coders less, our template hierarchy is great and serves us pretty well but for intermediate to advanced levels. We were concerned with user creates page or visits created BP page attempts to select one of their themes named templates - it doesn't work, my site is broken help!.

The single item screen is an issue, members set as a page and with the template for full width selected in twentytwelve does load the correct template both for directory and for single item however we are knocking out the body class template tokens in our single screens, which are the means to re-factoring styles based on the class 'full-width'.

[johnjamesjacoby]

In 9399:

Move page-template support into BP Legacy template pack. Props imath. See #6065.

[johnjamesjacoby]

Closing this up. Thanks everyone for all your help!