csswizardry / inuit.css

Powerful, scalable, Sass-based, BEM, OOCSS framework.
inuitcss.com
Other
3.83k stars 415 forks source link

Strange Grid Comments Undocumented #194

Closed marcwiest closed 10 years ago

marcwiest commented 11 years ago

Could someone explain the strange comments between grid__item tags? In my case, they seem to be required.

   <div class="grid__item  one-third">
       <p>One third grid</p>
   </div><!--

--><div class="grid__item  two-thirds">
       <p>Two thirds grid</p>
   </div>
csswizardry commented 11 years ago

These do need documenting, yes. Because the grid system uses display:inline-block; you need to remove the whitespace from between the grid items :)

marcwiest commented 11 years ago

Thanks, I didn't know inline-block can cause that kind of problem. I just did a quick search and found this http://css-tricks.com/fighting-the-space-between-inline-block-elements/

csswizardry commented 11 years ago

The grid system used to use a non-markup fix, but that had issues:

https://github.com/csswizardry/csswizardry-grids/commit/744d4b23c9d2b77d605b5991e54a397df72e0688

marcwiest commented 11 years ago

I see. The current solution is seems to be the most fitting. Any idea at what point this doesn't need to be accounted for anymore?

csswizardry commented 11 years ago

It will always need to be accounted for; whitespace between inline-block elements should exist, only we don’t want it to in this case.

marcwiest commented 11 years ago

Why did you opt. for inline-block? Wouldn't it be better to use display: block; float: left; instead?

csswizardry commented 11 years ago

inline-block allows for greater control through vertical alignment as well as manipulation with text-align :)

marcwiest commented 11 years ago

How do you feel about making inline-block optional via a variable?

csswizardry commented 11 years ago

I wouldn’t do it, there would be too much overhead maintaining two types of grid system and the benefits of inline-block far outweight the downsides.

bvdputte commented 11 years ago

Harry, will this fix be added to inuit? https://github.com/csswizardry/inuit.css/issues/170#issuecomment-14859371 That way people can opt to choose using $use-markup-grid-fix or not... If a better fix appears (from YUI or something) you can simply use that fix without changing the $use-markup-grid-fix var...

kevva commented 11 years ago

@bvdputte, I've done some extensive testing (see necolas/suit-button-group#2 and #107) and didn't find a better solution. I feel that this solution is pretty solid though.

KingScooty commented 11 years ago

Aahh, this is frustrating. Started a node project earlier today and was really looking forward to using inuit 5.0 on it. Commenting out the whitespace is such a deal breaker :( Unless i completely uglify the source by compressing all the HTML, i simply can't avoid the whitespace in the nicely formatted markup - commenting the whitespace in jade is beyond a ballache.

I get that using inline-block is appealing in order to use text-align etc. But i've never applied cosmetic styles to grid elements/wrappers anyway, only their children. So i find the switch from display: block; float:left; to display: inline-block; in inuit 5.0 a little frustrating :/

I'm hoping i can get away with overriding .grid__item on this project by simply adding the following after the inclusion of inuit without any repercussions:

.grid__item {
    float: left;
    display: block;
}

Although i have a feeling it'll affect something further down the line...

kevva commented 11 years ago

@KingScooty, https://github.com/csswizardry/inuit.css/issues/170#issuecomment-14859371.

euantorano commented 11 years ago

Just want to add a +1 for documenting this. Been using inuit for a while (old versions). Just today started a new project on the new version and I was stumped till I found this issue and added the comments.

aniketpant commented 11 years ago

Finally someone created an issue for this. Even I was struck by the same problem like most of you because of the white-space problem. And then I looked around the web a lot to solve the issue but I couldn't help but notice that @csswizardry had added html comments in the example. On doing that I was able to fix the breaking layout in most places except for one.

This issue should definitely be documented in _grids.scss

rosslavery commented 11 years ago

Just to add my $0.02, not a big fan of the markup needed to make the new grid system work.

I understand why it's needed, I understand the benefits of inline-block, it's just a bit too hacky for my tastes.

That said, I just have a copy of _grids.scss from an older version that I just @import on top afterwards, so it's not the end of the world. I just wonder whether this really is the best "smart default" to have.

No biggie either way, just thought I'd throw that into the mix.

kevva commented 11 years ago

@aniketpant, it is documented in _grids.scss, have a look at the comments. Maybe it could be more clear though.

@rabhw, as I've said previously, it's very easy to extend the grid so you won't have to use HTML comments. See this.

dotnetCarpenter commented 11 years ago

One could apply white-space: nowrap;to a parent element. That would prevent the wrapping but would still preserve the unwanted space. It's late here and I'm not trying to come up with a solution, I'm just saying...

dotnetCarpenter commented 11 years ago

Hey one last thing. I just found that someone requested a way to discard white-space over at W3C

There have been requests for the ability to "discard" white space; the current definition has no facility for this.

dotnetCarpenter commented 11 years ago

I couldn't let go and wrote on the CSS3 mailing list, in continuation of this request http://lists.w3.org/Archives/Public/www-style/2013Mar/0718.html, http://lists.w3.org/Archives/Public/www-style/2013Apr/0451.html

You're welcome to join the discussion.

chronick commented 11 years ago

Hi Everyone, I have been trying out Inuit for a new project and ran into this problem myself. Using an HTML minifier works great, but you must make sure that ALL spaces are removed between tags. However, this presents a potential problem when you want spaces between content and inline tags such as <span>, <em>, etc, since the spaces are stripped from those tags as well. Theoretically, a 'smart' HTML minifier could just remove the spaces between the tags that you want, but I do not have one like that available to me with my toolset.

A workaround to this issue that seems effective is to put explicit &nbsp; entities in places you definitely want spaces between tags. However, depending on the minifier you use, your mileage may vary. I just thought I would post this in case it helps anyone.

Ultimately, with this solution you must outweigh the inconvenience of the comments between the tags and that of the occasional &nbsp;'s in your markup.

I am using Middleman to generate my site for this project, with the Middleman HTML Minifier as the minifer.

Here is the issue discussing the problem I outlined above.

emagnier commented 11 years ago

I'm personally a big fan of inline-block for grids, because it give a more powerful, elegant and flexible alignment approach. And we don't loose the margin collapsing due to the clearfix in floated grids.

E.g.: coupled with the justified CSS grid technique, you can make very powerful layouts.

About the space between blocks, I'm also not very comfortable to be dependent of the HTML comments, but instead I like the font-size: 0 technique.

So in place of maintain another grid system, you could simply complete the actual grid with this:

.grid {
    font-size: 0;
    > * {font-size: $base-font-size;}
    .grid {font-size: 0;}
}

Maybe the best approach would be to have a boolean in the _vars.scss that activates this technique by default, and/or have a BEM modifier .grid--without-space for the specific cases.

Fire-Dragon-DoL commented 11 years ago

Took me a lot, as a newbie to the framework, to figure out that the comments were required (I thought it was to make code appear more nice for unknown way).

I think, it should be placed in uppercase letter on the top of grid.

Thanks anyway.

larrybotha commented 11 years ago

If you absolutely can't use comments to remove whitespace, GridPak uses a similar system, with the exception that the grid wrapper is clearfixed, and all grid items are floated left.

It's easy enough to extend inuit to do this (I use this method for WordPress galleries so that markup doesn't have to be modified):

.grid--floated {
    @extend .grid;
    @extend .cf;

    & > .grid__item {
        float: left;
    }
}

Using inline-block without floats does, however, provide far more flexibility - top, bottom, and middle alignment of content inside grid__items is a cinch.

emagnier commented 11 years ago

You could use also the font-size:0 technique if you don't want the HTML comments (see my previous post above).

Fire-Dragon-DoL commented 11 years ago

No problem guys, for me is just a matter of documentation, I'm ok by not using whitespaces (my html gets minified, so really it's not an issue).

@emagnier: I already added your hack: I've created a "hacks" file with switches much like inuit, setting $use-grids-whitespace to true in vars.scss include your hack.

Thanks for answers anyway. I like the framework, I don't drop it for such a stupid thing =P

tlgreg commented 11 years ago

Just pointing out the article about alternative solutions to this problem which marclarr also linked, it notes some problems with the font-size: 0 technique, also you can find even more in the comments, so keep in mind those.

herzinger commented 11 years ago

There is no 100% failproof way of doing it without affecting the way you go about something else, and that's why the commenting of spaces is the official workaround 'till this day.

dotnetCarpenter commented 11 years ago

