Skip to:
Content

#4639 closed enhancement (fixed)

Add template hierarchy support

Reported by: DJPaul Owned by: boonebgorges
Milestone: 1.8 Priority: normal
Severity: normal Version: 1.7
Component: Theme Keywords: has-patch
Cc: mercijavier@…

Description

Patch introduces a basic template hierarchy. Written based on feedback from theme developers around 1.7's theme compatibility, and how they want to use it.

WordPress' template hierarchy allows a default template to be overridden based on the name of a page template, or category ID, or taxonomy slug, and so on. BuddyPress doesn't let us do this. For example, if you wanted to change the group-header.php template for a specific group, you need to put a large conditional in the template. This is not ideal because the template files become bigger, more complicated for designers to modify, and because it doesn't match the WordPress way.

Some technical notes:

  • I've commented out the bp_add_template_locations filter, because it was misbehaving.
  • I've changed the order of items in bp_get_template_locations, because we are matching files in the BuddyPress directory inside bp-legacy before overrides in the child/parent themes. This probably merits being patched separately.

What does this patch let you do?

  • User IDs can be suffixed to member templates.
  • Group IDs can be suffixed to group templates.
  • Group status types (private, public, protected, …) can be suffixed to group templates.
  • Activity types (new_blog, new_blog_comment) can be suffixed to activity templates that are used INSIDE the activity loop.

Some examples:

  • members/single/home-1.php has a higher priority (essentially) than members/single/home.php for User ID = 1.
  • groups/single/members-private.php has a higher priority than groups/single/members.php for Private groups.
  • activity/entry-new-blog-comment.php has a higher priority than activity/entry.php for activity entries that are of the "new_blog_comment" type.

About the activity hierarchy: we're overriding templates used inside the activity loop because I see the situation as vaguely analogous to post format templates in WordPress. We're not overriding templates used in the groups loop (for example) because 1) there are many types of activity, 2) only 3 types of groups, and 3) groups has many more templates which are presented very differently from activity.

Feedback welcome.

Attachments (8)

4639.001.patch (3.7 KB) - added by DJPaul 18 months ago.
4639-ray.01.patch (1.6 KB) - added by r-a-y 18 months ago.
4639-ray.02.patch​ (1.6 KB) - added by DJPaul 18 months ago.
4639-ray.03.patch (6.9 KB) - added by r-a-y 18 months ago.
4639-ray.04.patch (6.8 KB) - added by DJPaul 18 months ago.
4639-alt.01.patch (3.2 KB) - added by DJPaul 17 months ago.
4639-refresh.01.patch (16.3 KB) - added by r-a-y 11 months ago.
Updated patch to use sanitize_file_name() where appropriate
4639-refresh.02.patch (16.8 KB) - added by r-a-y 10 months ago.

Download all attachments as: .zip

Change History (61)

DJPaul18 months ago

comment:1 karmatosed18 months ago

I really like the idea of the post format inspired format. Whilst you can do CSS changes it's nice to have this granular control. I know for me it's one of the common things requested to show different styling for different types of activity.

comment:2 rogercoathup18 months ago

That's quick work Paul.

Does the patch address specifying a different 'outer' template based on content type. e.g. the 'outer' template that's used for all single groups? (and similarly for all single profiles). i.e. so that the existing (1.7) template parts for groups can be 'injected' into a different outer template (other than the buddypress.php, page.php, etc. chain). That was more the initial problem I was trying to solve.

Does that make sense, or should I add further example?

comment:3 mercime18 months ago

  • Cc mercijavier@… added

comment:4 modemlooper18 months ago

add_filter('this_patch', 'on fire!')

r-a-y18 months ago

comment:5 follow-ups: r-a-y18 months ago

Nice first patch, Paul!

My patch follows a little closer to how bbPress does things as I outlined here.

It works with the 'bp_add_template_locations' filter so you can do things like this:

/wp-content/themes/twentytwelve/buddypress/members/single/home-{user_id}.php
/wp-content/themes/twentytwelve/community/members/single/home-{user_id}.php
/wp-content/themes/twentytwelve/members/single/home-{user_id}.php

