Skip to content

Commit

Permalink
Switch to Ruby sass, update Gemfile to Sass 3.4 and Compass 1.0, fix …
Browse files Browse the repository at this point in the history 
…errors in docs CSS
  • Loading branch information
@gakimball
gakimball committed Sep 5, 2014
1 parent aab71bc commit 8421ac4
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 17 deletions.
3 changes: 2 additions & 1 deletion Gemfile
View file
@@ -1,3 +1,4 @@
source "https://rubygems.org"

gem "compass", "0.12.2"
gem "sass", "~> 3.4.0"
gem "compass", "~> 1.0.0"
27 changes: 20 additions & 7 deletions Gemfile.lock
View file
@@ -1,16 +1,29 @@
GEM
remote: https://rubygems.org/
specs:
chunky_png (1.3.0)
compass (0.12.2)
chunky_png (1.3.1)
compass (1.0.1)
chunky_png (~> 1.2)
fssm (>= 0.2.7)
sass (~> 3.1)
fssm (0.2.10)
sass (3.2.14)
compass-core (~> 1.0.1)
compass-import-once (~> 1.0.5)
rb-fsevent (>= 0.9.3)
rb-inotify (>= 0.9)
sass (>= 3.3.13, < 3.5)
compass-core (1.0.1)
multi_json (~> 1.0)
sass (>= 3.3.0, < 3.5)
compass-import-once (1.0.5)
sass (>= 3.2, < 3.5)
ffi (1.9.3)
multi_json (1.10.1)
rb-fsevent (0.9.4)
rb-inotify (0.9.5)
ffi (>= 0.5.0)
sass (3.4.3)

PLATFORMS
ruby

