Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 2 years ago

Last modified 2 years ago

#4855 closed defect (bug) (wontfix)

Method to override a plugins use of plugins.php or add filters to bp_template_title and bp_template_content

Reported by: modemlooper Owned by:
Milestone: Priority: normal
Severity: normal Version: 1.7
Component: Templates Keywords:
Cc: hugoashmore@…

Description

Yes, another plugins.php related ticket.

Filtering bp_get_template_part works unless page is calling plugins.php. bbPress uses plugins.php and if you wanted to override the page you can't call plugins.php. If you do, your content and bbPress (or whatever plugin) content displays together.

This is needed because we are no longer relying on themes to display content. You can call a different template file for that part but it breaks a theme if the plugins.php you are calling is not matching the themes plugin.php. This is mainly an issue if the theme has a custom plugins.php file or you want to display different content than what a plugin is pushing through plugins.php

What we should be able to accomplish: Even if a plugin is using plugins.php we should be able to override bp_template_title and bp_template_content. add filters to bp_template_title and bp_template_content?

Maybe there is a method and I'm missing something.

Here is an example and it seems silly to remove a template file to simply replace it.

function my_template_filter_init() {
 
add_action( 'bp_template_title', 'my_filter_template_title' );
add_action( 'bp_template_content', 'my_filter_template_content' );
add_filter( 'bp_get_template_part', 'my_template_part_filter', 10, 3 );
 
}
add_action('bp_init', 'my_template_filter_init');
 
function my_template_part_filter( $templates, $slug, $name ) {
 
if ( 'members/single/plugins' != $slug )
    return $templates;
 
    return bp_get_template_part( 'members/single/plugins' );
}
 
function my_filter_template_title() {
    echo 'Title';
}
 
function my_filter_template_content() {
    echo 'Content';
}


Attachments (2)

4855.01.patch (3.9 KB) - added by r-a-y 7 years ago.
4855.02.patch (877 bytes) - added by r-a-y 7 years ago.

Download all attachments as: .zip

Change History (37)

#1 @boonebgorges
7 years ago

  • Keywords reporter-feedback added

add filters to bp_template_title and bp_template_content?