I've developed white-space to solve this exact problem. It's still better to minify your html but it should provide a 100% failproof way of doing it.

perkster commented 11 years ago

Thanks dotnetCarpenter! Good job on the js script. Its minimal enough to want to include it and have it work! Yeah! I'm a theme developer and this has been my Achilles heal!

dotnetCarpenter commented 11 years ago

Thanks @perkster, that's nice to hear. Let me know if you got any issues or feature requests.

mattandrews commented 10 years ago

This issue is enough to stop me using inuit for a new project. It's a shame because I always hear good things about it, but as far as I'm concerned, having to insert comments between divs is a massive hack. Yes, I understand why they're needed and how the layout works with them, but I'd sooner have easier code to maintain (particularly when working with devs who don't specialise in obscure CSS) than preserve some imagined CSS purity or whatever and put up with the hack.

aniketpant commented 10 years ago

@mattandrews It would be better if you create a new issue for this:

While I'm complaining, it'd also be much more usable if the list of objects & asbtractions in _defaults.scss was ordered alphabetically, so when I'm going through the files in objects to work out what they all do, I can easily match those up with the options and turn them on/off.

mattandrews commented 10 years ago

@aniketpant good point, done: https://github.com/csswizardry/inuit.css/issues/275

tlgreg commented 10 years ago

Btw #272 seems like an interesting possible solution, although depending on a font for the layout seems like a weird way to go and font-face FOUT could be a bigger concern.

TheDutchCoder commented 10 years ago

@tlgreg that won't work for IE8, as the URI is too big for IE8 to accept it.

@mattandrews HTML comments should not really be an issue, especially not since your HTML output should be concatenated for production anyway. In my experience it's only an issue with certain CMS's for example, that output code indented for some reason, but I guess it's a personal preference.

At least it's an actual solution to the problem. A zero width font might also work, as long as it's served from the server and not remotely requested (as to not get the FOUC on a grid lay-out, which would cause it to break until the font is loaded remotely).

Kellum's solution is interesting, especially if you build your own font-set, so you can serve it to IE8 as well.

patocallaghan commented 10 years ago

@mattandrews you shouldn't let this stop you using the rest of the inuit.css framework. You don't have to include the grids if you don't want them. Personally I don't like the inline-block approach either and I just override it in my own files to use floats. Or you could just import "insert grid library of choice" and use that instead.

mattandrews commented 10 years ago

@TheDutchCoder I think it's more that having to explain to other devs on the project that we have to write code in this way purely to make the CSS framework work is not a conversation I think will go well. @patocallaghan is right that I could just pull in another grid system, but to be honest I think inuit wasn't suited for me because of the lack of documentation (yes, I know the code is documentation, but there are a billion files and I don't want to read through all of them to find the specific thing I need, especially when some of them have less-than-opaque names). I ended up using Foundation which did what I needed and didn't need extra markup around grids.

aniketpant commented 10 years ago

@mattandrews I think you're going extreme when you talk about less-than-opaque names. The names are self-explanatory. Moreover, if you talk about the philosophy of the framework, inuit.css aims to be a no-forced-design framework, whereas Foundation and Bootstrap (the top choices) force design on you. If you didn't know about this, then you made a mistake while make a choice.

inuit.css is a great framework for designers because you get basic markup and then you have the freedom to extend it but if you're looking for elements like .hero, then it ain't for yoiu.

mattandrews commented 10 years ago

@aniketpant that's my point: I think I chose the wrong tool for the job. But the naming: "marginalia", "beautons", "this or this" etc aren't immediately obvious (or weren't to me).

Anyway: inuit is a good framework and has some brilliant code, it's just not for me and the code for grids in particular put me off. Thanks!

aniketpant commented 10 years ago

@mattandrews I understand your point now. And the grids can be putting off. You might want to try https://github.com/csswizardry/csswizardry-grids which is more powerful but suffers from the same hack.

patocallaghan commented 10 years ago

FYI @mattandrews I created this page for that very reason http://terabytenz.github.io/inuit.css-kitchensink/ . I was having a show-and-tell at work of the different frameworks but found I couldn't present inuit.css easily....

bvdputte commented 10 years ago

very easy fix for this using flexbox: https://github.com/csswizardry/inuit.css/pull/286 (on browsers that support flexbox that is!)

csswizardry commented 10 years ago

Please see #291.

Thank you.