Opened 10 years ago
Closed 10 years ago
#6457 closed defect (bug) (fixed)
buddypress.css stylesheet overloading behavior change in 2.3-RC1
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 2.3 | Priority: | high |
Severity: | blocker | Version: | |
Component: | Appearance - Template Pack | Keywords: | has-patch |
Cc: |
Description
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?
Attachments (2)
Change History (17)
#1
@
10 years ago
- Milestone changed from Awaiting Review to 2.3
- Priority changed from low to high
- Severity changed from minor to normal
#3
@
10 years ago
- Severity changed from normal to blocker
I think as this is a regression from 2.2, we need to fix it.
#4
@
10 years ago
To be clear, this appears to only happen when running BuddyPress from the /src
directory of trunk.
Is that correct?
#6
@
10 years ago
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.
#7
@
10 years ago
6457.02.patch proposes the folliowing:
- Abolish
BP_SOURCE_SUBDIRECTORY
constant, and useBP_LOAD_SOURCE
instead - Set
BP_LOAD_SOURCE
when loading from/src
inbp-loader.php
- Overload
plugin_dir
andplugin_url
inBuddyPress::setup_globals()
withsrc
orbuild
based on state ofBP_LOAD_SOURCE
- Add two new loops to
BP_Legacy::locate_asset_in_stack()
: one to add matching minified locations, another to checkbp_get_template_locations()
rather than a hard-coded array
This should address the following:
BP_SOURCE_SUBDIRECTORY
was poorly named and duplicated the functionality ofBP_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
#8
@
10 years ago
@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.
#9
@
10 years ago
@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.
#10
@
10 years ago
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.
#11
@
10 years ago
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?
#12
@
10 years ago
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 six 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.
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.