Skip to:
Content

Opened 2 years ago

Closed 17 months ago

#3741 closed enhancement (fixed)

BuddyPress should have theme compatibility

Reported by: johnjamesjacoby Owned by: johnjamesjacoby
Milestone: 1.7 Priority: highest
Severity: critical Version:
Component: Theme Keywords: dev-feedback
Cc: rachel@…, mercijavier@…, karmatosed@…, nvwd, netweb

Description

Currently BuddyPress relies on a custom theme to handle the presentation of BuddyPress content. This is uncool for most situations and provides no ability to easily meld an existing theme with BuddyPress functionality.

Please use this ticket for discussion, patches, and commits towards BuddyPress theme compatibility for the 1.6 dev cycle.

Attachments (8)

3741.patch (281.8 KB) - added by johnjamesjacoby 20 months ago.
First Pass
3741.2.patch (285.6 KB) - added by johnjamesjacoby 20 months ago.
Second Pass - Fixes Settings; adds support for Blogs and Activity
3741.3.patch (285.0 KB) - added by johnjamesjacoby 20 months ago.
Third Pass -- Fix newline errors and s/bbp/bp
3741.4.patch (364.9 KB) - added by johnjamesjacoby 20 months ago.
Fourth Pass - Bring over dtheme ajax; fix bp-default compatibility
legacyadjustments.diff (1.4 KB) - added by karmatosed 20 months ago.
3741-move-template-pack-folder.001.patch (580.8 KB) - added by DJPaul 19 months ago.
3741.register.01.patch (9.4 KB) - added by r-a-y 19 months ago.
page title hidden link.png (31.4 KB) - added by DJPaul 19 months ago.

Download all attachments as: .zip

Change History (67)

comment:1 johnjamesjacoby2 years ago

See #3739 for original ticket.

comment:2 follow-up: r-a-y2 years ago

I'm not sure what the goal for v1.6 is for theme compat, but an easy way while still keeping bp-default is if we could use a wrapper function for locate_template() in bp-default.

This wrapper function would check the STYLESHEETPATH for a BP template file first before falling back to bp-default's template files.

Then a simple plugin could be written to enqueue the JS and CSS and override the "bp_located_template" filter.

Related: #2649

Also see: http://bpdevel.wordpress.com/2011/01/12/there-is-a-dev-chat-today-wednesday-12t/#comment-743

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

comment:3 in reply to: ↑ 2 johnjamesjacoby2 years ago

Replying to r-a-y:

I'm not sure what the goal for v1.6 is for theme compat, but an easy way while still keeping bp-default is if we could use a wrapper function for locate_template() in bp-default.

This wrapper function would check the STYLESHEETPATH for a BP template file first before falling back to bp-default's template files.

Then a simple plugin could be written to enqueue the JS and CSS and override the "bp_located_template" filter.

Related: #2649

Also see: http://bpdevel.wordpress.com/2011/01/12/there-is-a-dev-chat-today-wednesday-12t/#comment-743

So, the problem with that method is falling back on a totally new page layout will only provide a more confusing experience. Having there be a "BuddyPress Theme" and then an "active theme" and try to sort out the bits won't fly.

Ultimately we need a bbPress 2.0 style approach, where we split bp-default up into template parts, hook into the_content early, render the proper template part in an output buffer, and replace the original the_content with the new BuddyPress template part. That way it's a seamless experience and your average user doesn't ever know what's going on.

comment:4 r-a-y2 years ago

I understand the end goal is to follow suit with how bbPress 2.0 uses output buffering.

However, this ticket is marked for the 1.6 milestone; is the bbPress approach attainable for v1.6? Or is it about doing a small enhancement for v1.6? If it's the latter, what will it entail?

comment:5 DJPaul2 years ago

It's in 1.6 so we can look at doing any groundwork or preparation that we'll need to for this in 1.7

comment:6 follow-up: modemlooper2 years ago

I dunno, bbpress has major child theme issues to be fixed before I'd consider using the same approach?i tried overriding bbpress in child themes and was a bust.

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

comment:7 in reply to: ↑ 6 johnjamesjacoby2 years ago

Replying to modemlooper:

