Support for bp-default template overrides

[boonebgorges]

This is not so much a bug as a point for discussion.

infinity-cbox uses theme compat, which requires new-style, bp-legacy template overrides. These overrides differ from the old system (bp-default) in the following ways:

1. bp-legacy templates are pulled from the template stack locations - by default, /buddypress/, /community/, or the infinity-cbox template stack location. bp-default templates are pulled from component directories in the root (/members/, /groups/, etc).
2. bp-default top-level templates (generally called index.php or home.php) include calls to get_header(), get_footer(), get_sidebar(). These calls are absent in bp-legacy templates.
3. Nested templates in bp-default use locate_template(). Nested templates in bp-legacy use bp_get_template_part().
4. bp-legacy top-level templates include a <div id="buddypress"> wrapper.

If you currently have a cbox-theme child theme that overrides templates (using the bp-default system), here's what happens when you switch to the new cbox-theme:

a. Overridden templates like /members/index.php are properly found and loaded. (If you've got both /members/index.php and /buddypress/members/index.php, the latter takes precedence, which I think is good.)
b. Any templates nested within these overridden templates are not loaded, because the locate_template() call fails - item (3) above.
c. bp-default templates look funky, though, tbh, not as bad as I'd expect.

This is going to result in a moderate level of brokenness for these themes. Let's work out a strategy. There are some things we can do technically, and other things we should document.

Technically, we could detect when a bp-default template is being loaded, and when it is, do one or more of the following:

• Noop the calls to get_header(), etc (see (2) above)
• Dump the whole thing in an output buffer and wrap in <div id="buddypress">, to make sure styling and scripts still work (see (4) above)
• Hook somewhere into locate_template() to redirect template lookups to the template stack (see (3) above)

I can imagine sinking a lot of time into this, and still not really getting it right - the pages are still going to look lousy, as at least some markup has changed in the bp-legacy templates. So, another technical strategy is to prevent BP from recognizing bp-default templates. Instead of having a messed-up site, custom themes would have a site that uses the bp-legacy/infinity-cbox templates, and perhaps we could show an admin notices letting admins know they'll need to migrate their templates over manually. (This seems more sane to me, but I'm curious what you guys think.)

In terms of documentation, it would be ideal to have documentation along these lines https://codex.buddypress.org/themes/theme-compatibility-1-7/bp-17-upgrading-template-packed-themes/, but more technically helpful - a brief walkthrough of an actual example (telling you to remove calls to get_header(), etc) would be nice.

@r-a-y @MrMaz Any thoughts about the above? Thanks in advance :)

[MrMaz]

Boone,

Thanks for taking the time to write a detailed opening to this discussion. I'm very happy that you are open to a reasonable amount of broken backwards compatibility, although I still want to do everything possible to avoid headaches for current CBOX adopters.

I am going to dig more deeply into the issues you raised before I reply to them specifically.

My only initial thought, which is more general, is that we need to be careful to differentiate between the causes of broken compat... not that one is any worse than the other, only that I think it will avoid confusion. I believe there are only two:

1. Compat issues that are a result of switching to bp-legacy (any theme could encounter this).
   1. Existing programmatic solutions might already exist to ease transitioning.
   2. User space upgrade guides might already exist as a starting point for our documentation.
2. Compat issues that are a result of Infinity core changes (likely fixable internally).
   1. Possibility that it really is a bug.
   2. Possibility that it is was an oversight during my manual template porting.

And I'm going to take the "not as bad as I'd expect" as a compliment, because I spent dozens of hours whittling down the overriding styles to only the rules that actual affected anything so that upstream changes to bp-legacy would trickle down. I'm glad someone noticed, because that was exactly zero fun lol.

Also, it would be helpful to have a "really old" custom cbox child theme that is actually encountering these issues to test with, instead of trying to roll one that is "supposed" to break.

[boonebgorges]

Thanks for your initial thoughts, Marshall. Regarding your distinction, most of the issues I'm thinking of here are not specific to infinity/cbox at all, but fall into your first category. In the case of BP at large, when 1.7 introduced theme compatibility, we wrote up some guides and let people transition their themes themselves. But in this case, we're forcing the transition on people, which is why I want to try to offer more of a helping hand.

And I'm going to take the "not as bad as I'd expect" as a compliment

So intended :)

Also, it would be helpful to have a "really old" custom cbox child theme that is actually encountering these issues to test with, instead of trying to roll one that is "supposed" to break.

Yeah, this would be helpful. @vothsco Are you familiar enough with any authors of cbox-theme-based sites to request a copy of their child theme from them? (Assuming some of the ones you've showcased are using a child theme.) Certainly, we can reach out to ML.

[r-a-y]

We can test the MLAA's CBOX Child theme:
https://github.com/mlaa/cbox-mla

[MrMaz]

I chewed on this for a while, and I'm leaning towards spending the time on good documentation of the template migration steps. Tinkering and testing programmatic solutions could turn into a protracted effort to abstract the problem away temporarily with a layer of code, which in the end, still might look less than great (@boonebgorges alluded to this as well).

We are definitely forcing people to change, but exactly when to change is at their discretion, since the parent theme (cbox-theme) doesn't necessarily have to be upgraded when CBOX is upgraded. Unless I am missing something?

I like @r-a-y's idea to test against the MLA child theme. I can fork their theme and go through the migration steps and create documentation. If any of the steps turn out to be unreasonably difficult or time consuming, then we can tackle them individually as opposed to a one-size-fits-all blanket type solution.

Will wait for others to weigh in before investing any time.

[boonebgorges]

We are definitely forcing people to change, but exactly when to change is at their discretion, since the parent theme (cbox-theme) doesn't necessarily have to be upgraded when CBOX is upgraded. Unless I am missing something?

That's an interesting point. Another possible strategy is: after commons-in-a-box is updated, if we detect that the site is running a child of cbox-theme, show a big warning message near the prompts to update the theme.

[christianwach]

I have a number of differently-built cbox child themes. I'm away at a couple of conferences this week but I'll test the upgrade process on them next week and report back.