Add a configuration setting for when to load custom CSS

added needs:concept approval triaged type:proposal labels

I think this would be a pretty great idea in general :)

Is there any reason for not wanting the custom css to be included in the public civicrm pages though? After all civicrm.css and civicrmNavigation.css are always included on those pages, so why would you not want that additional stylesheet there too?

Like @AkA84 I'm curious as to when you wouldn't want that file included when all other CiviCRM resources are being loaded. Forgive my naivety, but isn't the solution to the "overridden fonts" problem, ahem, not to apply the sans-serif font stack to the body element.

@jensschuppe It seems to me that your patch tries valiantly to paper over a deeper problem, which is that the Drupal module at minimum loads core styles in the civicrm_initialize() function. This means even an API call will cause core styles to be added to the document <head>. Trying to discover when to override this behaviour is going to be a major headache to do consistently for all CMSes and all situations.

In fact, in WordPress I'm not sure calling CRM_Core_Menu::get($path) will provide you with the correct information anyway. See my comment on GitHub for some more details on how core resources are loaded in WordPress.

My recommendation would be to look more closely at the Drupal module and see if you can separate civicrm_initialize() and civicrm_html_head() such that civicrm_html_head() is only called when needed. I'd prefer to see less code, not more!

Reflecting on what I wrote above, perhaps I could have been clearer.

I don't consider the loading of the custom stylesheet to be a problem for Core to resolve - I consider it a bug in the Drupal module. All Civi-related CSS and/or Javascript should only be loaded when a CiviCRM "entity" is being rendered in the browser - not every time civicrm_initialize() is called.

Unusually, Drupal needs parity with WordPress in this instance :-)

I still think the Shoreditch font problem is, as I said elsewhere, "a divergence from the aim to "contain" Shoreditch's CSS declarations to CiviCRM markup" and has a simple solution.

@haystack It seems very reasonable to review CiviCRM's way of loading its styles and scripts, as that is the very reason for subsequent problems with overridden style rules. Maybe consider my patch as a quick solution for treating symptoms in a backwards-compatible way.