bp_template_title and bp_template_content are just do_action() hooks. Plugins display their content there by echoing out out (as you've noted). If a plugin has not opted to put a filter on the output of these callbacks before echoing it, there's nothing that BP can do about it.

If you want a slightly simpler way to do what you're trying to do, just unhook the plugin's display functions using remove_action(). It looks like bbPress (2.3-beta2) doesn't actually use plugins.php when displaying content in groups, so there you'd have to find a different technique.

Is this enough of an answer? I'm not really clear what more BP could do.

#2 @modemlooper
7 years ago

I guess I'm requesting there be more logic to bp_template_title and content. Turn it into a function? bbPress uses plugins.php on profiles.

#3 @hnla
7 years ago

  • Cc hugoashmore@… added

#4 @boonebgorges
7 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

I think you're overthinking things.

First, I've confirmed that bp_get_template_part works fine:

function bp4855( $templates, $name ) {
	if ( 'members/single/plugins' == $name ) {
		$templates = array( 'foo.php' );
	}

	return $templates;
}
add_filter( 'bp_get_template_part', 'bp4855', 10, 2 );

This loads [my-theme]/foo.php rather than plugins.php.

Of course, foo.php does not contain any of the plugin content (in this case, bbPress). That's because bbPress has hooked its content to bp_template_content. So if I put the following line into foo.php, it works:

<?php do_action( 'bp_template_content' ); ?>

So, if you want to display totally different content from what bbPress is displaying, then you don't need to use the bp_template_content and bp_template_title hooks at all. Just put your new content directly in the replacement template. On the other hand, if all you want to do is add some additional content or markup, you can do so by making sure that your replacement template contains the do_action() line above. Finally, if you wish to continue using the plugins.php *template* but you don't want bbPress to show its *content* there, then unhook bbPress's display methods using remove_action().

I think this pretty much covers all the possible use cases, at least the ones I can see. If I've misunderstood, or if I'm missing something, please feel free to leave additional details on precisely what it is that you want to do. Until then, I'm marking this ticket invalid, as I believe you're asking for an enhancement that would introduce unnecessary complication to accomplish something that's already easily achievable in our template system.

#5 @modemlooper
7 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

using custom template parts is the wrong way to go. It's a bad idea considering theme compat is opening up usage to any theme. Plugin devs have no idea what themes will be used. Themes can create custom plugins.php files and if you want to override a plugins use if plugins.php it doesnt work properly.

#6 @boonebgorges
7 years ago

using custom template parts is the wrong way to go.

if you want to override a plugins use if plugins.php

It sounds like you're saying two contrary things here: either you think overriding plugins.php is a good thing, or you don't. Anyway, using custom templates *is* the right way to go if you need to make significant changes to the markup that is wrapping the content of the page. In this sense, it's no different from WP's core templating system: you override a template if you want to override the markup that wraps your content.

In any case, I have given you a number of suggestions of paths to take, depending on just what it is that you want to change on a given plugin page.

I'm afraid I still don't understand what it is that you're trying to do. If you simply don't like the content that bbPress is putting out to the screen, and you want to change a small part of it, then it's a bbPress problem. Can you please give a *specific example* of something that you're trying to change? I cannot test or give sufficient answers without specifics.

#7 @modemlooper
7 years ago

This ticket is an enhancement not a bug. what you suggest can work but not in the best way.

It's better for plugins to use plugins.php to display content, yes? If this is correct then there needs to be a way to override another plugins use of the file.

I already have come up against a problem with it and I can see other issues. plugins.php is not a full template, a template part. Markup can be broken when the first half of an element is in the parent template file and then you switch out plugins.php with another file. So, using custom files is not going to work when a theme can supply a custom plugins.php. The user nav is broken into two files with bp_get_displayed_user_nav() in the parent template and bp_get_options_nav() in the the child. I already have found issues with this because if you switch out the template file and plugins.php markup was changed the page breaks.

If every theme uses the same plugins.php markup then this would not be an issue.

My enhancement suggestion is either to leave plugins.php and add a sub template (plugins-content.php) and put add_actions there or add some sort of logic to bp_template_title

I cant get remove_action( 'bp_template_content', 'bbp_member_forums_topics_content' ); to remove the content

Last edited 7 years ago by modemlooper (previous) (diff)

#8 @boonebgorges
7 years ago

  • Milestone set to Future Release

there needs to be a way to override another plugins use of the file.

This is the part I guess I still don't get. I've shown you several ways to override plugins.php.

Markup can be broken when the first half of an element is in the parent template file and then you switch out plugins.php with another file

That's true, but that's true about all template parts, and all WP templates for that matter. There's only so much that we can do to prevent it.

I still don't see the use case here, and I still can't understand what kind of specific enhancements could be made (I don't fully grok how your sub-template suggestion would be helpful, or what kind of logic you're talking about). But it could just be me, so I'll put it into Future Release. If you can provide even just a proof-of-concept patch to show what you have in mind, that'd be great.

#9 @modemlooper
7 years ago

It's a template safety net to use a sub template and then switch that out instead of plugins.php. You've seen where header.php has
<body>

<div id="content">

, right?

What if header.php got swapped out by a plugin and then id=content was missing because the plugin has no way of knowing the mark up. That's one example that I'm dealing with.

I can not get remove_action( 'bp_template_content', 'bbp_member_forums_topics_content' ); to remove the bbPress content. If that worked it would solve half the problem of needing a sub template.

My suggestion for plugins.php:

		<?php do_action( 'bp_before_member_plugin_template' ); ?>

		<?php if ( ! bp_is_current_component_core() ) : ?>

		<div class="item-list-tabs no-ajax" id="subnav">
			<ul>
				<?php bp_get_options_nav(); ?>

				<?php do_action( 'bp_member_plugin_options_nav' ); ?>
			</ul>
		</div><!-- .item-list-tabs -->

		<?php endif; ?>

	       <?php bp_get_template_part( 'members/single/plugins-content' );  ?>

		<?php do_action( 'bp_after_member_plugin_template' ); ?>

then in plugins-content.php


       <?php do_action( 'bp_before_member_plugin_template_title' ); ?>
        
			<h3><?php do_action( 'bp_template_title' ); ?></h3>
		
		<?php do_action( 'bp_after_member_plugin_template_title' ); ?>
      
        
        <?php do_action( 'bp_before_member_plugin_template_content' ); ?>

			<?php do_action( 'bp_template_content' ); ?>
		
		<?php do_action( 'bp_after_member_plugin_template_content' ); ?>
Last edited 7 years ago by modemlooper (previous) (diff)

#10 @modemlooper
7 years ago

An analogy would be we are tearing down a wall to replace a window. Windows are built to certain standard sizes, what if walls were created with varying holes?

Last edited 7 years ago by modemlooper (previous) (diff)

#11 @r-a-y
7 years ago

  • Keywords 2nd-opinion removed

The 'bp_template_content' hook should be satisfactory, modemlooper.

The problem is you are trying to unhook the function too early.


Let's follow the yellow brick road!

  1. You're trying to remove this function call - bbp_member_forums_topics_content()
  2. bbp_member_forum_topics_content() is called from bbp_member_forums_screen_topics():

https://bbpress.trac.wordpress.org/browser/tags/2.2.4/includes/extend/buddypress/functions.php#L86

  1. bbp_member_forums_screen_topics() is called from BBP_Forums_Component::setup_nav():

https://bbpress.trac.wordpress.org/browser/tags/2.2.4/includes/extend/buddypress/loader.php#L189

  1. The setup_nav() method runs bp_core_new_subnav_item():

https://buddypress.trac.wordpress.org/browser/tags/1.7-beta1/bp-core/bp-core-buddybar.php#L199

  1. bp_core_new_subnav_item() is used to pass the screen function - bbp_member_forums_screen_topics() from point no. 3:

https://buddypress.trac.wordpress.org/browser/tags/1.7-beta1/bp-core/bp-core-buddybar.php#L279

Now, we've reached the end of the road!


The hook you're looking at is "bp_screens" and "bp_screens" runs at "bp_template_redirect" at priority 6.
https://buddypress.trac.wordpress.org/browser/tags/1.7-beta1/bp-core/bp-core-actions.php#L74

Conclusion: Run your hook any time after "bp_template_redirect" at priority 6.

Like so:

function bp4855() {
	remove_action( 'bp_template_content', 'bbp_member_forums_topics_content' );
}
add_action( 'bp_template_redirect', 'bp4855' );

This runs your removal hook at "bp_template_redirect" at priority 10, which is after when BuddyPress / bbPress runs its hook.

I wrote this quite quick, so I might be skipping a few steps, but hopefully you understand what you need to do in the future.

Run your code after the hook you want to override has executed.

Last edited 7 years ago by r-a-y (previous) (diff)

#12 @modemlooper
7 years ago

Thanks Ray, but there still is an issue with replacing plugins.php with itself.

If I wanted to replace activity.php I should use plugins.php to preserve theme structure. This method fails a bit with plugins.php. This issue is more about theme markup than content replacement and fixing the old issue of plugins using template files that break themes. I have 6 plugins I use that are broken because they supply templates and do not use plugins.php. One is bp ablum+ which I fixed yesterday.

The reason I suggest a sub template is it is easier to replace that and preserve page structure than removing a bunch of actions. BP album supplied templates and if they were based on the new legacy files then we have a broken site when the themes plugins.php and the supplied template files don't match.

If a plugin must supply a template then the plugin can swap sub template without fear of not having total theme support.

Last edited 7 years ago by modemlooper (previous) (diff)

#13 @r-a-y
7 years ago

If I wanted to replace activity.php I should use plugins.php to preserve theme structure.

activity.php is called from /members/single/home.php. So isn't that the file you need to adjust?

I have 6 plugins I use that are broken because they supply templates and do not use plugins.php.

This is indeed the problem; a lot of old plugins are going to have this issue.

These plugins have been doing it wrong, which is mostly our fault since the older version of the BP Skeleton Component told plugin devs to follow the 'bp_located_template' filter approach.

BP Album follows this older method:
https://github.com/BP-Media/bp-album/blob/master/includes/bpa.core.php#L356

I'm working on a similar problem with BP Followers and am working on a solution to maintain backpat and be future-forward with theme compat. I'll share that solution to everyone once I've finished testing. It looks like this is going to be quite easy to fix.

So I'm going to say that this is a dev awareness issue that devs will need to address for 1.7. Adding another template part inside plugins.php is only going to complicate matters.

Last edited 7 years ago by r-a-y (previous) (diff)

#14 @modemlooper
7 years ago

activity.php is a sub template of home. If you replaced home.php from a plugin the theme would break unless you specifically knew the mark up. This is essentially the issue. You would replace activity.php with plugins.php but what do you replace plugins.php with?

I don't think adding a template file will complicate things. It's an added measure to make sure the structure stays put. Curious to see your idea and how you are going to deal with overriding plugins.php when it's in use.

Last edited 7 years ago by modemlooper (previous) (diff)

#15 @r-a-y
7 years ago

If you replaced home.php from a plugin the theme would break unless you specifically knew the mark up.

Why would you want to replace home.php from a plugin? That is a part of the theme.

The theme should be handling those decisions and not a plugin. A plugin that wants to add templates should only look at injecting content into plugins.php. Unless that plugin is looking to add templates that are not under the members or groups component and is a new root level component altogether.

Like Boone said, it's hard to picture what you're trying to do without a decent case study.

I'll have a codex article up with my solution once I've tested with BP Album and can confirm it will work.

#16 @r-a-y
7 years ago

  • Component changed from Core to Theme
  • Keywords has-patch dev-feedback added; reporter-feedback removed
  • Milestone changed from Future Release to 1.7
  • Type changed from enhancement to defect (bug)
  • Version set to 1.7

Okay, I see a problem with plugins.php in bp-default. And I believe I see what modemlooper was referring to.

In bp-default, specifically /members/single/home.php, there is a call to plugins.php, however the actual /members/single/plugins.php in bp-default is a full template (with get_header() / get_footer() calls) rather than a template part.

However, bp-legacy uses plugins.php as a template part. So there is a discrepency between bp-default and bp-legacy. The problem is going to be addressing this so both bp-default and bp-legacy reference the same thing.

A solution I see is to leave bp-default's plugins.php alone for backpat, but introduce a new file called generic.php, which acts as a template part for plugins. To be compatible with bp-legacy, I have renamed plugins.php to generic.php. 01.patch illustrates what I mean.

If the other devs agree, we'll need to do the same thing with the groups component.

I know this is a late change in the game, but I think it's necessary.

@r-a-y
7 years ago

#17 @modemlooper
7 years ago

creating generic.php doesn't fix the issue but you now understand the issue. my suggestion to create the sub template allows the use of plugins.php regardless of bp-default or theme compat by creating a buffer around anything a plugin places inside plugins.php

#18 @r-a-y
7 years ago

Creating generic.php fixes the issue for the problem I noted in comment 16. This will be clear once I've created the accompanying codex article.

You can already buffer plugins.php if you know what you're doing. Just hook into 'bp_template_content' at priority 0 and 999 to do your buffer.

Last edited 7 years ago by r-a-y (previous) (diff)

#19 @boonebgorges
7 years ago

r-a-y, can you give an example of an existing plugin that fails in the way you describe? I don't totally understand what happens in such a case.

#20 @modemlooper
7 years ago

RAY, the issue brought up is exactly what I'm having but only on legacy plugins.php. Missmatched plugins.php, because themes have different markup. Just as you described plugins.php in bp-default being different. It's exactly the same issue.

#21 @r-a-y
7 years ago

r-a-y, can you give an example of an existing plugin that fails in the way you describe? I don't totally understand what happens in such a case.

In theory, no existing plugin has this problem, but it becomes a problem when a plugin is trying to support both bp-default and bp-legacy uniformity in BP 1.7, which is what modemlooper and myself are running into.

The major problem is the hooks and markup in /members/single/plugins.php are slightly different between bp-default and bp-legacy.

02.patch outlines the new conditional from bp-legacy that I've ported back to bp-default. The patch doesn't address the rearranging of hooks such as 'bp_before_member_plugin_template' and 'bp_after_member_plugin_template'.

Plugins will also need to hack bp_is_current_component_core() to remove the options nav if they need to. I'm running into this use-case.

02.patch is less of a nuisance than 01.patch, but custom themes would still need to apply this change to their plugins.php.

Let me know if I need to elaborate on anything.

Last edited 7 years ago by r-a-y (previous) (diff)

@r-a-y
7 years ago

#22 @boonebgorges
7 years ago

Thanks for the additional patch, r-a-y. I understand in theory why this is happening, but I still don't fully see what you're doing to try to support bp-default as well as bp-legacy. Can you share an actual snippet from a plugin where you're having the problem? I'm curious to see how you're loading the template.

#23 @r-a-y
7 years ago

I'm curious to see how you're loading the template.

With 02.patch, I'm loading the template the same way as always:

bp_core_load_template( 'members/single/plugins' );

This supports bp-default themes.

When theme compat is on, /wp-content/themes/YOUR-THEME/members/single/plugins.php will not exist and bp-legacy will kick in and load bp-templates/bp-legacy/buddypress/members/single/home.php.

bp-templates/bp-legacy/buddypress/members/single/home.php will load up the plugins.php template part on a non-core member page.

Last edited 7 years ago by r-a-y (previous) (diff)

#24 @modemlooper
7 years ago

The major problem is the hooks and markup in /members/single/plugins.php are slightly different between bp-default and bp-legacy.

My issue is this and that every theme can have a custom legacy plugins.php

#25 @imath
7 years ago

Hi modemlooper, r-a-y, boone,

As i'm not sure to understand all that have been discussed so far, i apologize if i'm wrong with the following.

A few time ago, i've worked on one of my plugin (bp-checkins) in order to be sure it will load correctly on bp-default / bp-legacy. Then as it worked i've published some content in the codex that describes the way i've done it. I haven't noticed the trouble you refer to in members area.

So i'm a bit confused, i hope i haven't missed something!!
So i've checked again with bbPress activated and everything works fine.

I think boone is right when saying:

can you give an example of an existing plugin...

It's always easier to understand when seeing the trouble in action ;)


My issue is this and that every theme can have a custom legacy plugins.php

modemlooper, i avoid this problem by using a custom name for my template on the same way that describes boone here

I first reference my plugin's path in the stack if my component is the current one and add a hook into the member screen callback if theme is not bp-default to include the filter

function bp_plugin_user_screen_callback(){
    //bp-default
    bp_core_load_template( apply_filters( 'bp_plugin_user_screen_callback', 'bp-plugin-main-user-template' ) );
 
    //if BP Default is not used, we filter bp_get_template_part
    if( !bp_plugin_is_bp_default() )
        add_filter( 'bp_get_template_part', 'bp4855', 10, 2 );
}

PS: i'll need to update the condition to include a current_theme_support('buddypress') check

#26 @modemlooper
7 years ago

imath, my issue is not 100% the same as what r-a-y brought up but they are relevant. Figuring out if it's bp-default or legacy is easy as you've shown. My issue is a plugin calling plugins.php and you want to remove the plugin from using that file and your plugin use that file instead. Yes, there has been suggestion to dehook every call to bp_template_content. But that is crazy when you want to add markup to the page with code. It just doesn't work. This is why I suggest a sub template that a theme or plugin can override.

If I want to support every WP theme available I need to be able to put content inside plugins.php that a theme supplies, not replace it with a file that most likely will not match the theme markup. That would be like replacing sidebar.php with so I can add a widget. The theme would break if the custom template file doesn't match the markup.

Another example: I want to block access to content being displayed in bp_template_content based on a certain criteria, removing every call to bp_template_content doesn't work because I have no idea what is hooking to this action. What to do? Replace plugins.php with a different template file, which has been suggested? You just broke every theme out there if they are not using the exact markup of the default legacy plugins.php.

Last edited 7 years ago by modemlooper (previous) (diff)

#27 @r-a-y
7 years ago

My problem is plugins.php hardcodes the options nav to use no AJAX with the 'no-ajax' CSS class; I want AJAX. (Line 45 in bp-default's plugins.php)

I've worked around this problem for bp-default; though it's not really elegant.


My issue is a plugin calling plugins.php and you want to remove the plugin from using that file and your plugin use that file instead. Yes, there has been suggestion to dehook every call to bp_template_content.

Follow what I mentioned in comment 18.

Buffer around 'bp_template_content' at a low priority and high priority to get rid of the plugin entirely without even knowing what was injected.

// start output buffer
function bp4855_start_buffer() {
	ob_start();
}
add_action( 'bp_template_content', 'bp4855_start_buffer', 0 );

// remove everything that was added in 'bp_template_content'
function bp4855_end_buffer() {
	// if you wanted to manipulate the contents of the buffer,
	// uncomment the following line
	// $buffer = ob_get_contents();

	// stop output buffer
	ob_end_clean();

	// do all your manipulation here
	// eg. add your template part, use the $buffer variable if needed
}
add_action( 'bp_template_content', 'bp4855_end_buffer', 999 );

Any other concerns before I close this discussion?

#28 @boonebgorges
7 years ago

  • Milestone changed from 1.7 to Future Release

Thanks for spelling out these solutions, r-a-y.

I think that your strategies are fine for the moment, and are really the only way we can maintain backward compatibility with existing themes.

However, I've been thinking more about it, and I think that in the medium-to-long-term, we should go with another layer of template, like modemlooper suggests. Basically, in place of having do_action( 'bp_template_content' ) (or, more likely, in addition to it), we would have bp_get_template_part( 'plugin-content', 'group' ). Then, internally, we would have a template hierarchy, so that 'plugin-content-bp-docs.php' or 'plugin-content-bbpress.php' or whatever, would be preferred if found.

I think this is a better way forward than using a do_action() callback, like we do now with bp_template_content. As we've seen, this callback method is problematic if only because it's often difficult to unhook (see r-a-y's output buffer trick).

#29 @modemlooper
7 years ago

Thanks, Boone. I am using output buffering before R-A-Y even suggested and I didn't really want to do that. Another reason to use a sub template is if a plugin supplies a template and then you want the ability for a theme to override your own template file like how we do it for BuddyPress. It's just easier to use a file than dehooking actions and messing with plugins.php which should be nothing more than a holder as it does now but with added benefit of not getting replaced by a plugin.

Last edited 7 years ago by modemlooper (previous) (diff)

#30 @boonebgorges
7 years ago

Right. I think that theme compat shows that the way of the future is template hierarchy, so it makes sense that we would have a system going forward that would encourage using template hierarchy as a best practice. I'm imagining that the plugin-content.php template would actually be *empty*, or at most would contain a do_action( 'bp_template_content' ) hook for backpat; then we would update our plugin documentation (ie the skeleton component) to encourage plugin devs to put their markup into an appropriate template part (instead of a callback), and then add their plugin template location to the bottom of the BP template stack. There are some details to be worked out with this config, but it seems the most elegant going forward.

#31 @modemlooper
7 years ago

Yes, better to have markup in template files that theme developers can customize.

#32 @DJPaul
7 years ago

  • Keywords has-patch dev-feedback removed

I have no idea on the status of this ticket, but removing has-patch as we seem to be some distance away from an agreed solution.

#33 @DJPaul
3 years ago

  • Component changed from Appearance - Template Parts to Templates

#34 @DJPaul
2 years ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

Closing most tickets related to BP-Default and BP-Legacy, since the upcoming BP-Nouveau template pack (planned for 3.0) will make these redundant.

#35 @DJPaul
2 years ago

Closing most tickets related to BP-Default and BP-Legacy, since the upcoming BP-Nouveau template pack (planned for 3.0) will make these redundant.

Note: See TracTickets for help on using tickets.