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

Create SASS version #41

Closed
kristoferjoseph opened this issue Dec 16, 2014 · 47 comments
Closed

Create SASS version #41

kristoferjoseph opened this issue Dec 16, 2014 · 47 comments

Comments

@kristoferjoseph
Copy link
Owner

Because for some reason people love complicating their build script. ;)

@GLStephen
Copy link

+1 for SCSS - we've already incorporated flexboxgrid into an SCSS build and once the build process is setup it is hands off for the rest of time. We incorporated Flexboxgrid into a major project a few months before you did the class name change. It would be nice to have an SCSS version where those names were configurable so we can keep up with the project more easily.

@stereokai
Copy link

What exactly are you planning? Mixins?

@GLStephen
Copy link

I don't think Mixins are needed exactly. These are core structural classes and I don't need to rename them exactly in an ongoing dynamic way. I would like to be able to maintain the original names e.g. "column-1-hand" since I liked that convention and thought it was more expressive than "sm, xs, etc." In my mind the origin convention puts me in the user's mind, not in the mind of a developer. I know that sounds silly but I see the user using my app in my mind when I read "hand" and "lap" I see a screen when I read "sm" and "xs".

In addition, it was used throughout a rather large code base extensively. I'm not completely against refactoring my classes, and currently I just use Beyond Compare to pull over the changes (it is only 1000 lines or so). If a SCSS version is planned then a way to make the naming convention dynamic would be great.

@kristoferjoseph
Copy link
Owner Author

@GLStephen Thanks for the feedback. Sorry if I caused you any stress.
I liked the old naming conventions as well, but the overwhelming requests from the community were that they wanted bootstrap compatible names.
I am planning on making class names configurable for exactly that reason.
The idea is that I will have a cli that will output the source files with using user configurable options. Then I will generate LESS, SCSS, STYLUS, Standards CSS, etc. from that. This will allow people to customize the amount of columns as well.

@chorijan
Copy link

+1 for the SASS version. any idea for a ETA?

@einthusan
Copy link

I see that the less version if there also, can be it used in production?

@kristoferjoseph
Copy link
Owner Author

@chorijan Soon? Was going to push something this weekend.
@einthusan I hope I am understanding what you are asking. The plan is to make production ready versions of those preprocessors listed.

@einthusan
Copy link

@kristoferjoseph okay thanks. I just wasn't sure if the production version was ready or not, but by seeing that the issue is still open, i realized it's still a work in progress.

Thanks for all your hard work!

@suenot
Copy link

suenot commented Feb 7, 2015

+1, I needed it to rebuild grid to 24 collumns.

@kristoferjoseph
Copy link
Owner Author

Worked on it a bit today. Will post it in a branch tomorrow.

@benmliang
Copy link

+1 awesome job

@kristoferjoseph
Copy link
Owner Author

Here are the beginnings of the SASS version: https://github.com/kristoferjoseph/flexboxgrid/pull/55/files

It is not working yet, but I wanted to share the direction I was heading with all interested parties.
The other flavors will be based off of this eventually.

Adds a ton more variables, source as .scss files and the ability to specify the number of columns you need.
💌

@guillaumevincent
Copy link

awesome feature, how can people help ?

@kristoferjoseph
Copy link
Owner Author

@guillaumevincent check out that branch and poke around. Feedback welcome. :)

@kristoferjoseph
Copy link
Owner Author

@GLStephen @stereokai @chorijan @einthusan @suenot @benmliang @guillaumevincent and @SlexAxton

PR #55 is ready for feedback. It is "Feature Complete" so it could be pushed to master, but I'd love a sanity check on where it is at currently. The output is correct and the mixins I could see being useful on their own.
Only part I dislike is the auto-width column function. Seems a bit arbitrary. If you can think of a better way to do that have at it.
Giving every class a responsive modifier has grown the file size larger than I'd like, but I feel like we could add a way to only add the properties you plan on using.
One step at a time. :)

@SlexAxton
Copy link
Contributor

I don't know a ton about SASS or what people need explicitly, so ignore my feedback but...

this seems p good to me.

@GLStephen
Copy link

At a glance it looks good. I will integrate into our SCSS flow this week and see what it emits.

@kristoferjoseph
Copy link
Owner Author

Thank @GLStephen & @SlexAxton

@ffub
Copy link

ffub commented Mar 23, 2015

#55 works well for me. I have a few thoughts on including flexboxgrid as part of a larger project.

As a module I'd rather an import didn't output any code until I run a mixin. You could have a master mixin called flexboxgrid that does that, and into which you could pass basic config. Likewise it would be good to use '!default' on any global variables.

@kristoferjoseph
Copy link
Owner Author

@ffub thanks for taking a look. Really appreciated. I will add your feedback to #55.

@codepunkt
Copy link

Does not compile for me.

Error: Invalid CSS after "... #{$sm}, #{$md}": expected selector or at-rule, was ", #{$lg};"
        on line 34 of flexboxgrid.scss
You may not @extend an outer selector from within @media
You may only @extend selectors within the same directive.

What sass version should i be using?
Would you mind adding a Gemfile?

@kristoferjoseph
Copy link
Owner Author

@gonsfx You can look at the package.json to see what version I am using for the build. It uses gulp-sass which uses node-sass...

This is one of the reasons I didn't add a SASS version to begin with. Maintenance can be difficult. I can really only support the way I build this project ( which uses gulp ) as I would only be guessing as to the myriad other ways people are building in the wild.

I don't use Gemfiles so I don't have the insight needed, but if you can get it to work, please make a pull request as I am sure lot's of others will run into the issue you are having.

Thank you very much :)

@codepunkt
Copy link

Don't get me wrong - i absolutely love the sass port and think it should be included in master better sooner than later.

Nevertheless, compiling did not work. If you choose to add it to master, be sure to explain that node-sass is used instead of the ruby gem and that those have different features.

On a sidenote, you have an npm-shrinkwrap.json next to your package.json so installing from your projects root directory with npm install installs the exact same versions that you used.

Problem is that the gulp-sass version you shrinkwrapped is 1.3.2 which in turn uses node-sass version 2.0.0-beta. When running one of the current node stable versions 0.12.x, your project doesn't compile. To compile with the newer node stable versions, node-sass is needed in version 2.0.1.

@kristoferjoseph
Copy link
Owner Author

@gonsfx Wow, thanks for pointing that issue out. I added that shrink wrap file to try and fix the possible issue, seems it made an issue of it's own. I run update and see if it fixes it.

When you say compiling did not work, are you saying when you run gulp it doesn't build for you?

@codepunkt
Copy link

Yes, that is correct. Running gulp threw errors.

@kristoferjoseph
Copy link
Owner Author

@gonsfx Please pull and let me know if you still see errors. Thanks.

@codepunkt
Copy link

Seems to work. Switched back from our fork to your #sass branch

@kristoferjoseph
Copy link
Owner Author

@gonsfx Awesome! Was wondering if you have any feedback on the proposal made in the LESS issue about project structure to assist in importing mixins etc.

Ideally I would have something similar for each preprocessor.

Thanks again for the feedback.

@cmoralesweb
Copy link

Just a small note, in case you didn't know it:

gulp-sass uses node-sass, but there is a ruby plugin available (gulp-ruby-sass) which uses the official library.

@kristoferjoseph
Copy link
Owner Author

@cmoralesweb Thanks. Please send a PR once 7.0 is released. I have moved away from gulp for the next release though due to all of the library version mismatches.

@cmoralesweb
Copy link

Moved to grunt or to nothing at all? I can help with grunt/gulp if needed.

I am tweaking the library so it generates %placeholders for the grid, in case you are interested. Similar to what I did here: https://github.com/cmoralesweb/atomic-foundation/blob/master/foundation-placeholders/_grid-placeholders.scss

@kristoferjoseph
Copy link
Owner Author

@cmoralesweb I have switched to npm run scripts. Much less headache involved. Grunt/Gulp scripts are overkill for simply running a file through a preprocessor. The overhead only begins to make sense at the scale of an application. input > output is a very simple thing to do in a run script and it has no dependencies to struggle with keeping up to date.

Placeholders are cool, not sure that is a use case I want to support. Too much room for error on the user side leading to a possible ton of issues filed. It would be akin to a foot cannon mixing in grid properties into other selectors invisibly.

