Skip to:
Content

Opened 17 months ago

Closed 14 months ago

Last modified 14 months ago

#4656 closed enhancement (fixed)

Theme Compat: Port over new bbPress template stack functionality

Reported by: r-a-y Owned by: 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 17 months ago.
First patch - bbPress port of template stack functionality
4656-port+fix.01.patch (7.8 KB) - added by r-a-y 17 months ago.
Second patch - bbPress port + bp_locate_template() fix
bp_locate_template.01.patch (2.1 KB) - added by r-a-y 17 months ago.

Download all attachments as: .zip

Change History (21)

r-a-y17 months ago

First patch - bbPress port of template stack functionality

comment:1 r-a-y17 months ago

  • Description modified (diff)

r-a-y17 months ago

Second patch - bbPress port + bp_locate_template() fix

comment:2 DJPaul17 months 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.

comment:3 r-a-y17 months 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.

comment:4 johnjamesjacoby17 months 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.

comment:5 follow-up: r-a-y17 months 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.

comment:6 in reply to: ↑ 5 johnjamesjacoby16 months 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.

comment:7 johnjamesjacoby16 months ago

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

comment:8 r-a-y16 months 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.

comment:9 johnjamesjacoby16 months 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?

comment:10 johnjamesjacoby16 months 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.

comment:11 johnjamesjacoby16 months ago

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

comment:12 r-a-y16 months 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.

comment:13 DJPaul15 months ago

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

comment:14 r-a-y15 months 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.

comment:15 mercime15 months ago

  • Cc mercijavier@… added

comment:16 johnjamesjacoby15 months ago

  • Owner changed from r-a-y to johnjamesjacoby

Meeee!

comment:17 johnjamesjacoby14 months ago

(In [6781]) Theme Compatibility:

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

comment:18 johnjamesjacoby14 months 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 14 months ago by johnjamesjacoby (previous) (diff)
Note: See TracTickets for help on using tickets.