I dunno, bbpress has major child theme issues to be fixed before I'd consider using the same approach?i tried overriding bbpress in child themes and was a bust.

Then you're doing it wrong. It isn't particularly easy to do the output buffer trick, so overriding it again isn't straightforward, but still easily achieved. bbPress 2.1 improves on this issue though.

comment:8 boonebgorges2 years ago

  • Keywords 1.7-early added
  • Milestone changed from 1.6 to Future Release

We've got a lot to talk about in terms of how this is going to be cashed out, especially in terms of backward compatibility with existing themes. Let's put it at the top of our list of things to discuss at the beginning of the 1.7 cycle.

comment:9 bowromir2 years ago

I think the BBPress way is "ideal" in terms of ease of theme development and making BuddyPress function with any regular old WP Theme. But like ModemLooper mentions currently the BBPress "way" makes the Parent/Child Theme setup we've gotten so used to not fuction. I know it's possible to achieve, but the process is a little to complex, especially for people who are new to BP/BBPress Theming. If JJJ can solve that issue (which I understand he's in the process of doing) and keep the current child/theme workflow intact, and at the same time load the template parts from the BP plugin folder, that would be great.

1: Activate BP and Templates are injected automatically for BP pages
2: Need to modify a component template like activity/index.php? Move template over to active parent theme and modify
3: Need to modify the forked template for some reason? Create a child theme of your active theme and repeat step 2.

Not sure if the last part is possible, might be a bit much, but honestly this functionality does come in super handy when you do a lot of BP theming/projects.

Looking forward to see how this problem will be solved in the future :-)

Last edited 2 years ago by bowromir (previous) (diff)

comment:10 johnjamesjacoby2 years ago

Can be ported over from bbPress pretty easily once 2.1 launches. The bp-default theme would end up being moved into a different set of template parts, and we'd need to possibly deal with decoupling bp-default and the BuddyPress components, as right now they are almost purpose-built for each other.

Anyhow, the bbPress method opens a lot of doors for BuddyPress and it'd be great to get some help porting that code over.

comment:11 boonebgorges21 months ago

  • Keywords 1.7-early removed
  • Milestone changed from Future Release to 1.7
  • Priority changed from normal to highest
  • Severity changed from normal to critical

This will probably be the marquee feature of the release. Let's discuss more at the next dev chat.

comment:12 rachelbaker21 months ago

  • Cc rachel@… added

Missed the dev chat yesterday, but happy to lend a hand here.

comment:13 mercime21 months ago

  • Cc mercijavier@… added

comment:14 Mamaduka21 months ago

  • Cc georgemamadashvili@… added

comment:15 sooskriszta21 months ago

Get rid of the top admin/tool bar.

It more or less works for WordPress as it is meant for site admins, editors and authors. The bulk of users of the site never log in and never have to see the bar.

This is not the case in BuddyPress. *All* users are supposed to log in. As a site admin, I don't want to saddle end-users with things like this bar, however cool it may seem in WordPress. I want to present my site in the theme of my choice very simply.

comment:16 MrMaz20 months ago

Has there been a discussion about how to handle, or what to do about, backwards compatibility for plugins which use the various plugins.php templates? There are also quite a few plugins which ship with their own component directory templates.

I think there should be a very clear path for plugin authors to follow in order to bring their templates/code in line with the new strategy. The proposed changes to templating are very exciting, but when plugins are activated which do it "the old way," its going to negate a lot of the advantages provided by the 1.7 changes, at least from a site admin perspective.

comment:17 boonebgorges20 months ago

MrMaz - The discussion hasn't quite gotten that far yet, but I'm 100% on board with you. (I plan to bear the title Backward Compatibility Nudnik for this feature.) I would like us to have a fallback system in place that prefers old-style theme templates wherever they are found. You're quite right that we'll also have to have a plugins.php template (or something that seamlessly does the work that the template does now) because otherwise lots of stuff will break.

It's likely that plugins that provide their own *entire* templates - from get_header() to get_footer() - will not fit neatly into all of this. At minimum, we can make sure that these plugins continue to *work*, even if it means that they hijack the theme's template for that particular page. But we also must spell out the correct steps for plugin authors to remediate their plugins to fit in with the new theme compat layer - this is a documentation issue, as you note.

If you have specific ideas about how this would work in a way to support backward compatibility in a maximal way, I'd be very curious to hear them.

johnjamesjacoby20 months ago

First Pass

comment:18 johnjamesjacoby20 months ago

  • Keywords 2nd-opinion needs-testing added

Attached patch is a rough first pass at BuddyPress theme compatibility:

  • Copies templates and CSS from bp-default.
  • No ajax/CSS yet, though not difficult to port over.
  • Groups, Members, Profiles done.
  • Activity, Blogs left to do.
  • Some actions, action order, filters, names, etc... are subject to change.
  • Does not change rewrite rules; still uses URI catcher.
  • Includes code from #4470, since it uses the buddypress() function in place of the $bp global where needed.

This patch is just enough to get it working as a proof of concept. The actions and orders are subject to scrutinization before anything is commit worthy, though it's pretty close to done. Also, the way this works when bp-default is active isn't fully tested, so it will probably break.

There's also no UI for picking the BuddyPress Theme Compat directory yet; this should be a theme option later, if/when there is more than option available.

johnjamesjacoby20 months ago

Second Pass - Fixes Settings; adds support for Blogs and Activity

comment:19 Mamaduka20 months ago

  • Cc georgemamadashvili@… removed

comment:20 karmatosed20 months ago

  • Cc karmatosed@… added

comment:21 DJPaul20 months ago

Applying 3741.2.patch…

patching file bp-activity/bp-activity-screens.php
patching file bp-blogs/bp-blogs-screens.php
patching file bp-core/bp-core-actions.php
patching file bp-core/bp-core-catchuri.php
patch: malformed patch at line 476: \ No newline at end of file

Then it stopped patching.

comment:22 follow-up: DJPaul20 months ago

Unable to try the patch because of the above, but reading the file, it looks like solid starting point -- really nice work, John!
I know this is going to get more iteration, but I noticed that you still have a few "bbp_" references in an option and some of the phpdoc.

There's also no UI for picking the BuddyPress Theme Compat directory yet; this should be a theme option later, if/when there is more than option available.

When I implemented theme compat. in one of my own plugins, I took this option out the UI. I couldn't see a valid use case for it. It seemed like a way to let people choose which compat. pack to use, which I found conceptually similar to the Appearance > Themes screen. I don't think we should provide a UI for this option, but I assume you had some use cases in mind when you built this for bbPress. I'd love to learn a little more about the idea behind the option.

johnjamesjacoby20 months ago

Third Pass -- Fix newline errors and s/bbp/bp

comment:23 in reply to: ↑ 22 johnjamesjacoby20 months ago

Replying to DJPaul:

I'd love to learn a little more about the idea behind the option.

The goal here is to enable themes and plugins to register their own packs of template parts, and allow an admin to switch the BuddyPress/bbPress templates only, without changing the blog's theme completely. This allows us to have a default/canonical set of template parts that we can constantly update, without the burden of the user never being able to switch off of them.

There's no great UI/API for this yet, though. It's literally just an option that saves the name of the registered theme compat, with a drop down to choose from one of them. I think this is fine enough for a v1, and we can look into fancying up the place when we're happy with how it's working.

Version 0, edited 20 months ago by johnjamesjacoby (next)

comment:24 nvwd20 months ago

  • Cc nvwd added

comment:25 netweb20 months ago

  • Cc netweb added

johnjamesjacoby20 months ago

Fourth Pass - Bring over dtheme ajax; fix bp-default compatibility

comment:26 boonebgorges20 months ago

Wooooo!

This is great, John. For the most part, it works like magic. Awesome work.