DEPENDENCIES
compass (= 0.12.2)
compass (~> 1.0.0)
sass (~> 3.4.0)
7 changes: 4 additions & 3 deletions Gruntfile.js
View file
Expand Up @@ -51,8 +51,9 @@ module.exports = function(grunt) {
sass: {
dist: {
options: {
includePaths: ['scss'],
sourceMap: true
loadPath: [__dirname + '/scss'],
sourcemap: 'auto',
bundleExec: true
},
files: {
'dist/assets/css/foundation.css': '<%= foundation.scss %>',
Expand Down Expand Up @@ -232,10 +233,10 @@ module.exports = function(grunt) {
grunt.loadNpmTasks('grunt-contrib-copy');
grunt.loadNpmTasks('grunt-contrib-uglify');
grunt.loadNpmTasks('grunt-contrib-watch');
grunt.loadNpmTasks('grunt-contrib-sass');
grunt.loadNpmTasks('grunt-karma');
grunt.loadNpmTasks('grunt-newer');
grunt.loadNpmTasks('grunt-rsync');
grunt.loadNpmTasks('grunt-sass');
grunt.loadNpmTasks('grunt-contrib-jst');
grunt.loadNpmTasks('grunt-string-replace');

Expand Down
6 changes: 3 additions & 3 deletions doc/assets/scss/docs.scss
View file
Expand Up @@ -4,7 +4,7 @@

//Foundation Libraries
@import
"scss/foundation/settings",
"foundation/settings",
"foundation";

//Marketing Site Common Library
Expand Down Expand Up @@ -274,7 +274,7 @@ footer { margin-top: 45px; }
margin-bottom: $paragraph-margin-bottom;
text-rendering: $paragraph-text-rendering;

&.lead { @extend %lead; }
&.lead { @include lead; }

& aside {
font-size: $paragraph-aside-font-size;
Expand Down Expand Up @@ -549,7 +549,7 @@ $custom-active-bg:darken($custom-link-color, 5%);
// .docs-wrap { margin-top: 30px; }
.docs-wrap .inner-wrap { background: #efefef; }
.docs-wrap .main-section { padding: 0 20px 0 20px; }
.main-section { @extend %kill-flicker; }
.main-section { @include kill-flicker; }
.doc-oc-list { background: $off-canvas-bg; }

// Make sure topbar dropdowns are above tab bar.
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Expand Up @@ -13,11 +13,11 @@
"grunt-contrib-jst": "~0.6.0",
"grunt-contrib-uglify": "~0.4.0",
"grunt-contrib-watch": "~0.6.1",
"grunt-contrib-sass": "~0.8.1",
"grunt-string-replace": "~0.2.7",
"grunt-karma": "~0.6.2",
"grunt-newer": "~0.7.0",
"grunt-rsync": "~0.5.0",
"grunt-sass": "~0.13.0",
"handlebars-helper-rel": "~0.1.2",
"handlebars-helper-slugify": "~0.2.0",
"highlight.js": "~7.3.0",
Expand Down
2 changes: 1 addition & 1 deletion scss/foundation/_functions.scss
View file
Expand Up @@ -10,7 +10,7 @@ $rem-base: 16px !default;
$modules: () !default;
@mixin exports($name) {
@if(not index($modules, $name)) {
$modules: append($modules, $name);
$modules: append($modules, $name) !global;

This comment has been minimized.

Sign in to view

Copy link
@christianvuerings

christianvuerings Sep 17, 2014

Contributor

@gakimball This is actually causing an error in our Rails app when we include 5.4.4

Invalid CSS after "...odules, $name) ": expected "}", was "!global;"

Screenshot at http://cl.ly/Xb19
Any ideas?

This comment has been minimized.

Sign in to view

Copy link
@schmidt

schmidt Sep 24, 2014

This problem is discussed in #5811
@content;
}
}
Expand Down
2 changes: 1 addition & 1 deletion scss/foundation/components/_offcanvas.scss
View file
Expand Up @@ -29,7 +29,7 @@ $tabbar-header-margin: 0 !default;
// Off Canvas Menu Variables
$off-canvas-width: rem-calc(250) !default;
$off-canvas-bg: $oil !default;
$off-canvas-bg-hover: background: scale-color($tabbar-bg, $lightness: -30%) !default;
$off-canvas-bg-hover: scale-color($tabbar-bg, $lightness: -30%) !default;

// Off Canvas Menu List Variables
$off-canvas-label-padding: 0.3rem rem-calc(15) !default;
Expand Down

15 comments on commit 8421ac4

@beebole
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I had to remove the !global to make it work with the last version of sass.

The error message was: bower_components/foundation/scss/foundation/functions:13: error: error reading values after

@gakimball
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it Ruby Sass or libsass? This will work with Ruby Sass 3.3 and up but not libsass, unless you remove !global.

@oller
Copy link
Contributor

@oller oller commented on 8421ac4 Sep 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for needing to remove !global to get libsass compiling again, it'd be great if this could be supported too, as opposed to having to break dependency.

Perhaps a compiler variable within _settings.scss ?

@jopotts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue here.

The sass-rails (4.0.3) gem (the latest stable version) has a sass (~> 3.2.0) dependency, which resolves to sass (3.2.19). So this commit's causing Foundation 5.4.4 to fail (unless !global is removed).

@oller's suggestion is probably best to enable backwards compatibility with older sass gems, even once sass-rails has caught up.

@luizkowalski
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same error here!

@gakimball
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could detect the libsass compiler using a user-defined variable (i.e. $libsass) or Sass's feature-exists() function, but even if you hide the !global keyword inside an @if statement, libsass will still notice it and throw an error.

Try opening this Sassmeister gist and switching between libsass and Ruby Sass.

@jopotts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Scuppered! We'll have to update our scss/sass's somehow. Thanks for looking into it.

@christianvuerings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue at #5811

@gakimball
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're talking about removing the !global flag to get back compatibility with Rails and libsass. We had a lot of Ruby users upgrading their Sass immediately and asking us about a fix when things stopped working, but we may have jumped the gun by upgrading to 3.4 right away.

If we roll back, the plan would be to remove the !global flag and add a Gemfile to our Compass template (which is what the CLI installs when you run foundation new) that enforces Sass 3.3. With no !global flag, it will compile in 3.2 fine, which means it will compile in libsass and foundation-rails just fine.

@oller
Copy link
Contributor

@oller oller commented on 8421ac4 Sep 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be prudent @gakimball, give other compilers a little more time to catch up. In my case i'm using libsass, so this would work great, and mean I can keep patching as opposed to locking at 5.4.3.

@christianvuerings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gakimball Thank you!

@DanielKehoe
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much for giving this priority. I'm not sure I'm clear on your proposal, but the fix should not require the CLI foundation new command because many of us install manually or use the rails_layout gem in conjunction with Rails Composer.

@rafibomb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanted to address what what happened here. When Sass 3.4 came out, we saw tons of people trying to update (and create new) Compass projects with the latest Sass version. We had been holding off updating to Sass 3.3 for some time because we knew it would cause some breaking changes. So when people were letting us know about the widespread problems people were having with Compass. Due to the huge volume of those issues, complains on twitter, and forum posts, we made the decision to upgrade compatibility.

Even though we put put a warning the same day on the Libsass install instructions, it could have been handled better. Libsass is great and we used it on our internal projects. Compass had much greater adoption so we decided to make the fix for that user base. It's a tough problem because the Sass community is split and we need Foundation to support multiple versions of Sass.

Right now we are working to figure out this problem from another angle. @gakimball reworked part of the codebase and the import process to see if we can remove the need for !global which is what causes older Sass versions to choke. This should make it Sass 3.2, 3.3, and 3.4 compatible.

We are testing today and should be able to deploy later today.

@DanielKehoe
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the explanation and the update.

@christianvuerings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rafibomb Thanks for the update, much appreciated.

Please sign in to comment.