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

Add a configuration setting for when to load custom CSS

Child items ...

Activity