A couple things:

  • BP_Group_Extension plugins aren't fully working. To display content from the display() method, the _display_hook() method loads /groups/single/plugins.php, instead of /groups/single/home.php. But the logic of BP_Groups_Theme_Compat::single_content() doesn't account for the existence of plugins.php (which is, itself, a top-level template). I can imagine a couple different ways to approach this. On one end of the spectrum, the fact that plugins.php is a top-level template has always been annoying, and this might be a chance to fix that, by merging plugins.php into home.php. However, this will probably cause backward compatibility plugins with at least some plugins that reference plugins.php directly (rather than through BP_Group_Extension). So a safer, if less elegant, fix would be to take the opportunity in BP_Group_Extension::_display_hook() to add a filter to the theme compat template (this would be a reliable and lightweight way to ascertain that the current page needs plugins.php vs home.php). FWIW, this problem doesn't arise for /members/single/plugins.php, because it's not a top-level template - it's loaded by the logic in members/single/index.php.
  • You've copied and pasted the AJAX handlers from bp-default into bp-theme-compat/buddypress-functions.php. When I first loaded my rig with bp-default enabled, I got fatal errors about the duplicate function names. A lightweight way to solve this would be to change the function prefix for the bp-theme-compat versions of these callbacks.
  • s/bbp_/bp_
  • :)

comment:27 imath20 months ago

  • Keywords 2nd-opinion needs-testing removed
  • Type changed from task to enhancement

Hello,

First : very interesting new way to manage theme compatibility : great work!

I've played a bit with it and the twentyeleven theme. I've noticed that the activity entry when calling the permalink was leading to 'members/username/home' BuddyPress template part instead of 'members/username/activity/p/acitivity_id' one. So i've edited the file /buddypress/bp-activity/bp-activity-screens.php by adding after line 346 to handle this case. It seems to work, but is surely improvable :

	public function is_activity() {

		//starting after line 346
		if( bp_is_single_activity() ) {
			add_action( 'bp_template_include_reset_dummy_post_data', array( $this, 'single_activity_dummy_post' ) );
			add_filter( 'bp_replace_the_content',                    array( $this, 'single_activity_content'    ) );
		}
	}
	// then i've added this 2 functions in the class :
	public function single_activity_dummy_post() {
		bp_theme_compat_reset_post( array(
			'ID'             => 0,
			'post_title'     => __( '', 'buddypress' ),
			'post_author'    => 0,
			'post_date'      => 0,
			'post_content'   => '',
			'post_type'      => 'bp_activity',
			'post_status'    => 'publish',
			'is_single'     => true,
			'comment_status' => 'closed'
		) );
	}
	public function single_activity_content() {
		bp_buffer_template_part( 'members/single/activity/permalink' );
	}

Then i kept on playing with it and in the wp editor i've changed the template file from default template to sidebar template in the Page Attributes box and it didn't changed the template. It was still page.php that was loaded. So i've edited some files to make it work and it worked... But i then try with a template that uses several get_template_part function like the showcase.php and i guess, i had a bad idea as the different content are all filled with BuddyPress content.

Just in case this is what i've done :

//in bp-core/bp-template-loader.php
// edited function bp_get_theme_compat_templates after line 302
	$wp_editor_template = bp_get_page_template();
		
	if( !empty( $wp_editor_template ) )
		$templates = array_merge( array( $wp_editor_template ), $templates );

	return bp_get_query_template( 'buddypress', $templates );

//in bp-core/bp-core-template.php added 2 functions to watch for a wp_editor template
/**
* do this BuddyPress page has a custom template ?
*/
function bp_is_page_template( $template = '' ) {
	if ( bp_is_blog_page() )
		return false;
	
	/* not sure of this one...*/	
	$page_id = get_queried_object_id();
	
	/* just in case */
	if( empty( $page_id ) ){
		$component_page_ids = bp_core_get_directory_page_ids();
		$current_component = bp_current_component();
		$page_id = $component_page_ids[ $current_component ];
	}

	$page_template = get_page_template_slug( $page_id );

	if ( empty( $template ) )
		return (bool) $page_template;

	if ( $template == $page_template )
		return true;

	if ( 'default' == $template && ! $page_template )
		return true;

	return false;
}

/**
* returns the template chosen from the wp editor
*/
function bp_get_page_template( $current_component = '' ) {
	$component_page_ids = bp_core_get_directory_page_ids();
	
	if( empty( $current_component ) )
		$current_component = bp_current_component();
	
	$page_id = $component_page_ids[ $current_component ];
	
	if( !empty( $page_id) )
		return apply_filters( 'bp_get_page_template', get_page_template_slug( $page_id ) );
		
	else 
		return false;
}