@codepunkt
Copy link

@cmoralesweb @kristoferjoseph
node-sass is a lot faster than ruby-sass so i would encourge to stay with it, if it compiles (which it does).
i don't see a reason to switch to ruby-sass.

@cmoralesweb
Copy link

@gonsfx @kristoferjoseph

Basically this: https://github.com/sass/libsass/wiki/The-LibSass-Compatibility-Plan

node-sass is faster than ruby-sass, indeed, but it lacks some features from the official library, mainly full placeholder support.

Regarding the "it compiles"... it does in node-sass, not in ruby-sass. I tried using it with ruby-sass (3.4) and it failed. I need ruby-sass in my development (I use placeholders a lot), so I can't import the grid along with the rest of my files, and it's not viable to have node-sass on one side and ruby-sass on the other.

@ffub
Copy link

ffub commented Jun 10, 2015

Just replace the use of extend with include for the columns. You’ll get a few extra lines of CSS, but SASS doesn’t allow extending within media queries.

On Wed, Jun 10, 2015 at 1:12 PM, Carlos Morales notifications@github.com
wrote:

@gonsfx @kristoferjoseph
Basically this: https://github.com/sass/libsass/wiki/The-LibSass-Compatibility-Plan
node-sass is faster than ruby-sass, indeed, but it lacks some features from the official library, mainly full placeholder support.

Regarding the "it compiles"... it does in node-sass, not in ruby-sass. I tried using it with ruby-sass (3.4) and it failed. I need ruby-sass in my development (I use placeholders a lot), so I can't import the grid along with the rest of my files, and it's not viable to have node-sass on one side and ruby-sass on the other.

Reply to this email directly or view it on GitHub:
#41 (comment)

@kristoferjoseph
Copy link
Owner Author

@cmoralesweb thank you for the info, I appreciate it. Just so we are all on the same page, I will not be switching to ruby-sass nor will i be making placeholders for this grid. With that said, if you want to file a separate issue with the specific error you get when you compile with ruby-sass I can see if there is a change I can make to my code to at least avoid the error. Thanks!

@cmoralesweb
Copy link

@kristoferjoseph I think @ffub nailed the error, so no need for a new issue, I guess 😄

I understand your decision about ruby/placeholders 👍

@kristoferjoseph
Copy link
Owner Author

@cmoralesweb thank you for understanding. Been really trying to limit scope so I can ship this dang release that has been taking fo'evah. We can see if people are needing placeholders etc. after 7.0 drops and we can reconsider. Thanks again for being chill bud.

@cmoralesweb
Copy link

Just mention me if you ever implement them and need my help.

@kristoferjoseph
Copy link
Owner Author

@cmoralesweb 👍 will do ❤️

@miraage
Copy link

miraage commented Jul 7, 2015

@kristoferjoseph , please, keep in mind some things.
It would be great too see mixins, so we can use your components in our projects. For example:

// Somewhere in flexboxgrid-filename.scss
@mixin make-column($count) {
  // column definition
}

// Somewhere in our projects
.article__controls {
  @include make-column(6);
}

@fluxxus
Copy link

fluxxus commented Aug 5, 2015

+1 @miraage

@fabianmarz
Copy link

👍

@Romainpetit
Copy link

What's missing now ? The Sass branch works like a charm here. It's just weird to see Grunt being switched to Gulp

@kristoferjoseph
Copy link
Owner Author

@Romainpetit There are still some incompatibilities with sass versions and compilers. The switch to gulp was done at the time to use the most compatible plugin. Just a note, not interested in debate over this change. Thanks.

@cmoralesweb
Copy link

@kristoferjoseph Hey, just a note, in case you are interested. Libsass has released 3.3 (https://github.com/sass/libsass/releases/tag/3.3.2) and it (finally!!) supports '%placeholders' properly. So my concerns in my previous comment (#41 (comment)) are no longer valid.

I know you never intended to switch to ruby, but still I thought it would be nice to point it out :)

@kristoferjoseph
Copy link
Owner Author

I will not be supporting any preprocessors. The workflow will be CSS only. 7.0 was stalled for way too long over preprocessor issues and I want this project to continue to look forward. Apologies.

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

No branches or pull requests