wpsharks / s2clean

s2Clean is a premium product. This public repo is for issue tracking only.
Other
1 stars 0 forks source link

bbPress not supported at all #35

Open raamdev opened 8 years ago

raamdev commented 8 years ago

At present, the s2Clean Pro theme does not support bbPress at all. Here's what shows up on the default /forums/ URI when bbPress is installed with a couple of forums:

2015-07-23_11-47-02

By contrast, even the basic Twenty Fifteen theme included with WordPress supports bbPress "out of the box":

2015-07-23_11-49-11

raamdev commented 8 years ago

@jaswsinc I'm looking into this issue and it appears that this may require less work than I first thought. bbPress is designed to work with any theme that follows standard WordPress theme practices. s2Clean does not--hence the reason /forums/ returns an "Archive" page instead of the bbPress forums--but other pages, such as /forums/forum/example-forum/ and /forums/topic/example-topic/ seem load the proper bbPress content with s2Clean right now (i.e., without any changes). If we can get the standard bbPress theme integration working the way it does with most WordPress themes, everything else related to bbPress should just work "out of the box".

I'm trying to work out exactly what needs to be changed in s2Clean to make the bbPress theme support "just work", the way it does with most other WordPress themes.

The bbPress plugin includes its own theme support via theme template parts in plugins/bbpress/templates/default/ and then applies the necessary content templates (and CSS styles) by hooking into the WordPress filters API (pretty cool, actually!). Here's where the various bbPress theme packages are set up.

What I'm having trouble figuring out is where s2Clean is overriding the "default behavior" that bbPress relies on and which makes bbPress → WordPress theme integration possible. Any pointers?

I realize that we could add a copy of all the necessary bbPress templates to the s2Clean theme itself, and then call each template part when appropriate, but I'm trying to avoid doing that so that any changes to those default bbPress theme files in future versions of bbPress do not need to be updated in s2Clean but instead just automatically get upgraded when bbPress itself is upgraded.

jaswrks commented 8 years ago

... s2Clean does not--hence the reason /forums/ returns an "Archive" page ... ... where s2Clean is overriding the "default behavior" that bbPress relies on

ha. I think you're operating with the assumption that bbPress is backed by and produced by WordPress, so it is we who are doing things wrong, and they are doing things right. That's a fair assumption to make. However, it is wrong in this case.

bbPress theme "hacks" (a much better term to use IMO, because they are not elegant), will only work if your theme contains a very basic set of WordPress template tags that don't do a lot of standards checking. s2Clean follows WP standards strictly. Thus, we need some tweaks that bend the rules a bit in order to consider bbPress theme hacks.

In order for bbPress theme hacks to work, your theme must not check is_singular() before rendering a template, because is_singular() returns false (as it should), but bbPress theme hacks rely on the_content(), even when is_archive() && !is_singular(). This is one reason for the is_bbpress() function to exist. The is_bbpress() function can be used to identify this design flaw; which breaks away from WP standards.

To clarify this point, you can test it out by loading /forums/ on a site running bbPress. Run print_r($GLOBALS['wp_query']) or call is_singular(), and you will see that the query itself is not a singular query, and that is_singular() returns false given that's it's not a singular query. It is_archive(), and it's a post type archive. Therefore, s2Clean deals with this properly. If it were any other post type archive (i.e., not bbPress forums), it works as expected. So it's bbPress that breaks away from the standard, in order to accomplish a default-theme hack.

To be fair, they don't have much of a choice. Getting this sort of integration to work within the limitations of what a Post Types API can offer—that is challenging. So they are implementing a hack which works in a majority of WP themes; i.e., those that don't follow standards strictly; with respect to is_*() checks. In addition, they need to consider themes that do implement a bbPress sub-theme of their own, in which case this is not an issue, because standards can be followed then.


Suggested Next Actions

Overview ...

Make use of the is_bbpress() function to help identify this problem in code and work around it.


Step 1

After this line of code, add the following.

if(!function_exists('\\'.__NAMESPACE__.'\\is_archive'))
{
  /**
   * @return boolean TRUE if `is_archive()`.
   */
  function is_archive()
  {
    if(function_exists('is_bbpress'))
      return \is_archive() && !is_bbpress();

    return \is_archive();
  }
}
if(!function_exists('\\'.__NAMESPACE__.'\\is_singular'))
{
  /**
   * @return boolean TRUE if `is_singular()`.
   */
  function is_singular()
  {
    if(function_exists('is_bbpress'))
      return \is_singular() || is_bbpress();

    return \is_singular();
  }
}

This will override s2Clean's own calls to is_archive() and is_singular() inside the s2clean namespace; i.e., in an isolated way to bend the rules a little bit here.


Step 2

Confirm that this resolves the issue with /forums/ when running s2Clean.


Step 3

Review the is_bbpress() function carefully and determine if any other special considerations need to be made in other parts of s2Clean. I haven't taken a good hard look at this myself yet.


Step 4

... I'm not sure what comes next in your plan.

You might consider that what I posted above is a quick hack. If might work better over the long term if we went through the entire s2Clean codebase at some point and did very specific checks for is_bbpress_*() instead. For instance, we could add a theme-specific ->is_bbpress() method in s2clean-pro/includes/functions.php and then call upon this in various places; i.e., instead of hacking the is_archive() and is_singular() functions.

jaswrks commented 8 years ago

Tip: When trying to understand how s2Clean works internally, in terms of it's integration with WordPress, always start your journey from /s2clean-pro/index.php

Some points to make about this file:


Tip: Aside from "template" files, the meat of s2Clean is located in includes/functions.php. This file should look somewhat familiar to you, because it is not that far away from the design that ZenCache has.


Tip: Instead of starting from the 000000-dev branch, please start from the websharks branch. s2Clean has been moved around from one developer to another over the past months, so I broke away from this and started a branch of our own (for our sites), and I have been maintaining that branch myself. Thus, it is the most stable up-to-date branch, and we should work from that branch when integrating bbPress. Referencing: https://github.com/websharks/s2clean-pro/tree/websharks

We can merge that into 000000-dev now or later; whatever is easiest. In the recent past, I was just trying to leave the 000000-dev branch open and free from my own mods for bit, in order to give someone else a chance to work as lead on this project—w/o me interrupting.