//and added a specific condition to add body class in bp_get_the_body_class function
			if( bp_is_page_template() ) {
				$bp_classes[] = 'page-template';
				$bp_classes[] = 'page-template-' . sanitize_html_class( str_replace( '.', '-', bp_get_page_template() ) );
			}

// and finally edited the bp-core/bp-core-theme-compatability.php file
//after line 368
$defaults['is_singular'] = bp_is_page_template() ? false : is_singular();

// then after $wp_query->is_single  = $dummy['is_single'];
	$wp_query->is_singular  = $dummy['is_singular'];

Tanks again for the great work.

comment:28 imath20 months ago

  • Keywords 2nd-opinion needs-testing added
  • Type changed from enhancement to task

comment:29 johnjamesjacoby20 months ago

BOOM

  • Let's make groups work like members going forward. It won't break backpat for existing themes that are already customized, and makes it work the way we want it to going forward in theme compat.
  • I knew this would happen; just didn't get to renaming all the bits in the patch.
  • Tsk tsk.
  • Profit!

comment:30 boonebgorges20 months ago

Let's make groups work like members going forward. It won't break backpat for existing themes that are already customized, and makes it work the way we want it to going forward in theme compat.

I think this will probably work, but I'll need to think more about the different scenarios. Don't want to end up with a situation where the old plugins.php (with header/footer) is called into the theme compat version of home.php. Should look at some old, pre-BP_Group_Extension plugins to see if that's a possibility.

comment:31 karmatosed20 months ago

I've added a small patch (legacyadjustments.diff) to fix up a few bits of formatting in the legacy theme. I tested mainly on Twenty-Twelve trying to use it as the bench mark then move into other testing once it all works well there.

comment:32 karmatosed19 months ago

I am going to tentatively raise that child themes may be an issue with the current theme compatibility. I just created a very simple child of Twenty Twelve which as I went to view activity failed with this:

[16-Sep-2012 19:12:35 UTC] PHP Warning: include(): Failed opening '/Applications/MAMP/htdocs/buddypressbeta/wp-content/themes/buddypressui/page.php' for inclusion (include_path='.:/Applications/MAMP/bin/php/php5.4.4/lib/php') in /Applications/MAMP/htdocs/buddypressbeta/wp-includes/template-loader.php on line 43

Would be interested in if it's just me or can anyone recreate this? I have had issues with my install this weekend so checking what others get.

Last edited 19 months ago by karmatosed (previous) (diff)

r-a-y19 months ago

comment:33 follow-up: r-a-y19 months ago

register.01.patch adds support for the registration / activation templates in theme-compat mode and does a few other things:

  • Introduces bp_is_theme_compat() to tell if theme compatibility is running.
  • Navigating to wp-login.php?action=register redirects to the BP registration page again
  • Accessing a restricted item like /members/USERNAME/settings/ did not show the login form. This is addressed by defaulting the mode in bp_core_no_access() to 2 when theme compat is running.

JJJ: Let me know if this approach is okay.

---

Edit: Just looking at my bp_is_theme_compat() function and that always returns true! Was trying to find a way to tell if theme compat was active. bp_is_theme_compat_active() only works for the current page you're on. I was trying to use this on wp-login.php via bp_core_wpsignup_redirect() > bp_get_signup_page() > bp_has_custom_signup_page().

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

comment:34 hnla19 months ago

  • Keywords dev-feedback added; 2nd-opinion needs-testing removed
  • Severity changed from critical to minor
  • Type changed from task to enhancement

Is there any reason why we can't move the 'css' & 'js' folders living under 'bp-legacy' into bp-legacy/buddypress/ thus being able to simply copy over the folder /buddypress/ lock stock into custom theme and have a neater theme structure rather than have to dump folders into theme route of a generic nature 'css', 'js' where one might have things nested under /inc/, /assets/ etc.

$file = is_rtl() ? 'buddypress/css/buddypress-rtl.css' : 'buddypress/css/buddypress.css';

comment:35 DJPaul19 months ago

  • Severity changed from minor to critical

comment:36 djpaul19 months ago

(In [6366]) Add new bp_group_class() template function to generate row class styles for the groups directory template parts. See #3741