@AkA84 A scenario for not wanting custom styles (e.g. Shoreditch's) is when using a custom theme for public CiviCRM pages. As we can not assume a theme handling all of CiviCRM's markup correctly, the admin should be able to choose between whether or not to include custom CiviCRM styles. We had a theme developed to include styles for CiviCRM's markup already, and everything was broken after starting to use Shoreditch.

@jensschuppe With respect, I believe your patch will introduce more problems than it solves. As I said, I am not persuaded that checking CRM_Core_Menu::get($path) will produce information of value in WordPress. See @totten's comments on the internal CiviCRM routes that are exceptions with respect to the menu XML data.

It seems to me that there are two stages to solving this:

Fix the Drupal-CiviCRM module which incorrectly triggers the loading of styles whenever civicrm_initialize() is called - which is responsible for custom styles being included even when a CiviCRM entity is not present on the page
Coerce Shoreditch to target only the markup it is responsible for and remove the style declarations that are outside its scope - especially those on the <body> tag

A third stage, which you allude to, is that it should be possible for modules/plugins/themes to prevent CiviCRM styles from loading, whether individually or collectively, or replace the assets with their own. I address this (for Shoreditch's internals) in this PR and have included sample code. If the same is required for other CiviCRM assets, then I suggest that a new hook is introduced which would put the responsibility for checking the conditions on to modules/plugins/themes instead of complicating Core. Doing so would allow your code to decide whether or not to include custom CiviCRM styles given the context.

Coerce Shoreditch to target only the markup it is responsible for and remove the style declarations that are outside its scope - especially those on the <body> tag

Just want to confirm that we're aware of the issues and we're working on it!

Thanks @AkA84 that's good news.

By way of an update on this, I've just pushed code to the CiviCRM Admin Utilities plugin master branch that gives you the option to disable the default CiviCRM stylesheet, the CiviCRM menu stylesheet, the Shoreditch custom stylesheet and the Shoreditch Bootstrap stylesheet on plugin's "Settings" screen. WordPress only, obviously.

What Drupal folks might be interested in is that this works without requiring any changes to CiviCRM Core - including this proposed change. This is partly because the CiviCRM-WordPress plugin separates civicrm_initialize() from the equivalent of civicrm_html_head() but also because it is easy to hook in just before the equivalent callback to civicrm_page_build() runs and disable the relevant resources before the region is rendered. I'm not sure how you'd make sure one callback runs before another in Drupal, but I'd assume there's a way. If so, then this resource-disabling code might be of interest.

Just adding two related PRs which are trying to solve this issue:

https://github.com/civicrm/civicrm-core/pull/12872 - Added new setting to Disable CiviCRM css on Frontend pages

https://github.com/civicrm/civicrm-core/pull/8523 - CRM-18792 - Create CSS theming subsystem

Will back link on each PR too to help people find this request.

@seamuslee @mattwire

added 1 deleted label

Nice crosslinks @justinfreeman

Also, a revised version of 8523 was requested during the sprint, so I posted: https://github.com/civicrm/civicrm-core/pull/12929

Awesome @totten - we'll be happy to review this one

Just to tag on here, I am coming to this thread after trying to track down why the Google Page Speed Insights show a very low mobile score (2%) with Shoreditch enabled, and why it jumps to a more normal level (87%) as soon as I disable the Shoreditch CSS.

https://github.com/civicrm/org.civicrm.shoreditch/issues/339

That's some heavy CSS to be loading on all of a site's pages, whether it's needed or not!

Thanks for all the work going on here to solve this.

Linking in another related issue.

Shoreditch - Changes styling of public-facing event info pages https://github.com/civicrm/org.civicrm.shoreditch/issues/387

@jamienovick said:

@AkA84 @Upperholme I think this may be a combination of issues actually manifesting itself.

I believe that CiviCRM does not differentiate between public and private pages in its CSS. In fact all CiviCRM CSS is loaded on public CiviCRM pages in general anyway. They just normally don't have a lot of styling so its less obvious. See here for discussion on this issue: #378 (closed)

This is then made worse as Shoreditch more aggressively styles the page and sometimes outside of the main CiviCRM divs. The buttons and and page margin/padding at the top is then affected as well on any page where CiviCRM is bootstrapped.

I think the long term solution to this is to solve 1 above but we have been short of time/budget to do so. I'm not sure if @Upperholme this is something you could help look at perhaps and help us progress?

Also note that @totten has indicated to me that next month he is likely to be looking at some theme related issues under some contract work he has and he may be better placed to weigh in then

mentioned in commit b203406b

mentioned in commit ff4f97ea

mentioned in commit 4ef562f6

Hello, finally managed to go back to this, so I was wondering if there is any update?

On Shoreditch's side we have removed the font family leak and we're currently working on decoupling any remaining drupal-related style from the theme, but unfortunately I think this won't be enough to fully address issues like this one until we can prevent in a reliable way the custom css, as good as namespaced it can be, to be injected into public pages (btw I have to image that this issue have been presenting itself with any custom css, not just with Shoreditch's?).

Over the course of the months different options had been suggested, so I wanted to ask is there now a proper way to fix it, or is one being worked on, or is this something that atm would better be tacked somehow by Shoreditch itself, etc

Thanks!

I think the documentation for the new settings is all merged in here https://docs.civicrm.org/dev/en/latest/framework/theme/

thanks @eileen , is this something that is not out yet? I can't find a reference to what version this system was introduced on in that wiki page

I tried on v5.15.1 to run

cv ev 'return Civi::service("themes")->getActiveThemeKey();'

and i get

[Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException]  
You have requested a non-existent service "themes".

@AkA84 I think it's merged into the rc (5.16) - we merged it in NY code sprint - but I would probably focus on fixing Shoreditch against the rc if I were you & tagging the shoreditch release as requiring that Civi version.

The way I manage things is that I release new tags of my extensions somewhat frequently but I also increment the Civi version they work against fairly often. That means I'm not writing something to cope with many variants of core - I'm working on the latest - but given that people on older versions of Civi already have a working extension version they have the choice of 'staying stable' or upgrading to get the new feature or improvement (in which case they have to update both).

In your case given the driver is to get the most out of your time before you leave I would definitely focus on doing the best fix you can rather than making it work on older versions - i.e possibly cut a release now (with the KAM fixes) & then work on this against the rc or master & when you tag the next release increment the civi version for it (older sites will still be able to use the version you release first).

thanks a lot @eileen for the help

I'm trying the new theming system on a local v5.16, and I have a couple questions (of course please let me know if I should ask them somewhere else):

It looks like a theme's civicrm.css file completely overrides the original civicrm.css that ships with core. Unfortunately this doesn't work well with Shoreditch, as our theme does not replace the original stylesheet but rather builds on top of it.

I can't seem to find a way to set up the theme to extend civicrm.css, is this scenario even contemplated by the theming system?

I could of course do the following in the theme's own civicrm.css
```
@import url("path/to/original/civicrm.css");

/* theme's own custom css here */
```
but I think that would pose its own set of challenges given that the @import path would change based on what folder the theme would be placed in
In the docs there are a couple of mentions of the ability to differentiate between "frontend" and "backend" pages, and to decide where the theme gets applied
```
cv api setting.create theme_frontend=newyork theme_backend=newyork
```
[Talking about the legacy mode]

They apply to all CiviCRM screens equally. If you want to provide a different look-and-feel on different screens (e.g. frontend vs backend), there's no way to selectively adjust these options.

But is this featured available in Drupal? That cv command i quoted didn't do anything on my local, and there is no option in the UI of "Display Preferences" anyway (from https://dmaster.demo.civicrm.org/civicrm/admin/setting/preferences/display)

compared to Wordpress (from https://wpmaster.demo.civicrm.org/wp-admin/admin.php?page=CiviCRM&q=civicrm%2Fadmin%2Fsetting%2Fpreferences%2Fdisplay&reset=1)

Thanks as always for your help

So I know that regarding drupal & frontend / backend forms @seamuslee put up this PR https://github.com/civicrm/civicrm-drupal/pull/546/files (which is seems a bit stalled but we could maybe prod it a bit)

I think @totten would need to comment on the questions about overriding...

mentioned in commit 81144b6d

@AkA84 so I spoke with @totten & he is going to update this with details on how to build on top of the css - there is a helper I believe for you. Hopefully by the time you come online he'll have gotten to updating this with that info ....

mentioned in issue community/community-engagement#4

thanks again @eileen , waiting to hear from @totten then! I also saw that https://github.com/civicrm/civicrm-core/pull/14876 got merged, I hope to test it tomorrow on my 5.16 local site then

@AkA84 yes - the setting is still hidden on drupal - I figured we'd wait on your testing & then expose

@eileen I've applied https://github.com/civicrm/civicrm-core/pull/14876/files on my local 5.16 and it seems to work great, I was able to prevent Shoreditch being injected on a public page

Last thing that is left to resolve then is how to make the theme extend the original civicrm.css file, because for now just for testing purposes the shoreditch css file manually @imports civicrm.css (cc @totten )

Again many thanks for your help :)

@AkA84 have you tried pinging @totten on chat - your tz must overlap with his somewhere - he does have a lot of people clamouring for his attention although he is aware of this.

In the meantime this might be a pointer - it's part of the addition to civix for creating themes & refers to a 'search order'

https://github.com/totten/civix/commit/4635909ee57f5a17d5967a4104de2a047d9b5af2#diff-9158b569d82bbf7916f8b4b34f6b426bR5

Also there is https://github.com/civicrm/civicrm-core/pull/12929 & the docs it links to

@AkA84 I've drafted some updated docs to demonstrate ways of combining CSS.

https://github.com/civicrm/civicrm-dev-docs/pull/653

mentioned in issue #300 (closed)

@totten many thanks! I've drafted a PR https://github.com/civicrm/org.civicrm.shoreditch/pull/399 and @d you in this comment, if you could take a look when you have time it'd be great, thanks

closed

mentioned in issue drupal#33 (closed)

mentioned in issue #1554 (closed)

Add a configuration setting for when to load custom CSS

Child items 0

Activity