Skip to:
Content

BuddyPress.org

Opened 12 years ago

Closed 12 years ago

#4546 closed enhancement (no action required)

Use locate_template() in bp_locate_template()

Reported by: r-a-y's profile r-a-y Owned by:
Milestone: 1.7 Priority: normal
Severity: normal Version: 1.7
Component: Core Keywords:
Cc:

Description

Currently in bp_locate_template(), we check for child and parent theme templates by looking up the directory with get_stylesheet_directory() and get_template_directory().

We can just use locate_template() to do this.

In the attached patch, I've also added two filters - 'bp_locate_fallback_template' and 'bp_locate_template'. The 'bp_locate_fallback_template' hook is necessary for 3rd-party plugin devs, while the 'bp_locate_template' will open up the possibility for plugin devs to override any core template from their plugin.

I'm open to removing the latter hook.

Let me know what you guys think.

Attachments (1)

4546.01.patch (3.3 KB) - added by r-a-y 12 years ago.

Download all attachments as: .zip

Change History (7)

@r-a-y
12 years ago

#1 @johnjamesjacoby
12 years ago

  • Keywords 2nd-opinion reporter-feedback added

I don't think the extra hook at the end is necessary, as hooks already exist to add additional templates in the stack. I'd rather those hooks were used than having everything get funneled into this one, at least until we see how theme compat works.

It's possible I'm alone in this, but I like the simplicity of the original code here. Being able to see the stack of child, parent, theme-compat reads easier than using locate_template(). Also, if the functionality of locate_template() ever changes in WordPress core, we're obligated to embrace that change forever, rather than on our own schedule.

Adding 2nd-opinion tag to this, but for right now I'm a nay.

#2 @r-a-y
12 years ago

  • Keywords reporter-feedback removed

I started looking at the theme-compat code this morning so I'm still a little green! :)

I don't think the extra hook at the end is necessary, as hooks already exist to add additional templates in the stack.

Yeah, I wasn't really sure about the last hook as well. But, I think the first hook - bp_locate_fallback_template - is valid as I currently don't see a way for 3rd-party plugin devs to use bp_get_template_part() in their own templates without it failing in bp_locate_template(). Unless I'm not looking in the right place?

Being able to see the stack of child, parent, theme-compat reads easier than using locate_template().

I don't really mind which way we go here. If we want to utilize WP code as much as possible, then we should use locate_template(), however I do see your point about having to adapt to WP if they decide to change that function.

#3 @boonebgorges
12 years ago

I don't care much one way or the other, but I say that we go with locate_template(). The only case in which I can imagine WP changing how locate_template() works is if they changed the way child theming worked (like to allow infinite child themes), in which case we'd have to make the changes ourselves anyway.

Agreed that the last filter is probably unnecessary.

I like 'bp_locate_fallback_template' though. Makes sense to have a way for plugins to filter BP-specific templates only.

#4 @r-a-y
12 years ago

(In [6394]) Theme Compat:

  • Introduce 'bp_locate_fallback_template' filter so 3rd-party plugin devs can load custom templates for their components if desired.
  • See #4546.

#5 @r-a-y
12 years ago

  • Milestone changed from Awaiting Review to 1.7

I've added in the 'bp_locate_fallback_template' filter in r6394, but I've left this ticket open to decide if we want to use locate_template() in the future.

#6 @r-a-y
12 years ago

  • Keywords has-patch 2nd-opinion removed
  • Resolution set to invalid
  • Status changed from new to closed

The template stack approach renders this ticket moot. See r6537, #4656.

Note: See TracTickets for help on using tickets.