This first pass adds classes for assistance with basic zebra striping, user status in the group (admin/member/mod), and the type of group (public/private/hidden).

comment:37 djpaul19 months ago

(In [6368]) Add new bp_member_class() template function to generate row class styles for the member directory template parts. See #3741

This first pass adds classes for assistance with basic zebra striping and whether the user has recently been active.

comment:38 djpaul19 months ago

(In [6382]) Add first pass at new bp_blog_class() template function to generate row class styles for the sites directory template parts. See #3741

comment:39 r-a-y19 months ago

(In [6401]) Theme Compat:

  • Introduce BP_Registration_Theme_Compat class - Adds registration / activation template support to theme compat.
  • Remove headers in bp-legacy register.php and activate.php template files as this is now handled by the_title().
  • Fix notice during signup when the XProfile component was disabled

comment:40 in reply to: ↑ 33 johnjamesjacoby19 months ago

Replying to r-a-y:

register.01.patch adds support for the registration / activation templates in theme-compat mode and does a few other things:

  • Introduces bp_is_theme_compat() to tell if theme compatibility is running.
  • Navigating to wp-login.php?action=register redirects to the BP registration page again
  • Accessing a restricted item like /members/USERNAME/settings/ did not show the login form. This is addressed by defaulting the mode in bp_core_no_access() to 2 when theme compat is running.

I don't think this function is necessary. The 'mode' bit in bp_core_no_access() is weird, too. We should just always redirect to wp-login.php, and also hooking into 'login_init' (or a more specific login page action) and redirecting from there if we can tell that a more suitable theme-side page exists.

Trying to predict "whether a theme has theme compat enabled" defeats the purpose of including theme compatibility in BuddyPress core. It's supposed to always be on, only kicking in when no template file is found in the parent/child theme stack.

Allowing themes to opt into or out-of using it via a switch, theme support, filter, or otherwise, is a slippery slope I'd rather we didn't fall into, at least until we absolutely need a way to do it in core ourselves, and I don't think this is that case.

comment:41 follow-up: r-a-y19 months ago

JJJ, after doing some more testing, you're absolutely right that bp_enable_theme_compat() is not necessary.

In r6404, I have removed bp_enable_theme_compat() and introduced a new function to check if bp-default is running - bp_is_theme_bp_default(). That should still do the same thing as what I originally intended.

Let me know what you think.

comment:42 DJPaul19 months ago

Per 'page title hidden link.png' (twentyten), the hardcoded links put into the page title area do not have sufficient formatting in all themes to differentiate between the title and the link.

comment:43 follow-up: DJPaul18 months ago

The groups and members widgets enqueue a JS file. It would be nice to find some way of not enqueue these files, and merging into a template pack's JS file.

comment:44 follow-up: DJPaul18 months ago

With the http://wordpress.org/extend/themes/easel theme, it access $authordata->thing directly without checking if the property exist. As we set post_author = 0 in the reset function, $authordata is never set and this throws a php warning. Do we care about this situation, or want to consider setting post_author = 1?

comment:45 johnjamesjacoby18 months ago

(In [6436]) Theme Compat:

  • Put back 'bp_register_theme_directory' sub action, hooked to 'bp_loaded'
  • Move register_theme_directory() back into BuddyPress root class method.
  • Introduces old_themes_dir variable, which will get deprecated when we remove bp-default from core.
  • Create 'bp-templates' root folder, for theme-compat templates to live in, and not conflict with 'bp-themes'.
  • Fixes issue where 'bp-legacy' would show up as "Broken" in Appearance > Themes.
  • Brute-force switch from bp-default to WP_DEFAULT_THEME on deactivation, and update theme roots, to fix issue when deactivating BuddyPress while theme stack relies on bp-default.
  • See #3741

comment:46 johnjamesjacoby18 months ago

(In [6439]) Theme Compat:

  • Move 'bp_add_template_locations' filter from 'bp_get_template_part' to 'bp_locate_template'
  • Allows for easier template location across multiple theme configurations.
  • Revert part of r6394 in lieu of this method.
  • See #3741

comment:47 johnjamesjacoby18 months ago

