Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add responsive grids css rules to cssgrids #363

Merged
merged 8 commits into from Jan 4, 2013

Conversation

Projects
None yet
8 participants
Contributor

tilomitra commented Dec 14, 2012

Now that the gridbuilder is out and people seem to be using it, I thought it would be a good idea to take the default CSS rules from there and bring it back into CSSGrids.

This pull request introduces a cssgrids-responsive.css file that concatenates cssgrids-units.css, cssgrids-base.css, and cssgrids-responsive-base.css.

Would love @msweeney to go through this and offer some feedback.

@msweeney msweeney was assigned Dec 14, 2012

Owner

davglass commented Dec 14, 2012

I think he's out until the beginning of the year, so it will need to wait until then.

Contributor

tilomitra commented Dec 14, 2012

That's cool - this is for 3.x anyway.

Member

okuryu commented Dec 15, 2012

Is there user guide about this?

Contributor

tilomitra commented Dec 15, 2012

@okuryu I'll add it to the cssgrids docs/ directory but the concept is the same as the gridbuilder docs

@ericf ericf commented on an outdated diff Dec 17, 2012

src/cssgrids/tests/manual/responsive.html
+ <div class="yui3-u-1-3">
+ <div class="cell">
+ <h4>Thirds</h4>
+ <p>This cell will be a grid even on mobile devices.</p>
+ </div>
+ </div>
+ <div class="yui3-u-1-3">
+ <div class="cell">
+ <h4>Thirds</h4>
+ <p>This cell will be a grid even on mobile devices.</p>
+ </div>
+ </div>
+ </div>
+
+</body>
+</html>
@ericf

ericf Dec 17, 2012

Owner

Needs a newline.

@ericf ericf commented on an outdated diff Dec 17, 2012

src/cssgrids/css/cssgrids-responsive-base.css
+ }
+ .yui3-hidden-phone {
+ display: none;
+ }
+ .yui3-visible-desktop {
+ display: none;
+ }
+}
+@media (min-width:768px) and (max-width:979px) {
+ .yui3-hidden-tablet {
+ display: none;
+ }
+ .yui3-visible-desktop {
+ display: none;
+ }
+}
@ericf

ericf Dec 17, 2012

Owner

Needs a newline.

Contributor

msweeney commented Dec 17, 2012

+1 Looks good.

Contributor

tilomitra commented Dec 18, 2012

@msweeney Thanks! 48 more hours until lazy consensus.

Owner

davglass commented Dec 18, 2012

I'm good now that Matt signed off.

Contributor

tilomitra commented Dec 18, 2012

W00t - let me add the docs in as @okuryu suggested and then I'll merge it in to dev-3.x.

Member

okuryu commented Dec 18, 2012

@tilomitra Thanks!

@ericf ericf and 1 other commented on an outdated diff Dec 18, 2012

src/cssgrids/css/cssgrids-responsive-base.css
@@ -0,0 +1,44 @@
+.yui3-g-responsive {
+ letter-spacing: -0.31em;
+ *letter-spacing: normal;
+ word-spacing: -0.43em;
+}
@ericf

ericf Dec 18, 2012

Owner

What's the thought behind repeating the styles that are already defined in .yui3-g? Could we simply require that you tag an element with our standards grid class, and that the responsive stuff applies to the contents within a grid?

@tilomitra

tilomitra Dec 18, 2012

Contributor

The reason we didn't go with that is so that developers can mix and match responsive and non-responsive behaviors on a page. Stuff inside a .yui3-g-responsive automatically collapse to 100%, but stuff inside .yui3-g doesn't (which is useful if you want to have a 3x3 grid of images on a mobile device or something).

(Edited for clarity)

@ericf

ericf Dec 18, 2012

Owner

Was the thinking that requiring both class names is confusing?

Going with @msweeney's idea of yui3-g-r:

<div class="yui3-g yui3-g-r"></div>

I guess the above does seem redundant :-/

@ericf ericf and 5 others commented on an outdated diff Dec 18, 2012

src/cssgrids/css/cssgrids-responsive-base.css
@@ -0,0 +1,44 @@
+.yui3-g-responsive {
@ericf

ericf Dec 18, 2012

Owner

For the sake of typing (and bytes) can we go with yui3-rg as the classname?

Maybe there was a discussion about this that I messed…

@tilomitra

tilomitra Dec 18, 2012

Contributor

Let me see how many bytes we save doing that. Newbies coming to yui3 grids from other grids have often told me they get confused with the g and u syntax. I was actually thinking a better naming convention would be:

y-row, y-col-1-3, y-row-responsive. I feel this would be less typing and easier to read semantically than yui3-g, yui3-u-1-3, yui3-g-responsive. What are your thoughts?

One of the reasons the GridBuilder exists is to allow people to customize these prefixes as they see fit.

@rgrove

rgrove Dec 18, 2012

Contributor

👍 to "row" and "col". That makes it easier to visualize what the names are for, which in turn makes it easier to remember how to use them.

@tilomitra

tilomitra Dec 18, 2012

Contributor

@rgrove @ericf @msweeney Thoughts on changing from yui3 to y?

@rgrove

rgrove Dec 18, 2012

Contributor

👍 to that too. Shorter is better, and I don't think we're likely to release YUI 4 anytime soon and cause a naming collision.

@brianjmiller

brianjmiller Dec 18, 2012

Contributor

Is the rest of the library going to change too? IOW, are widgets now going to be y-[widget name]-content, etc.? I'm all for short but really against inconsistency.

@tilomitra

tilomitra Dec 18, 2012

Contributor

That's a good point. We should probably discuss the naming change in another pull request, since it applies to more than just responsive grids.

@rgrove

rgrove Dec 18, 2012

Contributor

I don't think we need to change the rest of the library. YUI's CSS components tend to be used by lots of people who aren't interested in using YUI's JS components, and it'd be a shame to shove the "yui3-" prefix down their throats just for the sake of consistency.

@msweeney

msweeney Dec 18, 2012

Contributor

+1 to a shorter classname, but yui3-g-r might be more predictable than r-g. -1 to changing grid class names or the library prefix. These changes have broader implications, and should probably be discussed outside of the context of a new cssgrids module.

@ericf

ericf Dec 18, 2012

Owner

I dunno about "row" and "col", it doesn't make sense for nested grids (row in the same row) or cols that are 100% width (it's a row and a col).

@tivac

tivac Dec 18, 2012

Contributor

👎 to changing existing names

👎 as well to "row" & "col".

👍 to @msweeney's .yui3-g-r proposal

Contributor

tilomitra commented Dec 27, 2012

I changed yui3-g-responsive to yui3-g-r as per the earlier discussion. Also added an example, documentation and a basic example test. I'll leave this here for a while to capture any more suggestions before pushing it to dev-3.x.

@tilomitra tilomitra merged commit 5d7d6a5 into yui:dev-3.x Jan 4, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment