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

Semantic columns do not have alternative gutter-widths per breakpoint. (twbs 3.x) #11873

Closed
duhseekoh opened this issue Dec 13, 2013 · 9 comments

Comments

@duhseekoh
Copy link

Some times I like to provide a different gutter width at different sizes. Some example code.

.myElement {
  .make-xs-column(4, 0);
  .make-sm-column(4, 10px);
}

^ This does not work. It always takes the last applied gutter width, 10px in this example. Clearly it's because inside the mixin, the media query only includes float and width. I can't see a reason this would be? It seems like everything in the .make-**-column would be totally breakpoint dependent.

@mdo
Copy link
Member

mdo commented Dec 14, 2013

I can't see a reason this would be?

Probably because it was an oversight :). I think we should probably include it with v3.1 provided it doesn't break backward compatibility.

@mdo
Copy link
Member

mdo commented Dec 15, 2013

Works fine for me. Just ran it locally and had no problem. The mixins in v3.0.3 include everything you need, including the padding for the gutters.

@mdo mdo closed this as completed Dec 15, 2013
@duhseekoh
Copy link
Author

Right, it includes padding, but do you see that the 10px gutter I specified for sm overwrites the 0px gutter I specified for xs, even if I'm in an xs sized viewport?

// Generate the small columns
.make-sm-column(@columns; @gutter: @grid-gutter-width) {
  position: relative;
  // Prevent columns from collapsing when empty
  min-height: 1px;
  // Inner gutter via padding
  padding-left:  (@gutter / 2);
  padding-right: (@gutter / 2);

  // Calculate width based on number of columns available
  @media (min-width: @screen-sm-min) {
    float: left;
    width: percentage((@columns / @grid-columns));
  }
}

I would expect everything inside the mixin to be within the media query.

@mdo
Copy link
Member

mdo commented Dec 16, 2013

Ah, I don't know that we'd be able to fix that until v4 when we can safely rewrite these mixins. Changing the behavior to make all properties responsive, or even just the padding, might not fly. Would have to experiment more.

@mdo mdo reopened this Dec 16, 2013
@duhseekoh
Copy link
Author

Got it, thanks. Just to reiterate for future discussion it does seem more "natural" that column sizing mixins like that would not create side effects for mixins defined at different viewport sizes.

@mdo
Copy link
Member

mdo commented Dec 16, 2013

@dicicco2 Word, fur sure. It might be a distinction we make for mixins vs. predefined classes. For the classes, it makes a lot of sense so that things naturally scale up (on account of the mobile first approach). I'm torn though because I know folks want that behavior as well.

I had thought LESS supported scoped variables within media queries, but perhaps not. That'd solve this problem entirely.

Either way, not likely something to immediately change. Thanks for chiming in and reiterating <3.

@mdo
Copy link
Member

mdo commented Dec 18, 2013

Punting to v4 to revisit if and when LESS properly supports scoping the variables for this. Right now I don't believe it does after some quick tests. Plus, rewriting those mixins as I mentioned before can't happen until then anyway.

@mdo mdo closed this as completed Dec 18, 2013
@duhseekoh
Copy link
Author

Sounds good. Thanks.

@matplane
Copy link

Waiting for v4 this patch do the job as far as I tested.

.make-sm-column(@columns; @gutter: @grid-gutter-width) {
  @media (min-width: @screen-sm-min) {
    padding-left:  (@gutter / 2);
    padding-right: (@gutter / 2);
  }
}
.make-md-column(@columns; @gutter: @grid-gutter-width) {
  @media (min-width: @screen-md-min) {
    padding-left:  (@gutter / 2);
    padding-right: (@gutter / 2);
  }
}
.make-lg-column(@columns; @gutter: @grid-gutter-width) {
  @media (min-width: @screen-lg-min) {
    padding-left:  (@gutter / 2);
    padding-right: (@gutter / 2);
  }
}

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

3 participants