The attached patch only works for the users component as an example. You can also do /members/single/home-{username}.php as well. And Roger, if you use one of these templates, it will become the outer template.

Let me know what you guys think.

comment:6 in reply to: ↑ 5 DJPaul18 months ago

Nice patch, r-a-y. I think I like splitting up the logic for each component.

if you use one of these templates, it will become the outer template.

You are changing the overall page template file to a template specifically designed for use as a template part. BP-Default would work fine, but BP-Legacy/turtleshell's members/single/home.php is intentionally not a full page template.
This approach does not look like it would work for templates included within home.php (for example).

I know your patch was another iteration, but I noticed that the member slugs haven't been sanitised for use in a file name; for example, special characters, or people trying to feed in relative paths, and so on. I have some @todos in my patch around this.

comment:7 follow-up: DJPaul18 months ago

Having woken up properly now, I've just had a proper test of Ray's patch. It works, and adds hierarchy to the main templates that we load for each section, e.g. members/single/home.php. It replaces the entire template, and I now understand this is desirable, and it allows Roger to do the type of thing that he suggested. It also highlighted a bug in my patch, which also supported a custom home.php, but rather than replacing the entire file, it overrided a template part rather than the whole template.

For these main templates (need a better name), we should complete Ray's patch, and get it into core, as a first pass. These are loaded via bp_core_load_template() calls in core, and I'll make a list of them for reference.

A second step can be to introduce a hierarchy for templates included inside one of the main templates, giving people more granular control over specific template parts rather than the entire page template.

This is going to require a really comprehensive codex page to document :)

DJPaul18 months ago

comment:8 DJPaul18 months ago

Ray, I've 4639-ray.02.patch is a tweak of your patch. I've adjusted the phpdoc a little, switched to use the user login rather than the display name to prevent people renaming their account and ending up with specific templates intended for other people, and used sanitize_file_name() to prevent any mischief.

comment:9 DJPaul18 months ago

Here's a list of the main templates

'activity/index'

'blogs/index'
'blogs/create'

'registration/activate'
'registration/register'

/* Ignore these
'forums/single/forum'
'forums/single/topic'
'forums/index'
*/

'members/index'
'members/single/activity/permalink'
'members/single/home'
'members/single/settings/capabilities'
'members/single/settings/delete-account'
'members/single/settings/general'
'members/single/settings/notifications'

'groups/create'
'groups/index'
'groups/single/home'
'groups/single/plugins'

comment:10 in reply to: ↑ 7 mercime18 months ago

Replying to DJPaul:

This is going to require a really comprehensive codex page to document :)

Volunteering to help out in Codex :-) Thinking of setting it up in digestible tidbits so that it'll be easier to digest/understand. Or whatever setup you think is better :)

I'm really liking what's being set up here. Thanks, you guys.

comment:11 hnla18 months ago

So the 4639-ray.02.patch file is the current required file for implementation.

By the sound of this initial pass it will be possible to achieve all the flexibility devs will need, this was always a hugely important aspect of this whole BP iteration and I'm pleasantly surprised at what has been achieved in a very short space of time on this, well done Paul & r-a-y.

I'll merge the patch and start to have a good play around and see how things feel Saturday morning and the more feedback and testing the better on this one.

comment:12 r-a-y18 months ago

Thanks Paul for adding all the fun stuff like sanitization and switching to user_login. Do you think we should switch to user_nicename?

Glad you thought the approach was okay!

As for:

A second step can be to introduce a hierarchy for templates included inside one of the main templates, giving people more granular control over specific template parts rather than the entire page template.

This will be nice to have as well. I think we should nail down a filenaming convention before we start this second step because as you said, this can get confusing fast!

comment:13 hnla18 months ago

As nice as it would be to have granularity is it so vital where inner templates are concerned? If I have been able to do, say, home-special-group.php and navigating now to /groups/special-group/ loads the new template in that I can simply call new template parts via locate_template perhaps neatly squirrelled away in /special-group-templates/.

comment:14 karmatosed18 months ago

I actually like having inner template granularity. It's very useful from a theme designer view point.

comment:15 r-a-y18 months ago

  • Keywords dev-feedback added

Paul, ran into a problem.

bp_template_include_theme_supports() is hooked to "template_include", which is too early to do any type of BP component $x_template global checking like activity types, group info, etc.

bbPress works because it uses custom post types and the $post global is already populated by then, but BP populates its component globals way later in "template_redirect".

So, right now, we are limited to user checks, which works for the members component.

A workaround for groups is to check the $bp->unfiltered_uri combined with action checks to add a group slug check. For activity permalink, perhaps only do user checks and omit activity types.

Need some dev feedback before I proceed.

comment:16 boonebgorges18 months ago

Are we talking about directories or single item pages?

In the case of single-item pages (like groups), we should know by bp_setup_globals which group we're looking at. We don't have to wait for any bp_has_groups().

In the case of directories, it's true that the $x_template globals are important. But in that case, it would presumably be only for the template parts related to displaying a single item in the directory that we would need to know what group we're looking at. So,

if ( bp_has_groups() ) {
    while ( bp_groups() ) {
        bp_the_group();
        bp_locate_template( 'group' ); // or whatever
    }
}

And by this point, we *do* have access to the template global.

Or maybe I'm misunderstanding?

comment:17 boonebgorges18 months ago

(If I *am* misunderstanding, I am very wary of doing some sort of hack with the unfiltered_uri that involves duplicating logic that exists elsewhere in the application. I'd rather rework BP's load order so that it works more like WP's custom post type queries. This will be hard, of course.)

comment:18 DJPaul18 months ago

Ray, check the buddypress()->groups->current_group (for example). I did this in my patch because we're not in the template loop.

comment:19 r-a-y18 months ago

You're right about groups, Boone and Paul. Don't know how I missed that!

While I was working up the patch for the activity permalink, I thought we wanted to do something like /members/single/activity/permalink-{activity_type}.php.

This isn't possible in the $bp global.

comment:20 follow-up: boonebgorges18 months ago

Ah yes, you're right about activity permalinks.

Looking over the way we handle permalinks, it doesn't seem to me to be out of the question to rework the activity loading process so that BP_Activity_Component::setup_globals() does most of the work that's currently being done in bp_activity_screen_single_activity_permalink(). This would help with template hierarchy, and it would also more closely match the way that the members and group components work.

I'm agnostic about whether this should be done for 1.7. IMO it's not out of the question to release 1.7 without activity/permalink-{activity_type}, but then to refactor activity as described above for 1.8 and introduce activity-type-specific templates as an enhancement then.

r-a-y18 months ago

comment:21 r-a-y18 months ago

Paul, in 4639-ray.03.patch, I've changed the location of the custom templates as well as the filenames to be more granular.

Here are some examples:

members/single/custom/nicename-{user_nicename}.php
members/single/custom/id-{user_id}.php
members/single/custom/component-{component}.php

First, I've moved custom templates under a subdirectory called "custom". This is so you can easily keep track and tell what overrides you've made in your theme.

The second change is the prefixing of the template with the parameter. The reason for this change is previously it was possible for a user to have a user_nicename of "1" but have a different user ID altogether. This would mean that a user with user ID "1" would share the same template with user_nicename "1". This is a no-no and I'm surprised that this isn't really addressed in WordPress' own template hierarchies!

I've also added in a component check for the members component because some theme devs might want to style certain components differently (such as messages or settings).

Let me know what you think.

---

IMO it's not out of the question to release 1.7 without activity/permalink-{activity_type}, but then to refactor activity as described above for 1.8 and introduce activity-type-specific templates as an enhancement then.

I feel the same way; it would be nice to address this in 1.7, but if we don't have time, then 1.8 is fine with me as well.

Last edited 18 months ago by r-a-y (previous) (diff)

comment:22 in reply to: ↑ 20 DJPaul18 months ago

Replying to boonebgorges:

IMO it's not out of the question to release 1.7 without activity/permalink-{activity_type}, but then to refactor activity as described above for 1.8

Agree

comment:23 hnla18 months ago

Quick observations: (disclaimer: hurried tests & may have missed some points)

The process seems to work well creating /single/custom/component-profile.php loads the file for user profile screens, copying all of home.php into the file (that seems a little like overkill but a very minor point )

However what I still don't see - and really the intial point of this exercise - is how one overloads for all user account screens, we have drilled down past this point.

If I wanted to have a unique layout for user account screens in general I'm still stuck?

Is it not possible to work something along the lines of /single/custom/home-custom.php and using something like bp_is_user() to id this action (realise the naming is a possibly problematic area and if this possible would have to be a fixed name)

comment:24 r-a-y18 months ago

Is it not possible to work something along the lines of /single/custom/home-custom.php and using something like bp_is_user() to id this action (realise the naming is a possibly problematic area and if this possible would have to be a fixed name)

You're right. Should be possible by just adding an extra check like:

		$templates = array(
			'members/single/custom/nicename-'  . $user_nicename . '.php',
			'members/single/custom/id-'        . $user_id       . '.php',
			'members/single/custom/component-' . $component     . '.php',
			'members/single/custom/home.php
		);

The filenaming and convention is still up in the air.

Last edited 18 months ago by r-a-y (previous) (diff)

comment:25 hnla18 months ago

Of course, simpler than I was thinking it.

Tests fine. So I now have /single/custom/component-profile.php and /single/custom/home.php giving me control over all user account screens and then the profile screens.

So this essentially is great and affords the control required, if required.

comment:26 johnjamesjacoby18 months ago

  • Keywords commit added; dev-feedback removed
  • Milestone changed from Awaiting Review to 1.7

I like it. Works for me. Would like to see single activity permalink pages in the /activity/single/ folder.

comment:27 hnla18 months ago

Perhaps the naming of the directory ought to reflect what is being overloaded for added clarity i.e something along the lines of /home-custom/ or perhaps the files should be prefixed with 'home-nicename-', 'home-component-' ?

comment:28 rogercoathup18 months ago

for the future -- but see this developing nicely as member profile handling integrates with WP roles, and supporting different profile templates based on role

DJPaul18 months ago

comment:29 DJPaul18 months ago

Ray: I've added 4639-ray.04.patch​ which I believe includes the results of the conversation we had on IRC about the approach the other night. If you could confirm this, and sanity check it before commit, that would be wonderful.

comment:30 follow-up: hnla18 months ago

@DJPaul Paul is there a reason you have left out the earlier suggestion about adding a 'members/single/custom/home.php' from your latest revision, which is working for me nicely and is required in some form or another.

comment:31 in reply to: ↑ 30 ; follow-up: DJPaul18 months ago

Replying to hnla:

@DJPaul Paul is there a reason you have left out the earlier suggestion about adding a 'members/single/custom/home.php' from your latest revision, which is working for me nicely and is required in some form or another.

You can override home.php with a file of the same name already; we do not need to specify it explicitly (in this patch). We've removed the rather arbitrary "custom" directory to keep it matching the structure of the BP-Default/theme compat. template file.

comment:32 in reply to: ↑ 31 hnla18 months ago

Replying to DJPaul:

Replying to hnla:

@DJPaul Paul is there a reason you have left out the earlier suggestion about adding a 'members/single/custom/home.php' from your latest revision, which is working for me nicely and is required in some form or another.

You can override home.php with a file of the same name already; we do not need to specify it explicitly (in this patch). We've removed the rather arbitrary "custom" directory to keep it matching the structure of the BP-Default/theme compat. template file.

Paul I'm not seeing how then I override home.php to control the outer template, I already have overloaded all the bp template files and home.php is injected into a parent file be it page.php or buddypress.php etc but I don't see how I can change that, could with the template array having /custom/home.php but without that we have odd situation where I can ask that component-profile.php be used and add to that what ever I need but not able to do that for all user account screens in one hit.

Can you clarify what I'm misunderstanding in this process?

comment:33 in reply to: ↑ 5 ; follow-up: rogercoathup18 months ago

Replying to r-a-y:

The attached patch only works for the users component as an example. You can also do /members/single/home-{username}.php as well. And Roger, if you use one of these templates, it will become the outer template.

Let me know what you guys think.

So, to clarify on template hierarchy going forward - am I correct in thinking:

If I implement a home.php in my-theme/buddypress/members/single, it will be used as a template part and injected in to page.php (or buddypress.php, etc.)

If I implement a home.php in my-theme/members/single, it will not be injected into page.php, and will therefore have to be a full outer template including its own header, footer, sidebar, etc.

Is that correct?

If so, I guess if I implement both, the one in /my-theme/members/single takes precedence?

If I'm not correct -- how does it work :)

comment:34 in reply to: ↑ 33 DJPaul18 months ago

I was about to commit the latest patch, but spotted some issues.

These templates aren't supported correctly (currently it will try to use the members/single/home path):

'members/single/settings/capabilities'
'members/single/settings/delete-account'
'members/single/settings/general'
'members/single/settings/notifications'

This templates isn't supported correctly (currently it will try to use the groups/single/home path):

'groups/single/plugins'

comment:35 follow-up: r-a-y18 months ago

The settings templates are actually named that way because of a bug. See #3586.

I'm opposed to adding hierarchy for these when devs can use the more, flexible:
/members/single/home-settings.php (when on the settings component and when the patch is committed)

Chances are you'll want to style all the settings pages with the same outer template.


Paul: With what we talked about last week, I spent some time testing everything again (except group extensions).

I'm going to outline the problem noted by Roger above.

Problem

I like theme compat, but want to override a template loaded bp_core_load_template().

Let's say I like theme compat and want to continue using theme compat, but want to merely add or change the markup to a template loaded by bp_core_load_template() (such as /members/single/home.php).

A) If I move /members/single/home.php from bp-legacy over to my current theme (wp-content/themes/twentytwelve/members/single/home.php), bp_core_load_template() is loading this template before theme compat is executed:
https://buddypress.trac.wordpress.org/browser/trunk/bp-core/bp-core-catchuri.php#L379
https://buddypress.trac.wordpress.org/browser/trunk/bp-core/bp-core-catchuri.php#L398

Therefore, I won't see get_header(), get_sidebar(), etc, because bp-legacy's home.php is just a template part. It won't load the skeleton template from page.php (or any of the templates from bp_get_theme_compat_templates()).

The problem is we're referring to home.php in two different use-cases. Both as a regular page template and a template part.

B) If I move /members/single/home.php to my theme's subdirectory like /buddypress/members/single/home.php or /community/members/single/home.php, this works as expected.

It's quite confusing and we're the implementors!

Proposed solution

For template hierarchy, we should use index.php (or page.php, etc.) as our marker instead of home.php.

Then, the hierarchy would look like this:

$templates = array(
	'members/single/index-' . $user_nicename . '.php',
	'members/single/index-' . $user_id       . '.php',
	'members/single/index-' . $component     . '.php',
	'members/single/index.php'
);

This solves all problems with home.php disambiguation.

As a reference point, we've already renamed templates in theme compat before; the registration templates in theme compat were renamed from /registration/register.php to /members/register.php.

Last edited 18 months ago by r-a-y (previous) (diff)

comment:36 hnla18 months ago

This is confusing, but partly as clarity is perhaps not forthcomming on the way devs are meant to approach templating moving forward.

I'm confused by the fact that we backtracked on the notion of 'members/single/index.php' or 'members/single/home.php' as it was initially proposed and that I tried with success.

The suggestion that it wasn't necessary due to being able to do my-theme/members/single/home.php was - fundamentally - true; however ray you suggest that that would load without get_header()/footer/sidebar is because you copied home.php from 'legacy' so why didn't you simply add those get_template parts - my test was with home.php under my-theme/members/single as already existed i.e with all those parts and things seemed fine, unless I overlooked something - more than possible.

So Paul & ray it's unclear what is the proposed approach to templating on the one hand we start out in my-theme/buddypress/members/single/custom-files.php then suggest that we would also be running /my-theme/members/single/home.php to override home.php thus a confusion of BP files in various places.

Given the choice I would be wanting/expecting to simply have all BP templates in the theme under /buddypress/ and definitely would not want a mixture.

So the above proposed solution from ray, makes sense to me although 'index' sounds plausible, but then I'm not really sure why we consider home.php to be in a state of disambiguation, as does not the original proposal of a /custom/ dir solve that? why is that such a bad approach?

comment:37 karmatosed18 months ago

My personal preference would be to not have a home.php or at least only have when a index.php and home.php are needed. Keeping to index.php keeps it nice and simple and I think that's a really important thing to do in avoiding overcomplicating here.

comment:39 DJPaul17 months ago

Yes, you can move theme compatibility (TC) template parts into the theme to override them.
If you move a TC home.php into the theme, yes, it won't work; you need a regular WordPress template file.

I am sure it's not a good idea to consider renaming template files for this specific use case. That discussion should come in its own ticket, and in a future release.

We don't need to add a hierarchy for TC template parts; we need to add a hierarchy for our regular WordPress templates.

comment:40 hnla17 months ago

Nice codex page Paul.

Not sure exactly what you mean by the 'hierarchy for TC template Parts' I thought it was yourself that suggested those? but would agree not really needed at this juncture and even if so I would think it's within the scope of most devs to be able to fashion an approach.

as far as I can see the codex page and the detail you flesh out there is what was required?

As an example I would expect my process always be to copy al bp-legacy files to /my-theme/buddypress/members/single/etc but the naming is still an issue? Is not what Ray is saying that home.php provides issues thus renaming to index for custom use or dropping back to the idea of an additional dir /single/custom/ to provide separation/distinction.

what I definitely want to avoid is having my-theme/bussypress/single/etc working for all BP template files but then having to do my-theme/members/single/home.php simply to overload one file and ending up with a messy theme directory.

DJPaul17 months ago

comment:41 johnjamesjacoby17 months ago

  • Milestone changed from 1.7 to Future Release

Re: 4639-alt.01.patch, 'bp_located_template' is the wrong filter to use here; I was against having that filter in the first place, I think r-a-y mentioned reverting it, then didn't. It's not the correct place to be making template decisions, nor should we be setting the example that it's okay to do so.

Attached patches are confusing at best, with scattered intentions, ideas, and initiatives laced through-out. I don't see anything near final in any of these patches, and this is far outside the scope of what we decided for 1.7.

Best I can tell, what needs finishing up are:

  • Single activity stream permalinks working and in /activity/single/
  • Groups Component API support

Anything else should get punted to 1.8. 1.7 was supposed to be short and sweet, but we've lingered for all of October.

Those two things should probably have their own tickets. Moving this one to Future Release until we have a 1.8 milestone.

comment:42 in reply to: ↑ 35 ; follow-up: rogercoathup17 months ago

Replying to r-a-y:

The problem is we're referring to home.php in two different use-cases. Both as a regular page template and a template part.

For template hierarchy, we should use index.php (or page.php, etc.) as our marker instead of home.php.

I agree it's best to implement with separate names (index.php for the outer, with home.php for the inner template part).

@johnjamesjacoby - if you are canning it for 1.7, can you explain how one can best organise code / themes files to create sites that utilise template parts, but don't want to inject all BP related content into the same outer template? Where would you suggest we put the logic / code to determine the outer part?

comment:43 in reply to: ↑ 42 ; follow-up: modemlooper17 months ago

Replying to rogercoathup:

@johnjamesjacoby - if you are canning it for 1.7, can you explain how one can best organise code / themes files to create sites that utilise template parts, but don't want to inject all BP related content into the same outer template? Where would you suggest we put the logic / code to determine the outer part?

If you create a folder /activity/and place index.php in there you can style the whole page

I actually prefer this because it keeps theme folder clean.

comment:44 in reply to: ↑ 43 rogercoathup17 months ago

Replying to modemlooper:

Replying to rogercoathup:

@johnjamesjacoby - if you are canning it for 1.7, can you explain how one can best organise code / themes files to create sites that utilise template parts, but don't want to inject all BP related content into the same outer template? Where would you suggest we put the logic / code to determine the outer part?

If you create a folder /activity/and place index.php in there you can style the whole page

I actually prefer this because it keeps theme folder clean.

That's got nothing to do with the original problem; Which is about using template parts, and allowing different 'outer' templates which work in combination with those parts. [of course, we can rework the whole page, as we could in all previous versions of BP, but that's not what was wanted]

comment:45 modemlooper17 months ago

I know what this thread is about. Merely suggesting an option to customize outer pages. Anyway its not making it into 1.7 so might as well use what's available.

comment:46 boonebgorges12 months ago

  • Keywords commit removed
  • Milestone changed from Future Release to 1.8

Moving to 1.8 for further discussion.

comment:47 follow-up: r-a-y11 months ago

  • Keywords has-patch added
  • Version set to 1.7

refresh.01.patch is a slightly, different take on what I talked about here.

Technically, I've changed the hook from 'bp_get_root_template' to 'bp_get_buddypress_template' because it's a more accurate hook to use.

The hierarchy in the patch looks like this:

Member pages

  • members/single/index-action-{action}.php
  • members/single/index-component-{component}.php
  • members/single/index.php

Group pages

  • groups/single/index-action-{action}.php
  • groups/single/index-slug-{slug}.php
  • groups/single/index-status-{status}.php
  • groups/single/index.php

Group and Blog creation pages

  • groups/index-create.php
  • blogs/index-create.php

Registration and Activation pages

  • members/index-register.php
  • members/index-activate.php

All directory pages

  • members/index-directory.php
  • groups/index-directory.php
  • blogs/index-directory.php

Naming convention is still up in the air, but I've stuck with index.php as the fallback.

Notice that the creation, registration and directory pages do not have an index.php fallback. That's because index.php is already used as a template part in those directories... The index.php template part is inadequately named, but I think it's fine to omit the index.php fallback for these pages anyway.

For the member single pages, I omitted the user ID / login / nicename in the hierarchy as I don't see much of a benefit in adding them. I also added the context (such as action, component) to the filename. I'm open to removing it.

Let me know what you think.

r-a-y11 months ago

Updated patch to use sanitize_file_name() where appropriate

comment:48 boonebgorges11 months ago

I haven't been involved in much of the above, so I'll refrain from making comments on the technical implementation, other than to say that it makes sense to me and looks clean, at a glance anyway.

On the hierarchy itself:

  • You put groups/single/index-action-{action}.php ahead of groups/single/index-slug-{slug}.php. I can see why that might make sense (a single group can have many actions), but it seems unconventional to put the group-specific template anywhere but at the top of the hierarchy.
  • I personally like the idea of having ID/login/nicename-specific items in the hierarchy, both because it more closely parallels the way that the WP hierarchy works, and because I can imagine possible use cases where certain groups/profiles might require totally different markup from everyone else's. If it doesn't add too much overhead (of either the technical or cognitive variety), I'd say support should be added for them.

Thanks for plugging away at this one, r-a-y.

comment:49 in reply to: ↑ 47 hnla11 months ago

Replying to r-a-y:

refresh.01.patch is a slightly, different take on what I talked about here.

This seems to test fine for me r-a-y, feels like it covers all requirements?

I'm asuming that the need to be able to manage the template parts is simply covered by the two conditions 'component' & 'actions' in that having caught either of those two conditions (in respect of user screens) we can simply call our own copy of a template part as that process is now outside the core apps influence or concern so e.g having set index-action-public.php if wanted to then do something with the header for that action I could simply do:

bp_get_template_part( 'members/single/profile/public/member-header' )

It works and perhaps is acceptable, it's certainly fine to work with by the feel of it I would be able to manage that increase in template/folder complexity - whether it's the best approach open to debate.

However I think we are still left with a problem of drilling down to the inner parts:
/single/profile
as it in turn calls our three main item body parts 'edit', 'change avatar', 'profile-loop'

So if I wanted to get to 'profile-loop' how would I? I guess could re-direct to pick a new copy of single/profile.php in single/profile/ then in turn edit the:

bp_get_template_part( members/single/profile/profile-loop) to point to a new copy in a new folder in /profile/ but now it's starting to feel messy and confusing, although not unworkable.

Part of this whole issue was to get to these template parts but perhaps I overlook some obvious means to this end or for that matter maybe I'm just missing a glaring point in all this :)

As for the prefix 'index' not sure what else it could be named it doesn't feel ideal but perhaps not really something to worry too much over and - although never thought it great - does follow the WP convention for 'home.php' & 'index.php'

All in all if we ended up simply rolling with your last patch r-a-y I for one would feel it a vast improvement on present capabilities and other issues manageable by the developer in question at the time.

comment:50 boonebgorges11 months ago

It would be a real shame to let this one fall through the cracks, seeing as there is a patch in place.

  • r-a-y, could you please post a brief summary of what the most recent patch does, and the problem it's meant to solve?
  • Other core devs, once r-a-y has provided this summary, can you please have a look at his technique?

From my understanding of the goals of this ticket, I think r-a-y's solution is conservative, prudent, and a good foundation for future hierarchies. But I don't want to move forward unless some of the discomfort in the thread above is resolved. Thanks!

r-a-y10 months ago

comment:51 r-a-y10 months ago

refresh.02.patch addresses Boone's points from comment:48.

I'm going to reply to some of the points that were made above:

I personally like the idea of having ID/login/nicename-specific items in the hierarchy

Instead of user ID / nicename checks, I would prefer roles as that would be more useful. But since WP doesn't check for user roles in their own template hierarchy by default, I'm not sure BP should do it either.


r-a-y, could you please post a brief summary of what the most recent patch does, and the problem it's meant to solve?

My patch deals with adding a top-level template hierarchy depending on the BuddyPress page you're on. This is similar to how WordPress handles its template hierarchy.

I've written an unpublished codex article that describes this in more depth:
http://codex.buddypress.org/?page_id=5853&preview=true (make sure you're logged in to view)


Part of this whole issue was to get to these template parts but perhaps I overlook some obvious means to this end or for that matter maybe I'm just missing a glaring point in all this :)

The patch does not cover the concept of a "template part hierarchy" as originally described in the "Some examples" section of the ticket.

We could look at adding the second parameter to bp_get_template_part() to all our template parts, but that could get messy fast.

We can probably create a new ticket to discuss template part hierarchy to clear this ticket.

Last edited 10 months ago by r-a-y (previous) (diff)

comment:52 hnla10 months ago

We can probably create a new ticket to discuss template part hierarchy to clear this ticket.

I would second that. Think clearing the ticket at this point makes sense, the results thus far greatly extend the capabilities we have and possibly in sufficient respect for all custom requirements bar the most obtuse?

This ticket is convoluted and complicated so better to draw a line under spend a little time with the templates looking at use cases to see if there actually is a need to further refine things in which case start afresh with a new ticket off the back of this one.

@r-a-y & DJPaul well done, it's a great effort.

comment:53 boonebgorges10 months ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 7212:

Introduces template hierarchy for top-level theme compat templates

The "top-level" template is the outermost template included by theme
compatibility. As of BuddyPress 1.7, a stack of template names is checked when
BP decides which top-level template to use: plugin-buddypres.php,
buddypress.php, community.php, etc. (See bp_get_theme_compat_templates().)

This changeset introduces an additional level of template hierarchy, so that
themes may provide top-level templates that are specific to the current
component, such as blogs/index-directory.php, or specific to the current item,
action, or status, such as groups/single/index-id-{$group_id}.php. In this way,
BP's top-level template hierarchy becomes more flexible for theme devs, and
more closely parallels the template hierarchy used by WordPress.

Note that this changeset only implements hierarchy for the top-level templates.
Inner template parts may receive a parallel hierarchy in a future version of
BuddyPress.

Fixes #4639

Props r-a-y

Note: See TracTickets for help on using tickets.