Twentyfifteen BP Companion Stylesheet

[hnla]

We have a number of issues with BuddyPress rendering it's screens in Twentyfifteen, we need to address some of the more prominent ones.

The best approach to this will likely be to create an additional sheet enqueued in buddypress-functions.php and conditional on checking if twentyfifteen theme is active.

Styles / rulesets should be minimal, this isn't a re-style rather a corrective measure to ensure we display at our best.

Special attention should be given to any changes to check whether they actually are not best fed upstream to our main styles, we don't want to add styles that are countering something that could be changed for all themes.

@lead devs we'll need a new blank file created and linked to from buddypress-functions.php that can be patched ?

Moving forward we might consider this approach for all future themes in the Twenty* family as it allows / affords us the opportunity to showcase BP at it's best for the main WP themes.

[r-a-y]

Could be cool if we checked for a stylesheet named after the active theme in bp-legacy.

01.patch is an attempt at this and also comes bundled with twentyfifteen.css, which includes some of the fixes from #6088.

Question: Should the asset check use get_template() or get_stylesheet()? I've opted for get_stylesheet() for now.

[hnla]

@r-a-y yes would have though get_stylesheet() as we'll need to allow for overloading I guess, but that ought to be a safe choice?

[hnla]

@r-a-y Patch looks good. Thanks for porting @mercimes fixes over, this gives us a good start.

[mercime]

One issue is the navigation slide dance when you hover over the last link. Animated gif attached.

[mercime]

navigation slide dance

[boonebgorges]

r-a-y's patch seems like a good start for tackling the issue of theme-specific stylesheets from a technical point of view.

From a policy point of view, I've long been nervous about the prospect of making theme-specific changes in BP. For one thing, it's potentially a slippery slope regarding which themes we officially support. (The Twenties are a given, but what about, say, P2?) Perhaps more importantly, theme-specific stylesheets seem like an easy way to cheat: throwing little fixes into twentyfifteen's stylesheet is far easier than examining whether the declarations in our main stylesheet need revisiting. So, I think we should have a discussion about the process before jumping in.

All of that being said, I think it's time for us to go down this road, so let's do it for 2.3.

[DJPaul]

To throw a different perspective in on this suggestion, if we end up doing this, I would like the purpose of these theme-specific CSS files to be to (aggressively) style BuddyPress for that specific theme so that it ends up looking amazing.

[feedmymedia]

Child theme for BuddyPress compatibility.

[feedmymedia]

Hello. I have uploaded a child theme with BuddyPress compatibility, the Git repository can be found here - ​https://bitbucket.org/feedmycode/twentyfifteen-child/ - and the dev site here - ​https://twentyfourteen.feedmybeta.com/

My development team did this work in December, we are now all back at work, but the last couple weeks since we reopened have been busy. I would like to get started again later this week or early next week with further work on CSS. It would be great if you could provide guidance on what we should focus on.

[hnla]

@feedmymedia Thanks for uploading we'll run the sheet and see how things are looking and provide feedback where we can.

[feedmymedia]

Sorry demo site is here - ​https://twentyfifteen.feedmybeta.com/

[Prometheus Fire]

I heard this was on the roadmap when @jjj mentioned in the WordSesh talk so I came to track this down. This is a part of BP that I can actually contribute work on!

I'm going chime in here in support of @djpaul's suggestion that the theme-specific styles should look amazing, as opposed to just fixing the layouts so they work.

This is something wanting to do anyhow, and I'm going to look toward making a contribution here in the coming weeks. The argument for aggressive design here is that the days of BP Default are behind us and it is important to showcase how good BP can be. It really should have unique styles that integrate nicely with the existing elements of each default theme. Each of the themes here (the twentys) has some unique challenges that would interesting to take on.

Perhaps, most importantly, a lot of good, and often subtle, design work has gone on for the default themes. Those unique elements should be carried over into BP so that BP looks and feels organic inside of those themes. Little things like, the fact that the styles/layout of the blog comments serve as a good template for individual updates in the activity stream. Right now, elements like this are very divergent. An aggressive styling doesn't have to mean flashy, but it can certainly describe a push to make BP and the default themes organic together.

[hnla]

@Prometheus Fire That be great if you're willing to help out with theme specific supporting/complementary sheets.

I too agree to an extent that we have the opportunity to fashion BP to look at it's best with the WP themes as I mention in my closing paragraph on the ticket:

Moving forward we might consider this approach for all future themes in the Twenty* family as it allows / affords us the opportunity to showcase BP at it's best for the main WP themes

I'm not a fan, though, of the phrase aggressively :); these complimentary styles should be subtle and aim to bring elements of BP component display into parity with the WP themes handling of similar elements i.e comment forms etc so the BP elements look as though they are a seamless part of the theme, much as you say in your last paragraph.

The original intent of this ticket was and is to ensure that we do address those WP themes that do present layout issues as we saw with twentyfifteen and that we have an agreed programatic approach to calling these complimentary sheets or overloading stylesheets if a given theme is detected, a process that may pose it's own issues requiring solutions; if we can take this notion of ensuring we present well with the WP themes and extend that to really working well with that theme, it'll be great.

I suggest we have a few things to think about:

1/ How many WP themes we would support, all? All as an ongoing process working back from latest/current WP theme?

2/ Do we follow the principle of supporting sheets complimenting our primary stylesheet or detect theme and swap out/in a new sheet for specific theme

3/ If sites already have layouts and styles based on a stock WP theme and BP what if any impact would a newly introduced set of styles have and if having a potential detrimental effect what, if any, measures can we have in place to mitigate that.

[boonebgorges]

All of the above sounds great to me. In this context, I interpret "aggressively" as meaning that we don't have to worry about having a minimal amount of CSS, or making the CSS have a minimal effect beyond the bare minimum, as we do in the case of bp-legacy.

1/ How many WP themes we would support, all? All as an ongoing process working back from latest/current WP theme?

Yes, I'd say we should go with this strategy. Twenty Fifteen would be the first priority. We'd only ship a theme's companion stylesheet in a version if we thought it was ready.

2/ Do we follow the principle of supporting sheets complimenting our primary stylesheet or detect theme and swap out/in a new sheet for specific theme

I'd lean toward the former, as it'll mean a lot less repetition throughout the stylesheets, leading to easier maintenance. If it would help to break up bp-legacy stylesheets into smaller bits for this purpose (then joined using a build process?), I'd be supportive of that (I guess this would suggest your latter alternative, but without the repetition in core stylesheets).

[hnla]

So to progress I think we need to ratify the following:

• WP Theme support

Yes, I'd say we should go with this strategy. Twenty Fifteen would be the first priority. We'd only ship a theme's companion stylesheet in a version if we thought it was ready.

This would be my view as well. We can then work on previous themes as and when people are wiiling and available to take on as a mini project.

• Stylesheet inclusion process & sheets

I think r-a-y's patch pretty much covers requirements here. We check on theme activated and include our stylesheet twentythirteen.css

I considered whether we ought to maybe have some form of user selection, but it made little sense in view of the fact that these new theme specific styles are intended to be working to harmonize BP elements with the WP theme activated then they would be included by default, for those wishing to further customize the look of their site/theme they can still work with the principles in place to overload any BP template specific file to the theme.

I'd lean toward the former, as it'll mean a lot less repetition throughout the stylesheets,

I agree, Sheets will be as complimentary to BP default stylesheet, we have run trough the process of reducing as many design elements in the default sheet in previous iterations and in the last release we reviewed font sizing so our stylesheet is pretty lean and a pretty good foundation to build design themes/styles on, I did consider the notion of perhaps breaking out our default to individual parts as an interesting approach but when I thought it through it felt as though we would be creating a fair amount of work to do for not a huge gain and not the simplest process to identify the rulesets that would exist in those new module sheets.

If we agree these points then we can start to accept sheets for consideration and testing?

[Prometheus Fire]

I started doing some work on 2015. It is interesting how many little problems there are throughout all of it. On every page, I notice something that doesn't quite work right out of the box.

Here is kind of a first pass at getting the navigation to work. When designing a new BP site in any theme, I always find this the hardest to style in terms of decision making and how it should look.

Anyhow, I would strongly recommend that we keep this ticket as a discussion only for the theming of 2015 and open a new ticket for the overall technical/theoretical discussion, otherwise this ticket is going to be full of talk about design decisions in addition to the theoretical and technical stuff about how to deploy it into BP.

[Prometheus Fire]

I've created an umbrella ticket for the overall discussion and tracking of the default theme support #6124

[hnla]