(In [6458]) Theme Compat:

  • General theme friendliness.
  • Update bp-legacy CSS to blend better with default WordPress themes.
  • See #3741.

comment:48 johnjamesjacoby18 months ago

(In [6459]) Theme Compat:

  • Tweak single activity permalink CSS.
  • Tweak more paddings and margins.
  • See #3741.

comment:49 johnjamesjacoby18 months ago

(In [6462]) Theme Compat:

  • Add 'bp_add_template_locations' filter back to 'bp_get_template_part' filter.
  • Add array_unique() to bp_add_template_locations() to remove possible duplicates.
  • See #3741.

comment:50 johnjamesjacoby18 months ago

(In [6464]) Theme Compat:

  • Remove list-style from assorted list items.
  • Enforces list-style: none; on elements that might have it inherited from the parent or child theme.
  • See #3741.

comment:51 in reply to: ↑ 44 johnjamesjacoby18 months ago

Replying to DJPaul:

With the http://wordpress.org/extend/themes/easel theme, it access $authordata->thing directly without checking if the property exist. As we set post_author = 0 in the reset function, $authordata is never set and this throws a php warning. Do we care about this situation, or want to consider setting post_author = 1?

Good catch, but I don't think it matters. Accessing array keys or object variables directly is fragile, and at worst triggers a debug notice for those of us with WP_DEBUG on. Annoying, but not worth worrying about too much.

Would probably make a better patch upstream to the theme author, to put an empty() check in there to avoid the notice(s) as needed.

comment:52 in reply to: ↑ 41 ; follow-up: johnjamesjacoby18 months ago

Replying to r-a-y:

JJJ, after doing some more testing, you're absolutely right that bp_enable_theme_compat() is not necessary.

In r6404, I have removed bp_enable_theme_compat() and introduced a new function to check if bp-default is running - bp_is_theme_bp_default(). That should still do the same thing as what I originally intended.

Let me know what you think.

In various commits above, I've reworked some logic so that bp_is_theme_bp_default() isn't necessary either. bp_locate_template() can now cover all scenarios with relative ease: parent, child, theme-compat, filtered. It could before, but looking more into this issue allowed for some hardening of the logic involved.

Give it a go, let me know if things are still working to your expectations.

comment:53 in reply to: ↑ 43 johnjamesjacoby18 months ago

Replying to DJPaul:

The groups and members widgets enqueue a JS file. It would be nice to find some way of not enqueue these files, and merging into a template pack's JS file.

We could defer the enqueue to the footer, and enqueue the JS on the widget display. It's a hack, but would guarantee the JS would only load when the widget needs it.

comment:54 karmatosed18 months ago

Just kicking around the coolness that is the recent flurry of updates to theme compat - loving it so far. I did notice that the messages is still overlapping. This is on the view messages table. I had a patch a while back on this ticket: http://buddypress.trac.wordpress.org/attachment/ticket/3741/legacyadjustments.diff - this solved that and a few other things (some of which I think are now sorted though).

comment:55 in reply to: ↑ 52 ; follow-up: rogercoathup18 months ago

Replying to johnjamesjacoby:

What's best prac (1.7) for specifying the 'outer' page template to use for our content e.g. want with sidebar for single user, but full width for single group?

Off the shelf, it just injects all our template parts in to the theme's default page template, but want (need) to be able to change the page template that's used dependent on which component / content is being displayed.

comment:56 in reply to: ↑ 55 mercime18 months ago

Replying to rogercoathup:

Some thoughts on this, though it would be cool to have an easier way to accomplish specifying the 'outer' page templates.

comment:57 DJPaul18 months ago

Is the group plugins.php support fixed in trunk now?

comment:58 r-a-y18 months ago

(In [6490]) Theme Compat:

  • In bp_locate_template(), make sure templates placed in custom template

locations (as set in bp_add_template_locations()) are recognized.

  • Previously, the fallback template would stop the foreach loop from

checking the next available template because the fallback template will
always exist.

  • Now, the fallback template is moved out of the foreach loop and into a

separate one so all template locations are checked.

comment:59 johnjamesjacoby17 months ago

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

Closing this, as it's in and working well, and there are other tickets for other specific issues.

Note: See TracTickets for help on using tickets.