Skip to content

Support $grid-gutter-widths for responsive gutter widths#24620

Closed
gloaec wants to merge 4 commits intotwbs:v4-devfrom
gloaec:v4-dev
Closed

Support $grid-gutter-widths for responsive gutter widths#24620
gloaec wants to merge 4 commits intotwbs:v4-devfrom
gloaec:v4-dev

Conversation

@gloaec
Copy link
Copy Markdown

@gloaec gloaec commented Oct 31, 2017

This is actually a missing feature that appears on the doc:

Variable : $grid-gutter-widths
https://v4-alpha.getbootstrap.com/layout/grid/#variables

This approach aims to define different gutter widths depending on the defined breakpoints.


.row {
margin-left: ($gutter / -2);
margin-right: ($gutter / -2);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Expected "margin-right" to come before "margin-left" order/properties-order

.col#{$infix},
.col#{$infix}-auto {
padding-left: ($gutter / 2);
padding-right: ($gutter / 2);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Expected "padding-right" to come before "padding-left" order/properties-order

@for $i from 1 through $columns {
.col#{$infix}-#{$i} {
padding-left: ($gutter / 2);
padding-right: ($gutter / 2);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Expected "padding-right" to come before "padding-left" order/properties-order

@gloaec
Copy link
Copy Markdown
Author

gloaec commented Oct 31, 2017

I read from other PRs that this feature was removed because it generates to much code (~5k). Maybe there is a way to get less code using instead :

@include media-breakpoint-only($breakpoint, $breakpoints) {
  $gutter: map-get($gutters, $breakpoint);

  [class^='col'],
  [class*=' col'] {
    padding-right: ($gutter / 2);
    padding-left: ($gutter / 2);
  }

  .row {
    margin-right: ($gutter / -2);
    margin-left: ($gutter / -2);
  }
}

@andresgalante
Copy link
Copy Markdown
Collaborator

Hi @gloaec thanks a lot for your contribution.

The reference to the docs you provide is for an alpha release, beta docs are now here:
http://getbootstrap.com/docs/4.0/getting-started/introduction/

As you mention, the responsive gutters were removed here: b013b98

If we want to prosigue with getting them back, it should also be introduce back to the docs

And although I honestly don't think this approach solves the problem, and it's missing how to handle the make- mixns. http://getbootstrap.com/docs/4.0/layout/grid/#mixins I do think that
there is value is having responsive gutters.

@dkozickis
Copy link
Copy Markdown

Seems that @mdo idea is to implement responsive gutter width via implementtion similar to .no-gutters as per this #22944

@hrvojegolcic
Copy link
Copy Markdown

Here in documentation it still stands
"Grid gutters now have a Sass map so you can specify specific gutter widths at each breakpoint."

https://getbootstrap.com/docs/4.0/migration/
https://getbootstrap.com/docs/4.1/migration/

I'd assume this is wrong?

@gloaec
Copy link
Copy Markdown
Author

gloaec commented Jun 11, 2018

@hrvojegolcic yup, from my knowledge, responsive gutters were definitely removed. I ended up adding custom sass in my projects (and honestly, since "css flexboxes", there's no real need for the grid system anymore).

I do think that there is value having responsive gutters.

@andresgalante I think it makes sense to a lot of people to have smaller gutters on smaller screens. No hard feelings if the goal is now to have the lightest library, but yeah, the docs need to be updated ;)

Cheers !

@joshwand
Copy link
Copy Markdown
Contributor

Can someone please update the docs to remove this feature if it's not implemented! Went down a rabbit hole today trying to get this to work :(

@hrvojegolcic
Copy link
Copy Markdown

hrvojegolcic commented Jul 31, 2018

While the solution from @gloaec works fine I have noticed that this it messes up the .no-gutters class. So in my case this worked:

// Override breakpoints for certain viewport
@include media-breakpoint-up(lg) {
    .container,
    [class^='col'],
    [class*=' col'] {
        padding-right: floor($cust-grid-gutter-width-lg-up / 2);
        padding-left: ceil($cust-grid-gutter-width-lg-up / 2);
    }

    .row {
        margin-right: ceil($cust-grid-gutter-width-lg-up / -2);
        margin-left: floor($cust-grid-gutter-width-lg-up / -2);
    }
}

// FIX for .no-gutters class
// Since we overwrite gutters in each viewports we need to repeat this class here
.no-gutters {
    margin-right: 0;
    margin-left: 0;

    > .col,
    > [class*="col-"] {
        padding-right: 0;
        padding-left: 0;
    }
}

Another sideaffect is that this would also match classes like .collapse not only .col so be careful

@lucasklaassen
Copy link
Copy Markdown

@hrvojegolcic nice! Here's the mixin version I used from your example:

@mixin responsive-gutter($size, $gutter-width) {
  @include media-breakpoint-up($size) {
    .container,
    [class^='col'],
    [class*=' col'] {
      padding-right: floor($gutter-width / 2);
      padding-left: ceil($gutter-width / 2);
    }

    .row {
      margin-right: ceil($gutter-width / -2);
      margin-left: floor($gutter-width / -2);
    }
  }
}

@include responsive-gutter(xs, 16px);
@include responsive-gutter(sm, 20px);
@include responsive-gutter(md, 28px);

@mdo
Copy link
Copy Markdown
Member

mdo commented Aug 1, 2018

We're skipping on new grid gutters given our newly added negative margins. See #26825.

@mdo mdo closed this Aug 1, 2018
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.

9 participants