Technical discussions and theme ticket branching in #6248

[hnla]

Updated stylesheet for BP section formatting.

[hnla]

Twentyfifteen-comp-style.patch provides twentyfifteen.scss & twentyfifteen.css compiled.

Compiled file is in dev mode uncompressed. If we do run with pre-processor files for this exercise we'll run a final production compile to compressed output on commit?

[boonebgorges]

If we do run with pre-processor files for this exercise we'll run a final production compile to compressed output on commit?

My thinking was that we would only compile on the 'build' task, for purposes of release. Anyone who is running a git/svn checkout would have to compile themselves for the purposes of testing. We can set up a grunt watch task like core has, to make this easier for devs.

[hnla]

@boone Ok wasn't sure what was possible by way of compiling the file and as we would need to compile locally during development included the compiled file by default. So moving forward the patch can stand, however subsequent updated patches need only include the Sass file, which works for me, my only thought on that is the necessity for anyone wanting to test or view the work having to first run a compile to obtain a working file but guess every one is setup locally to handle that.

[boonebgorges]

IMO we should not be checking compiled CSS into the repo at all - fine to include it with the patch for testing/review, but let's make sure to leave it out. Someone will need to write a grunt task for compiling this file (maybe grunt thememods?), and ideally a watch task as well. We can probably handle this in a separate ticket - maybe #6124 - but let's keep it in mind before any commits happen here.

[hnla]

@boone gotya - yes we'll record the details/process for this aspect on #6248 for tech points.

[DJPaul]

Replying to boonebgorges:

If we do run with pre-processor files for this exercise we'll run a final production compile to compressed output on commit?

My thinking was that we would only compile on the 'build' task, for purposes of release. Anyone who is running a git/svn checkout would have to compile themselves for the purposes of testing.

I kind of disagree with this. What I like about our trunk is that anyone can check it out, and they can start developing with it immediately. None of this build tool rubbish: people don't ever *have* to run npm install.

I know that statement is a bit simplistic and there are edge cases (around the type of work and the contributor's experience level), but if we add a SASS file in src without also providing a complied version, this adds two extra steps to the "How can I grab a dev BP build and start hacking on it" process (otherwise you'd waste time figuring out why the CSS isn't working on your site, etc).

Which I think would be a shame to lose. I want to make clear I am not against this course of action but wanted to make sure the consequences were clearly spelt out.

[hnla]

@DJPaul, @boonbgorges

I guess this may not be so much of an issue; my patches contain both .sass and a compiled .css uncompressed dev version, if those are eventually comitted to /src/ people have a compiled version that will load if they checkout trunk, on whatever process runs the build can that not re-compile the .sass file to compressed output in the .css file for the zip/install packages?

[hnla]

For the purposes of keeping things clean I've opened a new ticket to handle the specific 2015 task (rather than use this ticket for 2015 styling) and will follow same format for subsequent twentysomething tasks

• Twentyfifteen specific task #6291
• Companion styles tasks technical discussions #6248

[boonebgorges]

Replying to DJPaul:

Replying to boonebgorges:

If we do run with pre-processor files for this exercise we'll run a final production compile to compressed output on commit?

My thinking was that we would only compile on the 'build' task, for purposes of release. Anyone who is running a git/svn checkout would have to compile themselves for the purposes of testing.

I kind of disagree with this. What I like about our trunk is that anyone can check it out, and they can start developing with it immediately. None of this build tool rubbish: people don't ever *have* to run npm install.

I know that statement is a bit simplistic and there are edge cases (around the type of work and the contributor's experience level), but if we add a SASS file in src without also providing a complied version, this adds two extra steps to the "How can I grab a dev BP build and start hacking on it" process (otherwise you'd waste time figuring out why the CSS isn't working on your site, etc).

Which I think would be a shame to lose. I want to make clear I am not against this course of action but wanted to make sure the consequences were clearly spelt out.

I don't feel very strongly about it. In a purist sense, keeping compiled assets in /src/ is not good practice (we don't do it for minified JS files, or for RTL files, etc). At the same time, I don't want to raise barriers for contribution. If others think that needing to compile Sass is an unreasonable hurdle, then I'm fine being overruled on this point. I will note, though, that if CSS is checked into the repo, then you're going to get contributions in CSS, which will mean a bit more work for committers/reviewers, since it'll have to be translated back into Sass eventually.

[netweb]

Replying to boonebgorges:

I don't feel very strongly about it. In a purist sense, keeping compiled assets in /src/ is not good practice (we don't do it for minified JS files, or for RTL files, etc).

