Skip to:
Content

Opened 6 months ago

Last modified 3 weeks ago

#3218 reopened defect

Change in behavior of CSS inclusions

Reported by: scmsteve Owned by: johnjamesjacoby
Milestone: 2.6.1 Priority: normal
Severity: normal Version:
Component: Appearance - Theme Compatibility Keywords: needs-patch
Cc:

Description

I don't know if this is classed as a bug, but rather a point of discussion. It is a change from 2.5 and will potentially break sites upgraded from there.

Previously we had a css/bbpress.css file in our child theme, and it was included. It looks like the v2.5 logic was to include bbpress.css and, if on an rtl site, to include bbpress-rtl.css.

Now it is doing logic and ending up with only one file to be included based on rtl and DEBUG flag... If RTL it adds an -rtl to bbpress, and if DEBUG is not set it adds ".min." before css. So you would get only one file included, either bbpress.css, bbpress-rtl.css, bbpress-rtl.min.css or bbpress.min.css.

This breaks 2.5 behavior where you either had enqueued both bbpress.css and bbpress-rtl.css if on an rtl site.

Is it posibble to check for a bbpress.css if bbpress.min.css does not exist on a site, ignoring the -rtl options for the moment? Sites coming from 2.5 as we did will break as it is now.

I had commented in the thread here:

https://bbpress.org/forums/topic/bbpress-2-6-beta/page/4/#post-194632

Someone earlier had mentioned it broke for them as well.

Or, do you just want to document this somewhere as a known break with past behavior and require a .min. version of the css for a non-debug site?

Change History (4)

#1 @johnjamesjacoby
6 months ago

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

In 6862:

Theme Compat: introduce bbp_locate_enqueueable() and bbp_urlize_enqueueable().

These functions are used to help make locating enqueueable assets easier, and use bbp_locate_template() interntally, now accepting an array of files.

In addition, bbp_locate_enqueueable() also internally juggles minimized file variations, and stacks them according to the SCRIPT_DEBUG constant. This ensures that both minimized and unminimized file variants are in the array in the preferred order.

This fixes a regression between bbPress 2.5 and 2.6 caused by the bundling of minimized assets in theme compatibility, and ensures that sites with their own bbpress.css files in their own locations will continue to get loaded, regardless of the SCRIPT_DEBUG setting.

Fixes #3218.

#2 @scmsteve
5 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This didn't fix the problem exactly, because if the child theme did not have a .min. version it would not use the child .css version but would instead prefer the default .min. version. I changed with @johnjamesjacoby about this but I don't think it was resolved.

Also, the need to entirely replace/replicate the .css in the site theme makes updating quite problematic. Would it be possible to extend this in some way so that the site's version would be loaded after the default version? Maybe this behavior isn't full supported, but in my brief testing the style are applied in the order they are loaded, overriding previous ones, so if the default .css were loaded and then the site theme customization file was loaded after the fact, wouldn't it override just those elements?

This would definitely make site changes more evident and easy to maintain, especially when doing version upgrades. Right now it is somewhat of a pain to figure out what may have changed between v2.5 and our site version, and then what changed between v2.5 and v2.6, and then to fit our site changes back into it. If we only had to be concerned about the exact parts we were needing to override it would be cleaner. :)

#3 @scmsteve
5 months ago

I implemented a "fix" here, or at least a workaround.

Copying from my notes:

In reading the wp_enqueue_style() call, it has the ability to specify an array of items that you depend on. So, it would seem, we could call this in our theme, specifying 'bbpress-default' as a depend-upon, and it would load our smaller customization css file after them. As long as I tested that we had defined them in the same or more restrictive case, they seemed to override the earlier definitions.

So, implemented by putting this in functions.php:

/* BBpress overrides */
wp_enqueue_style ('bbpress-scm',
    get_stylesheet_directory_uri() . '/css/bbpress-ours.css',
    array( 'bbp-default')
);

And then including just our customizations in a css/bbpress-ours.css file in our theme. Just putting this out as an alternative approach to not having to maintain the entire duplicated/customized css file. I suppose I could be missing something, so if this is doomed to failure let me know. :)

#4 @johnjamesjacoby
3 weeks ago

  • Component changed from General to Appearance - Theme Compatibility
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 2.6.1

@scmsteve I meant to reply earlier, but forgot. Sorry about that.

Your suggested alternative is definitely another way to go. Probably better, to keep your CSS outside of what bbPress would normally want to auto-enqueue.

The bbPress core way of auto-enqueuing CSS is designed mostly to provide convenience to themes, so they don't need to worry about their own enqueueing, and just let bbPress look for it and use it.

We could include another file in the stack, with a filename that doesn't already exist. The end result would be similar to what you've accomplished here, and load bbpress-extras.css or something after the fact.

Moving to 2.6.1, because I have a gut feeling this change might negatively affect someone upgrading from 2.5 to 2.6, but don't have any evidence to support that just yet.

Note: See TracTickets for help on using tickets.