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 causing white space in Drupal #110

Open
rekaeps opened this issue Nov 28, 2017 · 15 comments
Open

CSS causing white space in Drupal #110

rekaeps opened this issue Nov 28, 2017 · 15 comments
Labels

Comments

@rekaeps
Copy link

rekaeps commented Nov 28, 2017

I just updated shoreditch to alpha14. I immediately noticed there was white space added to the top of my Drupal pages, both on the front-end and back-end. I took a look at the developer console and it looks like 60px of padding is being added:

body:not(.toolbar) {
padding-top: 60px !important;

I'm running a bootstrap theme for my Drupal front-end. Drupal version is 7.56, CiviCRM is 4.7.27.

whitespace

@AkA84
Copy link
Contributor

AkA84 commented Nov 29, 2017

@rekaeps thanks for spotting this, could you give me the exact conditions to replicate?

  1. User's permissions
  2. Drupal theme used for admin
  3. Any custom modules installed
  4. Examples of a front-end and back-end page where the issue can be seen

Thanks!

@rekaeps
Copy link
Author

rekaeps commented Nov 29, 2017

Hi @AkA84 ,

The whitespace is showing for both authenticated and anonymous users. We only have one role active, which is the default admin role. We have the default Seven admin theme, and we are using a premium theme called Jango (http://jango.nikadevs.com/?theme=jango) for our front-end. The purchased theme has its own custom modules that integrate with shortcodes (https://www.drupal.org/project/shortcode). These custom modules mostly just make use of the shortcodes so that content (views, blocks, etc.) can easily be inserted into fields.

I'm attaching a few different images of the whitespace, as it occurs on our development website. In some instances, the white space does not occur. I've also attached those images. Finally, I've attached an image of the latest version of shoreditch installed on our CiviCRM instance. It looks like the menu was enlarged in this latest release which corresponds with the whitespace height.

Thanks.

back-end no whitespace
back-end whitespace
front-end no whitespace
front-end whitespace
civicrm latest shoreditch

@reswild
Copy link

reswild commented Dec 18, 2017

I was also having an issue with extra white space on the top of my pages, and the problem turned out to be that I'm using the Administration Menu module instead of the default toolbar in Drupal, and this uses margin instead of padding on the body tag to adjust for the height of the top menu. I solved this by adding the following rule to the Shoreditch css:
html body:not(.toolbar) { margin-top: 0 !important; }

@rekaeps
Copy link
Author

rekaeps commented Feb 8, 2018

Thanks. Administration Menu is used by mostly everyone. Should something be committed to fix this issue?

@mukeshcompucorp
Copy link
Contributor

@AkA84 This is fixed now under new menu changes I think, can we close this?

davialexandre pushed a commit that referenced this issue Jun 5, 2018
…r-on-SSP

PCHR-3724: Use other method of outside namespace for Datepicker for mobiles
@benasp123
Copy link

benasp123 commented Jun 19, 2018

I'm having the same issue. Running the latest version of CiviCRM, Shoreditch and Drupal. Anyone experiencing the same?

@jamienovick
Copy link

jamienovick commented Jun 19, 2018

@mukeshcompucorp @benasp123 this is due to conflict with admin toolbar Drupal module and shoreditch theme. This is a known issue, we have this in the backlog and will fix once backstop JS tests are complete. Thanks for reporting @benasp123

@rekaeps
Copy link
Author

rekaeps commented Aug 7, 2018

@jamienovick Any update on if you all were able to address this? I noticed that there has been a couple recent versions of the alpha being released so just wanted to check.

@jamienovick
Copy link

@rekaeps we've got this one in the backlog and should be part of next release. It didn't quite make the last cut unfortunately.

Thanks

@AkA84
Copy link
Contributor

AkA84 commented Aug 10, 2018

Hi @rekaeps , i've spent a couple of hours investigating the problem, and it seems to me there are two sides of it:

When in civi pages (under /civicrm)

This is indeed the theme not interacting well with admin_menu, as the "legacy mode" (custom-civicrm.css) had been styled with the Toolbar module in mind.

When Toolbar is enabled, we make it the same exact height of the new civicrm menu in order to push down <body>

body {
  .toolbar-menu {
    height: $crm-main-menu-height !important;
    padding-bottom: 0 !important;
    padding-top: 0 !important;
  }
}

When it is disabled, we have to push down <body> directly, by using padding

body {
  &:not(.toolbar) {
    padding-top: $crm-main-menu-height !important;
  }
}

This doesn't work well when the "Administration menu" module is enabled, as it comes with a style that itself pushes down <body>, but by using margin

body.admin-menu {
  margin-top: 20px !important;
}

In this case switching our css rule from padding to margin fixes the issue

body {
  &:not(.toolbar) {
    margin-top: $crm-main-menu-height !important;
  }
}

An additional problem though comes if you also enable the "Administration menu Toolbar style" module. This comes with yet again another style that pushes <body>, by using margin

html body.admin-menu {
  margin-top: 29px !important;
}

Unfortunately the previous fix won't work, because html body.admin-menu has more specificity of body:not(.toolbar) so it "wins" over our fix.

We'll have to think of some better selector that will work the same in all these scenarios.

When in non-civi pages (outside /civicrm)

This looks to me a bug with civi when the administration menu is enabled @colemanw

When you're outside of /civicrm the custom-civicrm.css file is usually not injected at all in the page, as civi is not even initialized (at least this is what i see from my tests), so you shouldn't see any issue (and in fact you're showing also screenshots where everything looks fine)

This though is not the case when you clear the drupal cache. When you clear the cache and then refresh a non-civi page, you should see the issue. If you re-refresh (= use the cache), the issue should not be there anymore.

It seems that on the first page load after clearing the cache, the civicrm_html_head hook is called, which then invokes addCoreStyles

if (arg(0) == 'civicrm') {
  // ...
}
else {
  CRM_Core_Resources::singleton()->addCoreStyles();
}

The method checks if the "Custom CSS URL" field is used (which it is by Shoreditch to inject custom-civicrm.css), and if it is, it adds it to the page

if (!empty($config->customCSSURL)) {
  $customCSSURL = $this->addCacheCode($config->customCSSURL);
  $this->addStyleUrl($customCSSURL, 99, $region);
}

A couple of notes:

  1. This happens regardless of Shoreditch. You can put any file in "Custom CSS URL" (in civicrm/admin/setting/url) and you will see the same behaviour
  2. We are planning to create a "Settings" page where one can turn on/off the legacy mode without having to rely on this field, so at that point the issue should not affect the theme anymore
  3. Weirdly enough, I could replicate the issue only with the admin_menu module
  4. Tested on CiviCRM 5.3.2, admin_menu v. 7.x-3.0-rc5

Hope this was helpful! We'll try to fix the issue under /civicrm before the next release

@AkA84
Copy link
Contributor

AkA84 commented Sep 12, 2018

@rekaeps if you pull latest, you should see that the issue detailed in the "When in civi pages (under /civicrm)" section of #110 (comment) is fixed

As I explained in my comment, the gap you see in the non-civi pages seems more related to a bug in civi itself

@rekaeps
Copy link
Author

rekaeps commented Sep 12, 2018

@AkA84. I just pulled the latest and everything looks great, including the fix to the stretched image. Thank you (and everyone else) for working on getting the /civicrm side of things working. Looking forward to the non /civicrm fixes.

@rekaeps
Copy link
Author

rekaeps commented Mar 13, 2019

@jamienovick @AkA84 @mukeshcompucorp This issue seems to be still be open and the non-civi side of things still seem to persist, at least on my installation. I know there has been a push in recent months to get Shoreditch moving and I remember reading somewhere on one of CiviCRM's many communication mediums (stack exhange, mattermost, etc) that a lot of lessons have been learned throughout this process. Is there any update on the non-civi issue, as well as Shoreditch as a whole? It seems like commits are coming periodically, but I haven't heard of a beta release or anything of that nature in a while.

@kelizoliva
Copy link

This is still an issue for me, running v0.1-alpha31 on CiviCRM v5.13.5 on Backdrop 1.14.x-dev. Have unnecessary 60px of top margin, due to the !important. Backdrop native menu is different than whatever menu this is trying to accommodate. Is the !important necessary? Could a more precise identifier be used to ensure that this only affects instances on Drupal?

And this manifests on non-civicrm pages as well. I've created a custom extension to try to override some things in shoreditch that don't work for us, but while the css for shoreditch loads on non-civicrm pages, css for my extension does not. Is there a way to keep Shoreditch from loading out of context, so we at least don't have to override by more than one method?

@benasp123
Copy link

I tried again last week and it's fixed for me.

CiviCRM 5.14.0.
0.1-alpha31

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

No branches or pull requests

7 participants