buddypress.css stylesheet overloading behavior change in 2.3-RC1

[dcavins]

So this old trick has worked since BP1.7: Add an empty file called buddypress.css at my-theme/css/buddypress.css to prevent BP from loading its own stylesheet from bp-legacy.

​https://codex.buddypress.org/themes/theme-compatibility-1-7/add-buddypress-styles-to-a-theme/

DJPaul made a change in r9630 that allows BP to use minified assets when available, but since BP is looking for the minified version, the old trick fails. In this brave new minified world, the theme dev needs to add my-theme/css/buddypress.min.css to get the old behavior back.

This could be surprising for theme developers. We have no idea how many sites this will affect (probably not very many?), but is there any better way to keep the behavior consistent? Or maybe updating that old article and writing a new post about best practices would be the better answer?

[r-a-y]

01.patch partially reverts r9630, which should fix this issue.

This removes the .min string from the $file variable only for parent and child theme lookups, not bp-legacy.

[DJPaul]

I think as this is a regression from 2.2, we need to fix it.

[johnjamesjacoby]

To be clear, this appears to only happen when running BuddyPress from the /src directory of trunk.

Is that correct?

[johnjamesjacoby]

Related: #5652, #5963, #6213,

[johnjamesjacoby]

In short, if we want to support /css/buddypress.min.css it should be in addition to /css/buddypress.css and not in place of. To do this, will require an additional loop to recalculate the $locations and insert the minified asset for each location.

We can do this relatively intelligently, by comparing whether the $file parameter matches the $file parameter after it's been stripped of its .min name part or not.

Suggested code-change imminent. Please test it to confirm it addresses all scenarios.

[johnjamesjacoby]

​6457.02.patch proposes the folliowing:

• Abolish BP_SOURCE_SUBDIRECTORY constant, and use BP_LOAD_SOURCE instead
• Set BP_LOAD_SOURCE when loading from /src in bp-loader.php
• Overload plugin_dir and plugin_url in BuddyPress::setup_globals() with src or build based on state of BP_LOAD_SOURCE
• Add two new loops to BP_Legacy::locate_asset_in_stack(): one to add matching minified locations, another to check bp_get_template_locations() rather than a hard-coded array

This should address the following:

• BP_SOURCE_SUBDIRECTORY was poorly named and duplicated the functionality of BP_LOAD_SOURCE
• BP_LOAD_SOURCE is now set when the environment is as such
• DIR and URL are modified just-in-time based on BP_LOAD_SOURCE state
• Assets will attempt to locate both minified (new) and raw (old) sources
• BONUS: Assets will now also attempt to locate assets according to filters attached to bp_get_template_locations() results

[hnla]

@jjj The issue was with the build rather than src, in source the files are correctly overloaded - adding buddypress.css to the theme or child theme will be loaded in preference to the core legacies .min version.

It's this behavior we need to revert to, not necessarily being able to also run overloaded .min copy, users overloading to create new or modify are less likely to run the file as .min.

It's a bit late now :) but I confirm the issue having run build on the src files neither buddypress.css or the new twentyfifteeen.css files will overload the core min versions.

N.B writing this as noting the previous patches appearing but haven't had chance to test those.

[hnla]

@jjj trying to test patch but running build is failing on paths - are there steps to follow other than running build, and commenting out define('SCRIPT_DEBUG', true); enabled to run /src/? At the moment it tries to find the files in a subdirectory /build/ which the files are all running from already.

[hnla]

I've had to update bp-loader in the build dir from:
$this->plugin_dir = trailingslashit( constant( 'BP_PLUGIN_DIR' ) . $sub_directory );

to manual string for subdirectory:#
$this->plugin_dir = trailingslashit( constant( 'BP_PLUGIN_DIR' ) . 'build' );

Not sure why $sub_directory is returning empty... Just spotted why:

if ( defined( 'BP_LOAD_SOURCE' ) ) {
 if ( true === constant( 'BP_LOAD_SOURCE' ) ) {
 $sub_directory = 'src';
 } else {
 $sub_directory = 'build';
 }
}

we're nesting the $sub_directory = 'build'; in the if ( defined( 'BP_LOAD_SOURCE' ) ) is defined which of course it isn't if we have a build directory.

[hnla]

Testing patch:

Running from /build/

We are able now to overload files as .css, so I can overload both buddypress.css & twentyfifteen.css in the theme.

However it doesn't look as though we're loading minified assets from core for some reason nor can I overload them from the theme, so simply not seeing them to load?

[r-a-y]

If we want to do 02.patch, let's wait until 2.4.

01.patch gets us back to what we were doing since 2.1.1 (#5888), which is not looking for minified assets in parent and child themes.

The main issue I have with 02.patch -- looking for another .min asset in the stack -- is this adds a maximum of two more file_exists() calls for each asset check. Multiply that by 4 if we count every single call to locate_asset_in_stack(). Multiply that by 6 if the site is RTL.

Most themes will not be overriding our assets with a minified CSS file / JS file. If there are themes that do, they probably will have already made this change in their build environments since we've added asset stack checks since v1.9 (#4949). In this case, we'd be adding another .min asset check for no reason.

If we're getting rid of the BP_SOURCE_SUBDIRECTORY constant, we might need to deprecate it. Though, this isn't that big of a deal because this only affects development installs.

[johnjamesjacoby]

I agree that the additional file_exists() checks are not ideal.

I'll commit 01.patch for 2.3.

[johnjamesjacoby]

In 9908:

Theme Compat: Revert part of r9630.

Restores previous behavior of allowing buddypress.css to not be minified in the asset stack.

Fixes #6457. Props r-a-y.