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

Reducing redundancy in output of the semantic grid columns system #13426

Closed
wants to merge 2 commits into from

Conversation

dschenk
Copy link

@dschenk dschenk commented Apr 25, 2014

I often have the following use case when generating semantic columns:

.module {
  .make-xs-column(12);
  .make-sm-column(6);
  .make-md-column(3);
}

Meaning I need a class that fills a variable amount of columns, depending on breakpoints.
Before this patch, the code above would generate the following CSS:

.module {
  position: relative;
  min-height: 1px;
  padding-left: 15px;
  padding-right: 15px;
  float: left;
  width: 100%;
  position: relative;
  min-height: 1px;
  padding-left: 15px;
  padding-right: 15px;
  position: relative;
  min-height: 1px;
  padding-left: 15px;
  padding-right: 15px;
  margin-bottom: 24px;
}

@media (min-width: 768px) {
  .module {
    float: left;
    width: 50%;
  }
}

[etc. for the other media-queries]

I propose to use the grid system in the following way to reduce this redundancy:

.module {
  .make-column();
  .make-xs-column(12);
  .make-sm-column(6);
  .make-md-column(3);
}

or:

.module {
  .make-column();
  .make-md-column(3);
}

This means using the make-column mixin whenever we want to define a semantic column class. In my opinion this disadvantage beats redundant output though.

Your thoughts?

padding-left: (@gutter / 2);
padding-right: (@gutter / 2);
}

// Generate the extra small columns
.make-xs-column(@columns; @gutter: @grid-gutter-width) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

So then there's no point in having the @gutter parameter.

Copy link
Author

Choose a reason for hiding this comment

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

True. Using the system in the following way

.module {
  .make-xs-column(12, 10px);
  .make-sm-column(6, 20px);
  .make-md-column(3, 30px);
}

Did not make sense before the patch. It would just output and use the last given gutter (30px).
If somebody would want to use a custom gutter width she or he could do the following:

.module {
  .make-column(30px)
  .make-md-column(3);
}

We can remove the gutter params for the make-[bp]-column mixins then, right?

@cvrebert cvrebert added the css label Apr 25, 2014
@dschenk
Copy link
Author

dschenk commented May 1, 2014

@cvrebert What do you think?

@cvrebert
Copy link
Collaborator

cvrebert commented May 1, 2014

It's a good idea, but seems like it would break backwards compatibility for mixin-users.

@mdo
Copy link
Member

mdo commented May 1, 2014

Super into this though. I realized I screwed up the mixins far too late. Will slate for v4.

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

Successfully merging this pull request may close these issues.

3 participants