Hi - I'm using Basic as a starter for a number of sites (thanks for the great work!) and ran into this issue today - I was using it in a new test project, and the sidebars weren't displaying in their proper places. Digging around for a while I realized that the em() functions in _grid--settings weren't computing properly in the compiled CSS files. That led me to discover that in Bourbon 5 (still in beta, but in use in places, such as with the CodeKit app, which I use as a local compiler) the em() function has been deprecated and will no longer work. As a result the grid functions break in Basic.

It's trivial to add the function back in to _mixins, (or in Codekit, you can use a lower version of Bourbon by manually copying files) which will make it work for now, but it's something to be aware of for future releases. I will contribute code back if I can, but I have to admit I'm not a SASS or Bourbon expert who can suggest a specific fix for this right away.

Discussion about it is here: https://github.com/thoughtbot/bourbon/issues/691

CommentFileSizeAuthor
#8 basic_em-deprecated-2827661-8.patch7.01 KBleahtard
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

michaelschutz created an issue. See original summary.

michaelschutz’s picture

michaelschutz CreditAttribution: michaelschutz commented
Title: Bourbon 5 deprecated the em() function » em() function is deprecated in Bourbon 5
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
Version: 8.x-1.2 » 8.x-1.x-dev

Seems worth while to keep up with what they are doing.

We could just do the mixin math inline?
width: #{24px / 16px}em;/code>

birchy82’s picture

birchy82 CreditAttribution: birchy82 commented

I'm currently stuck in the same situation. I use codekit to compile and it comes with the latest version of Bourbon. All of my columns are stacked instead of floating next to each other on desktop screens.

I replaced Bourbon with the stable latest version 4. But, I am still getting the columns stacked. Michael, is there anything else you did to correct this?

Thanks

michaelschutz’s picture

michaelschutz CreditAttribution: michaelschutz commented

birchy82, because I couldn't see anywhere else the variables were used, I just simplified things and went with pixels in _grid-settings.scss:

// Breakpoints
$small-screen:  320px;
$medium-screen: 720px;
$large-screen:  960px;

// Define your mobile first breakpoints using min-width
$small-screen-up:   new-breakpoint(min-width $small-screen 4);
$medium-screen-up:  new-breakpoint(min-width $medium-screen 8);
$large-screen-up:   new-breakpoint(min-width $large-screen 12);

(Since these are the only places the breakpoint variables seem to be used, one could simply even further and just put the values in the new-breakpoint function directly, but that's probably more hack-like than it should be. (Note - I also changed the number of columns for my project from the default.))

I know the arguments for ems instead of pixels, but in terms of a grid, I'm going to stick with pixels for now for the projects I'm working on.

Another option is to do what joelpittet suggested, or define the em() mixin in _mixins.scss.

leahtard’s picture

leahtard CreditAttribution: leahtard at The Jibe commented

Thanks michaelschutz!

We will review that fix and get something committed soon. Thanks for reporting.

Cheers, Leah

michaelschutz’s picture

michaelschutz CreditAttribution: michaelschutz commented

Thanks, Leah. I'm not suggesting my solution as the best fix for the project; I'm trusting that you can take the issue and provide a better one. :)

leahtard’s picture

leahtard CreditAttribution: leahtard at The Jibe commented
Status: Active » Needs review
FileSize
basic_em-deprecated-2827661-8.patch7.01 KB

I spent a little time investigating Bourbon 5 and Neat 2 and I am seeing many other items we are going to have to update. With these libraries still being in beta, we still have some time to figure this out. Having said that, think it is good practice to start to remove code we know will be deprecated. The Bourbon documentation is using pixel values when defining these breakpoints -- much like michaelschutz's updates above. I would say we follow this pattern and remove the use of em(). I have attached a patch outlining these code updates.

Cheers, Leah

  • leahtard authored 3b6a190 on 8.x-1.x 
    Issue #2827661 by leahtard, michaelschutz, joelpittet: em() function is...
leahtard’s picture

leahtard CreditAttribution: leahtard at The Jibe commented
Status: Needs review » Fixed

This fix is now committed.

birchy82, I think you might be experiencing something different here. Maybe it has something to do with how codekit is compiling the Sass. Watch how many decimal places are being rendered in the percentages in layout.css. It should be 5. Example:

@media screen and (min-width: 720px) {
  .one-sidebar.sidebar-second #content {
    float: left;
    display: block;
    margin-right: 3.22581%;
    width: 74.19355%;
  }
  .one-sidebar.sidebar-second #content:last-child {
    margin-right: 0;
  }
}

If you are still having issues, don't hesitate to open a support ticket. Happy to help where we can.

Cheers, Leah

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

kyletaylored’s picture

kyletaylored CreditAttribution: kyletaylored at LevelTen Interactive commented

I recently got onboarded to a new project that turned out to be using the Basic theme, and I ended up strolling through this issue queue as I was researching deprecations. I saw the patch removed some of the offending functions, but it still uses the new-breakpoint() function. In the latest version of Neat (2.0.0), that function has been deprecated (well, a lot of features have been), so this might need to be revisited in the future or even start a Neat 2.0 upgrade issue.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented

@kyletaylored please do start a neat 2.0 issue

leahtard’s picture

leahtard CreditAttribution: leahtard at The Jibe commented

When using the 8.x-1.x branch, you need to be using Bourbon 4.x and Neat 1.x. If you are using Grunt to pull in the libraries, this is automatically taken care of. I recently updated package.json to prevent Bourbon 5.x and Neat 2.x from being brought in. This is just because these changes in these libraries are going to require a lot of rewriting to the rest of Basic. We have a roadmap issue started for Basic 8.x-2.x where these updates will be taking place: https://www.drupal.org/node/2847769.

So if you are manually downloading the libraries, make sure you are pulling the correct versions. We should add some notes to the readme about this. Any takers?

Cheers, Leah