Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSS file loading in wrong order? #13

Closed
sternhagel opened this issue Apr 10, 2017 · 14 comments · Fixed by #26
Closed

CSS file loading in wrong order? #13

sternhagel opened this issue Apr 10, 2017 · 14 comments · Fixed by #26

Comments

@sternhagel
Copy link

Not being sure whether this is an issue with the Bootstrap Lite theme, or with Backdrop core, I'm posting this issue here, hoping someone can help.

I did a fresh Backdrop install with "Bootstrap Lite" as a parent theme, and then created a child theme, following the official guide from the Backdrop Docs: https://api.backdropcms.org/creating-sub-themes

It's all working generally well, but my custom theme css ("style.css") is being loaded before the default 'bootstrap.min.css' and the 'overrides.css' from the "Bootstrap Lite" theme.
The resulting loading order is:

  • Child Theme CSS
  • Bootstrap CSS
  • Parent Theme CSS

This seems wrong to me because it's very hard to override any CSS rules like that. I probably did something wrong in the child theme, but didn't find any way how to change this.

Another small issue (probably connected to the first, so I'll post it here, too):
When logged in as admin, there are two separate menu entries for the theme settings in the "Design" menu - one called "Child theme settings" and one called "Bootstrap Lite settings". Changing the "Child Theme" settings doesn't have any noticable effect, but changing the "Bootstrap Lite" settings is working even for the child theme.
So in my opinion the "Child Theme settings" entry should not be there, but again, I didn't find a way to disable this. Is there a way to configure this in the child theme settings? Or is it a bug in Backdrop or "Bootstrap Lite"?

@Gormartsen
Copy link
Member

Gormartsen commented Apr 10, 2017 via email

@sternhagel
Copy link
Author

Hi, thanks for the quick response!

Well, I'm not sure how much I can help with it myself. I'm a rather experienced Drupal developer, but this is my first Backdrop project - so at the moment I'm a bit stuck with where to start debugging.

If you don't have too much time at the moment, maybe you can point me into the right direction, and I can try to work on it myself.

Best regards!

@Gormartsen
Copy link
Member

@sternhagel Welcome to backdrop!

For theme settings, I assume there is a need to do something with

https://github.com/backdrop-contrib/bootstrap_lite/blob/1.x-1.x/theme-settings.php

specialy with:

theme_get_setting('bootstrap_lite_container', 'bootstrap_lite'),

For my subtheme I created (copypaste) this file and used next line:

  theme_get_setting('bootstrap_lite_breadcrumb_home', 'flame'),

instead of provided. flame is a name of subtheme.

So I assume that bootstrap_lite/theme-settings.php need to be modified to know that it serve subtheme settings instead of forcing bootstrap_lite settings.

Try to check default backdrop theme to see how it has been done there.

for my subtheme style.css I added next code to flame/template.php

/**
 * Implements hook_css_alter().
 */
function flame_css_alter(&$css) {
  // Add overrides.
  $theme_path = backdrop_get_path('theme', 'flame');
  $override = $theme_path . '/css/style.css';
  $css[$override] = array(
    'data' => $override,
    'type' => 'file',
    'every_page' => TRUE,
    'media' => 'all',
    'preprocess' => TRUE,
    'group' => CSS_THEME,
    'browsers' => array('IE' => TRUE, '!IE' => TRUE),
    'weight' => -1,
  );
}

Again it's hacking the order I assume.

Maybe we need to change weight value in this code

bootstrap_lite/template.php

/**
 * Implements hook_css_alter().
 */
function bootstrap_lite_css_alter(&$css) {
  $theme_path = backdrop_get_path('theme', 'bootstrap_lite');

  if ($bootstrap_cdn = theme_get_setting('bootstrap_lite_cdn')) {
    // Add CDN.
    if ($bootswatch = theme_get_setting('bootstrap_lite_bootswatch')) {
      $cdn = '//netdna.bootstrapcdn.com/bootswatch/' . $bootstrap_cdn  . '/' . $bootswatch . '/bootstrap.min.css';
    }
    else {
      $cdn = '//netdna.bootstrapcdn.com/bootstrap/' . $bootstrap_cdn  . '/css/bootstrap.min.css';
    }
    $css[$cdn] = array(
      'data' => $cdn,
      'type' => 'external',
      'every_page' => TRUE,
      'media' => 'all',
      'preprocess' => FALSE,
      'group' => CSS_THEME,
      'browsers' => array('IE' => TRUE, '!IE' => TRUE),
      'weight' => -2,
    );
    // Add overrides.
    $override = $theme_path . '/css/overrides.css';
    $css[$override] = array(
      'data' => $override,
      'type' => 'file',
      'every_page' => TRUE,
      'media' => 'all',
      'preprocess' => TRUE,
      'group' => CSS_THEME,
      'browsers' => array('IE' => TRUE, '!IE' => TRUE),
      'weight' => -1,
    );
  }
  if ($font_awesome = theme_get_setting('bootstrap_lite_font_awesome')) {
    $awesome = 'https://maxcdn.bootstrapcdn.com/font-awesome/' . $font_awesome . '/css/font-awesome.min.css';
    $css[$awesome] = array(
      'data' => $awesome,
      'type' => 'external',
      'every_page' => TRUE,
      'media' => 'all',
      'preprocess' => FALSE,
      'group' => CSS_THEME,
      'browsers' => array('IE' => TRUE, '!IE' => TRUE),
      'weight' => -2,
    );
  }

}

Try it out, let me know the result.
If you find a way to fix, drop PR here. I would really appritiate that!

Your drupal developer skils should be enough to digg it out!
There is no so much difference drom Drupal 7.

@olafgrabienski
Copy link
Member

I could reproduce the issue. As you know, there is an empty css/style.css in the parent theme. If you put something in this file, it also appears to load before the Bootstrap CSS, just the same issue as when you use the sub-theme.

In contrast, css/overrides.css of the parent theme loads correctly after the Bootstrap CSS. I couldn't find css/overrides.css in the .info file though. @Gormartsen Have you declared it otherwise?

@Gormartsen
Copy link
Member

@olafgrabienski Yes.

check file: bootstrap_lite/themplate.php

/**
 * Implements hook_css_alter().
 */
function bootstrap_lite_css_alter(&$css) {
...
    );
    // Add overrides.
    $override = $theme_path . '/css/overrides.css';
    $css[$override] = array(
      'data' => $override,
      'type' => 'file',
      'every_page' => TRUE,
      'media' => 'all',
      'preprocess' => TRUE,
      'group' => CSS_THEME,
      'browsers' => array('IE' => TRUE, '!IE' => TRUE),
      'weight' => -1,
    );

So basically we do have issue with proper weight management.

@olafgrabienski
Copy link
Member

@Gormartsen Thanks for your feedback! So far I'm not experienced in using CSS overrides, so no idea which weight values would make more sense in bootstrap_lite/template.php. If needed, I'll use your Flame sub-theme workaround in the meantime.

@sternhagel
Copy link
Author

@Gormartsen Thanks for looking into this.
Your workaround to call hook_css_alter in the child theme's template.php is definitely working for me - although I had to set the weight to something positive to get the right order.

But still I wouldn't know what to change in bootstrap_lite's template.php - according to the API docs, default weight for a CSS file should be 0, so files with negative weight should get loaded before any CSS loaded without explicit weight setting. So for me it makes sense to give the parent theme CSS files a negative weight - I couldn't find out yet why it's not working. I found this documentation page quite useful though:
https://api.backdropcms.org/api/backdrop/core%21includes%21common.inc/function/backdrop_add_css/1

I thought about reimplementing the whole thing without using hook_css_alter, using backdrop_add_css instead, but didn't have the time yet.

For the issue with the theme settings, I don't really get the idea of it. I looked into all the backdrop core themes, but none of them really uses theme settings at all - so I just don't understand how theme settings are being "handed over" from child theme to parent theme (or the other way round?). I hope to have some more time soon to look into it, or maybe find something in the docs.

For the time being I'm quite happy with your workaround with an additional template.php.

@auxiliaryjoel
Copy link

I'm having the same issue, my parent (Bootstrap Lite) theme's CSS is not being overridden by my subtheme CSS files which I've declared in my "mytheme.info" file:
stylesheets[all][] = css/suboverride.css

Does anyone know how to fix this?

@sternhagel
Copy link
Author

Yes, I fixed it with the workaround described in my last comment.

To be more specific, you need a file called template.php in your subtheme directory. In this file put something like this:

<?php
/**
 * Implements hook_css_alter().
 */
function NAMEOFYOURSUBTHEME_css_alter(&$css) {
  // Add overrides.
  $theme_path = backdrop_get_path('theme', 'NAMEOFYOURSUBTHEME');
  $override = $theme_path . '/css/suboverride.css';
  $css[$override] = array(
    'data' => $override,
    'type' => 'file',
    'every_page' => TRUE,
    'media' => 'all',
    'preprocess' => TRUE,
    'group' => CSS_THEME,
    'browsers' => array('IE' => TRUE, '!IE' => TRUE),
    'weight' => 1,
  );
}

Replace "NAMEOFYOURSUBTHEME" with the machine name of your theme (e.g. "mytheme" in your example).

This worked for me.

@auxiliaryjoel
Copy link

ok great thanks for that @sternhagel I'll give that a go

@ghost
Copy link

ghost commented Dec 14, 2020

I was helping someone debug this issue in their bootstrap_lite sub-theme today, and I believe the issue here is that adding CSS files via hook_css_alter() isn't the right way to do it (it seems that the weight value has no effect, presumably because that hook is called too late...?).

I believe what this theme needs to do to allow sub-themes to override bootstrap.min.css is add the CDN-CSS via backdrop_add_css() in something like template_preprocess_page() instead.

@bugfolder
Copy link
Collaborator

bugfolder commented Feb 10, 2021

I just ran into this. The root cause is that there is an undocumented 'every_page_weight' key that needs to be added in backdrop_lite_css_alter(). Specifically, add

'every_page_weight' => -1,

to each of the CSS additions in backdrop_lite_css_alter(). That will fix the problem. I will create a PR with the fix.

Here's why.

The function backdrop_add_css() sets the value of this key. It's used to achieve the sort order in the documentation for the 'weight' key. If CSS is added via hook_css_alter(), implementations need to set this key to -1 if 'every_page' => TRUE and +1 otherwise, to match the behavior of backdrop_add_css().

The documentation for hook_css_alter() should probably call out the need for implementers to set this key, since it's not documented elsewhere.

@bugfolder
Copy link
Collaborator

PR created.

@stpaultim
Copy link
Member

So, I'm not 100% sure how to test this quickly, but I have some faith that @bugfolder understands the problem. I'm going to merge this into 1.x-1.x for testing. I'd love to have someone confirm that this works before the next release. Hopefully, fairly soon.

@BWPanda - Do you have any thoughts on this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants