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

Placeholders #4134

Closed
wants to merge 5 commits into from
Closed

Placeholders #4134

wants to merge 5 commits into from

Conversation

cmoralesweb
Copy link

I don't like to put presentational classes in my HTML, such as class="small-5 preffix-1" . This can be avoided in Foundation using mixings, but that usually leads to repeated code. A great way to have the best of both worlds is to use placeholders (as explained by http://ianstormtaylor.com/oocss-plus-sass-is-the-best-way-to-css/ )

I've created placeholders for Foundation grid.

@hashanp
Copy link
Contributor

hashanp commented Jan 23, 2014

+1

@smileyj68
Copy link

Moving this pull request to a later milestone - we can't use @extend in any real capacity at the moment due to libsass support. We won't close this, just move it up a bit.

@maximelebreton
Copy link

+1
And we save ~1000 to ~2200 lines of generated code (all media queries activated)
Thanks for the commits!

@cmoralesweb
Copy link
Author

You are welcome :)

@smileyj68 I don't understand why @extend can't be used , could you elaborate, please? (this is not a complain, in case it sounds like that)

@maximelebreton
Copy link

I think that foundation uses libsass (C/C++) instead of Sass (ruby) for the compiling process, and libsass has some limitations :
http://www.solitr.com/blog/2014/01/state-of-libsass/

@vinayraghu
Copy link
Contributor

I completely agree that the grid would be an ideal candidate for sass placeholders, however, this would make debugging difficult. I can understand the grid without having to look at the CSS rules. .large-6 is easier to read than width: 50%

Having said that, I hate bloating my markup with something like below
large-6 medium-8 small-12 columns large-centered medium-uncentered

So I actually extend foundation's grid in a separate layout.scss file. So my .tab-container is super readable inside of my scss and does not bloat my markup. I also get to easily debug in dev tools.

@cmoralesweb
Copy link
Author

@rvinay88 I don't really get what you explain about readability or debugging, could you please explain it? Are you refering to the code in the browser? Otherwise, if you are refering to the scss files, it's as easy as "regular" foundation, since I used the same names for my placeholders as Foundation uses. So, that bloated code would be:

.tab-container{
@extend %large-6;
@extend %medium-8;
@extend %small-12;
(columns is not needed)
@extend %large-centered;
@extend %medium-centered;
}

It's basically the same as applying the code to the HTML... just doing it in the CSS, where it should be.

@vinayraghu
Copy link
Contributor

Like. if you think about 100 places where you need to extend %large-6
and then you debug that code, it will essentially be 100 classes separated by commas and then width: 50%

I know we can work around but do you think we need that kind of chaining?

@cmoralesweb
Copy link
Author

@rvinay88 Using Firesass (or a similar solution) you get detail info about where that was set in the original files.

By the way, I'm not saying everybody should use this. However, both sass mixins and html classes are provided as options for people to choose from... Placeholders could be offered to, they can be easily turned on an off (commenting the imports the same as the rest of the framework) so people could use them if they wanted and, if not, no side effect (since placeholders wouldn't be compiled anyway).

@vinayraghu
Copy link
Contributor

Hey cmoralesweb, I agree with you. I like the idea and I have a few points to make

  1. Selective @imports that do not output CSS is on the list of things that sass wants to handle. Its been around for (quite) a while and they might get to it soon. Until then we don't have any other choice but to go with your idea.
    @import files as modules sass/sass#353 (comment)
    Point number 4
  2. We have to consider xlarge and xxlarge, tie your placeholder idea to those variables and output grids accordingly. If user enables xlarge and xxlarge grids, then we have to handle that.
  3. We have to use a loop to go over this rather than manually including grid-column mixin
    I have made a (very) rough pen as a POC. Check line 71
    http://codepen.io/rvinay88/pen/BeFld
  4. We have to take this discussion to the forum and see if other people find it interesting enough. I have never liked grid classes in my markup.

@cmoralesweb
Copy link
Author

:)

Do you want me to try the loop and include the xlarge and xxlarge options?

@vinayraghu
Copy link
Contributor

Yes! I added this topic in the forum as well, so we can take discussions there and keep your PR discussion free
http://foundation.zurb.com/forum/posts/2956-extend-only-grid-classes

@cmoralesweb
Copy link
Author

Ok, great! I'll try that and upload the commits when I have some time.

@vinayraghu
Copy link
Contributor

Hey @cmoralesweb Can you give me your email?

@vinayraghu
Copy link
Contributor

@smileyj68 Could you expand a little more on libsass issue with @extends please? I read the article posted by @maximelebreton but I am not sure if that means we cannot use @extend at all.

@cmoralesweb
Copy link
Author

@rvinay88 I've just sent you an email ;)

@vinayraghu
Copy link
Contributor

@cmoralesweb didn't receive it..could you please resend or hit me up on twitter?

@vinayraghu vinayraghu mentioned this pull request Mar 17, 2014
9 tasks
@hellapixels
Copy link

Hey @rvinay88 F6 will be rewritten from the ground up, so this will not be accepted as a pull request. I am however going to open an issue for @extend in Foundation6 so that we can consider its use during the brainstorming and development of Foundation 6. Thanks!

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 this pull request may close these issues.

7 participants