v4: Follow-up grid fixes #20349

Merged
merged 3 commits into from Jul 24, 2016

Projects

None yet

3 participants

@mdo
Member
mdo commented Jul 24, 2016 edited

This PR restores that v3-like grid system goodness that I fucked up in #19099.

The mistake I made in that PR was simplifying things too much; the single mixin approach meant every grid tier class received their position, min-height, and padding-left/-right within their media queries. With this PR, we're back to setting all that grid column prep separately, outside the media queries, for a simpler and more familiar approach. The two grid mixins are now make-col-prep and make-col.

As part of this, I've tried to prevent the need for an xs class for the flexbox grid with a new width: 100% on that grid prep mixin. This "initial" width is then overridden by the flex values for each grid class within each grid media query. I mention this because it's the whole reason I went down the path I did with these mixins in #19099.

Worth noting I've also updated the docs the grid mixins accordingly.

Fixes #19803 and #20349.

/cc @twbs/team for sanity checks.

@mdo mdo Follow-up to #19099 for grid fixes
- Restores two-mixin approach to generating semantic grid columns (now with 'make-col-ready' and 'make-col')
- Removes need for .col-xs-12 by restoring the mass list of all grid tier classes to set position, min-height, and padding
- Adds an initial 'width: 100%' to flexbox grid column prep (later overridden by the column sizing in 'flex' shorthand or 'width') to prevent flexbox columns from collapsing in lower viewports
a8879c8
@mdo mdo added css v4 labels Jul 24, 2016
@mdo mdo added this to the v4.0.0-alpha.3 milestone Jul 24, 2016
@houndci-bot houndci-bot commented on an outdated diff Jul 24, 2016
scss/mixins/_grid-framework.scss
@@ -4,22 +4,32 @@
// any value of `$grid-columns`.
@mixin make-grid-columns($columns: $grid-columns, $gutter: $grid-gutter-width, $breakpoints: $grid-breakpoints) {
+
+ // Common properties for all breakpoints
+ %grid-column {
+ position: relative;
+ // Prevent columns from collapsing when empty
+ min-height: 1px;
+ // Inner gutter via padding
+ padding-left: ($gutter / 2);
@houndci-bot
houndci-bot Jul 24, 2016

Properties should be ordered position, min-height, padding-right, padding-left

@mdo mdo linting
cdc55a4
@mdo
Member
mdo commented Jul 24, 2016

Here's part of the diff showing this in action for those following along at home. Unsure what's up with that outline business though O_O.

➜ ~/work/bootstrap (v4-grid-fixes) git diff dist/css/bootstrap.css
diff --git a/dist/css/bootstrap.css b/dist/css/bootstrap.css
index 40ab9a9..05299dd 100644
--- a/dist/css/bootstrap.css
+++ b/dist/css/bootstrap.css
@@ -392,7 +392,6 @@ a:focus, a:hover {
 }

 a:focus {
-  outline: thin dotted;
   outline: 5px auto -webkit-focus-ring-color;
   outline-offset: -2px;
 }
@@ -812,110 +811,69 @@ pre code {
   clear: both;
 }

-.col-xs-1 {
+.col-xs-1, .col-xs-2, .col-xs-3, .col-xs-4, .col-xs-5, .col-xs-6, .col-xs-7, .col-xs-8, .col-xs-9, .col-xs-10, .col-xs-11, .col-xs-12, .col-sm-1, .col-sm-2, .col-sm-3, .col-sm-4, .col-sm-5, .col-sm-6, .col-sm-7, .col-sm-8, .col-sm-9, .col-sm-10, .col-sm-11, .col-sm-12, .col-md-1, .col-md-2, .col-md-3, .col-md-4, .col-md-5, .col-md-6, .col-md-7, .col-md-8, .col-md-9, .col-md-10, .col-md-11, .col-md-12, .col-lg-1, .col-lg-2, .col-lg-3, .col-lg-4, .col-lg-5, .col-lg-6, .col-lg-7, .col-lg-8, .col-lg-9, .col-lg-10, .col-lg-11, .col-lg-12, .col-xl-1, .col-xl-2, .col-xl-3, .col-xl-4, .col-xl-5, .col-xl-6, .col-xl-7, .col-xl-8, .col-xl-9, .col-xl-10, .col-xl-11, .col-xl-12 {
   position: relative;
   min-height: 1px;
-  padding-right: 15px;
   padding-left: 15px;
+  padding-right: 15px;
+}
+
+.col-xs-1 {
   float: left;
   width: 8.333333%;
 }

 .col-xs-2 {
-  position: relative;
-  min-height: 1px;
-  padding-right: 15px;
-  padding-left: 15px;
   float: left;
   width: 16.666667%;
 }

 .col-xs-3 {
-  position: relative;
-  min-height: 1px;
-  padding-right: 15px;
-  padding-left: 15px;
   float: left;
   width: 25%;
 }

 .col-xs-4 {
-  position: relative;
-  min-height: 1px;
-  padding-right: 15px;
-  padding-left: 15px;
   float: left;
   width: 33.333333%;
 }

 .col-xs-5 {
-  position: relative;
-  min-height: 1px;
-  padding-right: 15px;
-  padding-left: 15px;
   float: left;
   width: 41.666667%;
 }

 .col-xs-6 {
-  position: relative;
-  min-height: 1px;
-  padding-right: 15px;
-  padding-left: 15px;
   float: left;
   width: 50%;
 }

 .col-xs-7 {
-  position: relative;
-  min-height: 1px;
-  padding-right: 15px;
-  padding-left: 15px;
   float: left;
   width: 58.333333%;
 }

 .col-xs-8 {
-  position: relative;
-  min-height: 1px;
-  padding-right: 15px;
-  padding-left: 15px;
   float: left;
   width: 66.666667%;
 }

 .col-xs-9 {
-  position: relative;
-  min-height: 1px;
-  padding-right: 15px;
-  padding-left: 15px;
   float: left;
   width: 75%;
 }

 .col-xs-10 {
-  position: relative;
-  min-height: 1px;
-  padding-right: 15px;
-  padding-left: 15px;
   float: left;
   width: 83.333333%;
 }

 .col-xs-11 {
-  position: relative;
-  min-height: 1px;
-  padding-right: 15px;
-  padding-left: 15px;
   float: left;
   width: 91.666667%;
 }

 .col-xs-12 {
-  position: relative;
-  min-height: 1px;
-  padding-right: 15px;
-  padding-left: 15px;
   float: left;
   width: 100%;
 }
@@ -1070,98 +1028,50 @@ pre code {

 @media (min-width: 544px) {
   .col-sm-1 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 8.333333%;
   }
   .col-sm-2 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 16.666667%;
   }
   .col-sm-3 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 25%;
   }
   .col-sm-4 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 33.333333%;
   }
   .col-sm-5 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 41.666667%;
   }
   .col-sm-6 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 50%;
   }
   .col-sm-7 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 58.333333%;
   }
   .col-sm-8 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 66.666667%;
   }
   .col-sm-9 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 75%;
   }
   .col-sm-10 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 83.333333%;
   }
   .col-sm-11 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 91.666667%;
   }
   .col-sm-12 {
-    position: relative;
-    min-height: 1px;
-    padding-right: 15px;
-    padding-left: 15px;
     float: left;
     width: 100%;
   }
@@ -1283,98 +1193,50 @@ pre code {
@bpierson

Looks like that will do the trick!

@mdo mdo Merge branch 'v4-dev' into v4-grid-fixes
b36e6da
@mdo
Member
mdo commented Jul 24, 2016

According to the v4 PR (#17021), we haven't had a passing CI in awhile. Going to merge this for now and deal with that later.

@mdo mdo merged commit ed9977c into v4-dev Jul 24, 2016

1 of 4 checks passed

continuous-integration/travis-ci/push The Travis CI build failed
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
savage Savage has initiated its special separate Travis CI build
hound No violations found. Woof!
@mdo mdo deleted the v4-grid-fixes branch Jul 24, 2016
@mdo mdo added a commit that referenced this pull request Jul 27, 2016
@mdo mdo One more follow up to #19099, #20349, and #20361
Remove mention of base class and fix grid examples
ded1bf9
@sbleon sbleon referenced this pull request in twbs/bootstrap-rubygem Aug 1, 2016
Closed

.col-xs-12 now required on every responsive col? #48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment