Skip to:
Content

BuddyPress.org

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#4656 closed enhancement (fixed)

Theme Compat: Port over new bbPress template stack functionality

Reported by: r-a-y's profile r-a-y Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 1.7 Priority: normal
Severity: major Version: 1.7
Component: Core Keywords: has-patch dev-feedback
Cc: mercijavier@…

Description (last modified by r-a-y)

r6490 introduced a fix to override template parts in BP's custom directory locations.

For example, I want to override /members/single/member-header.php and place it in /wp-content/themes/twentytwelve/buddypress/members/single/member-header.php

Before r6490, this wasn't possible.

However, JJJ noted that bbPress introduced the new method of the template stack in [BB4324], which is more flexible for developers and we should look into using that instead.


The first patch does this verbatim with a tiny change from bbPress that reverts [BB4326].


However, I've also attached a second patch, which includes the first patch plus a fix when using bp_locate_template() by itself.

If you used bp_locate_template() by itself, it would not use the custom directory locations as set in bp_get_template_locations() because only bp_get_template_part() has a filter that is hooked in by bp_add_template_locations().

Second patch removes this:

add_filter( 'bp_get_template_part', 'bp_add_template_locations' );

And hooks into the later-running filter, 'bp_get_template_stack', to add the custom directory locations.

This fixes problems when using such functions as bp_has_custom_signup_page() and bp_has_custom_activation_page(), which both rely on bp_locate_template().

If the second patch is not deemed acceptable, what we need is an approach that makes sure both bp_get_template_part() and bp_locate_template() references the same templates at all times.

Feedback welcome.

Attachments (3)

4656-port.01.patch (7.0 KB) - added by r-a-y 12 years ago.
First patch - bbPress port of template stack functionality
4656-port+fix.01.patch (7.8 KB) - added by r-a-y 12 years ago.
Second patch - bbPress port + bp_locate_template() fix
bp_locate_template.01.patch (2.1 KB) - added by r-a-y 12 years ago.

Download all attachments as: .zip

Change History (21)

@r-a-y
12 years ago

First patch - bbPress port of template stack functionality

#1 @r-a-y
12 years ago

  • Description modified (diff)

@r-a-y
12 years ago

Second patch - bbPress port + bp_locate_template() fix

#2 @DJPaul
12 years ago

The template hierarchy ticket #4639 needs to be resolved before this should be looked at. I'm struggling to understand the difference between the two, apart from this one including some sort of bbPress functionality which in turn is an implementation of a patch to WordPress core.

#3 @r-a-y
12 years ago

  • Type changed from defect (bug) to enhancement

If you revert r6490, try doing what I stated above - overriding a template part from bp-legacy into your theme's custom directory location.

It won't work.

The template stack approach in bbPress is an alternative way of fixing this problem and gives plugin / theme devs some flexiblity in adding additional directory locations.

I can see how this is related to template hierarchy, but it's not. It's a theme compat issue and more of an enhancement since r6490 fixes the issue stated at the top of the ticket.

#4 @johnjamesjacoby
12 years ago

(In [6537]) Theme Compat:

  • Literal merge of template stack code from bbPress 2.2.
  • See #4656.
  • Does not include additional bp_template_stack_add_custom_locations() function in attached patch. This needs more testing.

#5 follow-up: @r-a-y
12 years ago

JJJ, glad to see that template stack functionality is in the codebase.

However, let's say I want to override /bp-legacy/buddypress/members/single/member-header.php and place it in /wp-content/themes/twentytwelve/community/members/single/member-header.php.

r6537 does not address using custom locations in bp_get_template_locations(). It only sees the first location ('buddypress') and ignores the others.

bp_locate_template.01.patch reverses the foreach loops and addresses this problem.

#6 in reply to: ↑ 5 @johnjamesjacoby
12 years ago

Replying to r-a-y:

bp_locate_template.01.patch reverses the foreach loops and addresses this problem.

Can you confirm that this patch doesn't break the reverse case? I seem to recall it needing to be in this order for a specific reason.

#7 @johnjamesjacoby
12 years ago

  • Owner set to r-a-y
  • Status changed from new to assigned

#8 @r-a-y
12 years ago

Can you confirm that this patch doesn't break the reverse case? I seem to recall it needing to be in this order for a specific reason.

JJJ, can you give an example of the reverse case so I can do some testing?

---

Also, the second part of the ticket I listed in the ticket description needs addressing.

If you click on the "Register" link in some WP themes, it will link to:
/wp-login.php?action=register

Currently in the codebase, if you click on this link, it won't redirect to BP's registration page.

#9 @johnjamesjacoby
12 years ago

JJJ, can you give an example of the reverse case so I can do some testing?

It's related finding templates in a certain hierarchical order; unclear what exactly, but we can experiment to figure it out.

I don't see where you mention trapping the original register page in the original post. Sounds like a separate issue?

#10 @johnjamesjacoby
12 years ago

(In [6658]) Theme Compat:

  • In bp_template_include_theme_compat(), move root template check beyond reset post ation, so post gets reset even if root template is found.
  • See #4656.

#11 @johnjamesjacoby
12 years ago

(In [6667]) Introduce bp_deregister_template_stack() to allow manipulating of the template stack at run-time. See #4656.

#12 @r-a-y
12 years ago

I don't see where you mention trapping the original register page in the original post. Sounds like a separate issue?

Yeah, I guess I wasn't super clear when I wrote the original post.

It's alluded to here:

This fixes problems when using such functions as bp_has_custom_signup_page() and bp_has_custom_activation_page(), which both rely on bp_locate_template().

Since the template stack port is pretty much done, I'll create a new ticket for this.

#13 @DJPaul
12 years ago

Ray, can we close this ticket in favour of the new one you created with the patch?

#14 @r-a-y
12 years ago

Ray, can we close this ticket in favour of the new one you created with the patch?

Almost! This comment and patch I made is still valid:
https://buddypress.trac.wordpress.org/ticket/4656#comment:5

As JJJ stated, patch needs testing against other use-cases that I haven't encountered yet.

#15 @mercime
12 years ago

  • Cc mercijavier@… added

#16 @johnjamesjacoby
11 years ago

  • Owner changed from r-a-y to johnjamesjacoby

Meeee!

#17 @johnjamesjacoby
11 years ago

(In [6781]) Theme Compatibility:

  • Fix some copy-pasta.
  • See #4656.

#18 @johnjamesjacoby
11 years ago

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

(In [6780]) Theme Compatibility:

  • When locating template files, filter 'bbp_get_template_stack' rather than 'bbp_get_template_part'
  • Fixes bug where using bbp_locate_template() directly would result in missing subdirectory locations.
  • Props r-a-y.
  • Fixes #4656.
Last edited 11 years ago by johnjamesjacoby (previous) (diff)
Note: See TracTickets for help on using tickets.