For RTL CSS you do, see #5622

Replying to boonebgorges:

...I will note, though, that if CSS is checked into the repo, then you're going to get contributions in CSS, which will mean a bit more work for committers/reviewers, since it'll have to be translated back into Sass eventually.

This was one of the reasons RTL CSS was kept in /src so that (paraphrasing) "Contributing updates/fixes to RTL CSS easy as possible", I can't think of the last time a ticket/patch was submitted based on RTL CSS directly.

[netweb]

6124.03.patch is a refresh of 6124.03.patch without the twentyfifteen.css

[hnla]

Requesting a review of the 6124.03 patch with a view to committing by whomever has a moment: @jjj @boonebgorges @DJPaul

[boonebgorges]

In 9694:

In bp-legacy, load theme-specific stylesheets if found in the stack.

When enqueuing stylesheets, bp-legacy will look for files called
{stylesheet}.css, where {stylesheet} is the name of your current theme. This
will enable bp-legacy to ship with supplementary stylesheets that target
specific themes, such as WP's twenty* default themes.

Props r-a-y.
See #6248, #6124.

[boonebgorges]

[9694] contains the changes in 6124.03.patch​.

We're only loading stylesheets for get_stylesheet(). This means that if you have a twentyfifteen child theme, bp-legacy won't load twentyfifteen.css. This is probably OK - anyone building a child theme probably expects to have to do their own work, and in any case they can easily enqueue our stylesheet if they'd like - but it should probably be documented.

I'll leave it to someone else to determine whether this ticket or some other one should be closed as fixed.

[hnla]

Yes we're loading the stylesheet based on using get_stylesheet() to find the theme name so the problem will be building the file handle for enqueueing based on that and a child theme being essentially differently named from our physical file name.

So currently in theory the stylesheet could be overloaded to the main theme i.e 2015 and will be loaded but if overloaded to a child theme would need to be renamed to match child theme name under a suitable bp directory.

I think this is the way we'll have to play things, we could change to get_template() then would load twentyfifteen.css in a child theme based on twentyfifteen if that wasn't wanted then it could be overloaded or de-registered.

I guess I can document this in the stylesheet - explain in header comments that child themes will need a copy of the file in the path described and re-named to the child theme name.

I can't decide what's the better option, it could be the case that the user simply needs to overload a template or two for simple changes then as is they would need to overload the stylesheet and re-name it; is this too much to ask?

[boonebgorges]

I think this is the way we'll have to play things, we could change to get_template() then would load twentyfifteen.css in a child theme based on twentyfifteen if that wasn't wanted then it could be overloaded or de-registered.

Right. Technically, it's not an issue - we can use get_template() to encompass child themes and parent themes in one fell swoop. The question is whether we *want* to do this. I'm leaning toward "yes" - it seems to me that in most cases, child themes will want BP to apply the same theme-specific fixes as would be applied for the parent theme - but I don't feel strongly about it.

[hnla]

The question is whether we *want* to do this. I'm leaning toward "yes"

Hmm in that case then I would have to agree with 'yes', we load the stylesheet in all theme/sub themes, this is better for the majority of cases, users not requiring those styles are likely to be the more proficient user and their option is simply to de-register the stylesheet or use it for their own styles.

I'll patch for use of get_template().

[hnla]

Use get_template instead of get_stylesheet - let child themes inherit the complimentary stylesheet.

[johnjamesjacoby]

​6124.04 makes sense to me. The stylesheet/child-theme name is a less appealing override IMO, at least for 2.3.

[r-a-y]

05.patch supports RTL for companion stylesheets.

Patch also tweaks how buddypress-rtl.css is loaded. We shouldn't unnecessarily look for buddypress-rtl.css if RTL is not in use.

[hnla]

In 9882:

Companion styles stylefile enqueueing

Commit updates stylesheet paths in buddypress-functions.php from get_stylesheet() to get_directory() to ensure child themes inherit the companion stylesheets.

Commit also updates rtl checking & enqueueing only if rtl is in use.

Fixes #6124 Props r-a-y

[r-a-y]

In 9883:

Follow-up to r9882.

See #6124.