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

Bootstrap 5 grid #28517

Merged
merged 10 commits into from
Mar 10, 2020
Merged

Bootstrap 5 grid #28517

merged 10 commits into from
Mar 10, 2020

Conversation

MartijnCuppens
Copy link
Member

@MartijnCuppens MartijnCuppens commented Mar 16, 2019

Set gutter width in rem instead of px

The gutter width is now 1.5rem instead of 30px. This way we 'll be able to use the spacing utilities to align things with the grid.

The container paddings are now set to 1rem.

Fixes #27072

Change approach on how column paddings are set

The col classes were extended lead to this list of 60 selectors in v4. By using .row > *, we can simplify this a lot. This change will apply paddings to all children of .row. Specificity won't be influenced by this change.

Columns: switch to width instead of flex-basis and max-width

Just using width has the benefit the .col-* classes can be used without the need of a .row class. This is also how the primer works.

This also provides a solution for sizing utilities #21943. Because we only set paddings to direct children of .rows, the cols won't have any padding whenever they're not in a .row.

Closes #28312
Closes #29505

More control over gutter widths & vertical gutters

This PR introduces new responsive gutter classes. There are 3 types of gutter classes available:

  • gx-* classes control the horizontal/column gutter width
  • gy-* classes control the vertical/row gutter width
  • g-* classes control the horizontal & vertical gutter width

These gutter classes can be added to the .row and influence the negative margins on the row and the padding on the columns.

Responsive variants are also available to get control per breakpoint. With this change we might consider ditching (or disable by default) the negative margins which increase our file size quite a lot.

How do the gutters work?

The way the gutters are set in horizontal direction is kept the same as in v4 (negative margins on the row and paddings on the columns). The vertical gutters work a little different. Margin is added to the top of each column and to counteract the top margin, a negative margin is added to the row. We use margins instead of paddings to prevent overlapping issues (like we have with the horizontal paddings).

New .row-cols-auto

Fixes #29866

Removal of .form-row

.form-rows had a smaller gutter width, but since we now have the gutter classes, we can use them for even more control over the gutter widths.

Removal of .form-inline

.form-inline is removed in favor of the more flexible grid. The children can now be wrapped in .col-md-auto divs when needed. With the gutter classes, we can have easier control over the vertical spacing instead of using the responsive margin utilities.

Remove position: relative from cols

Closes #25254
Closes #26512

Removal of card decks

We currently have as well card decks as the grid system, but our grid offers more responsive control, so there's not really a reason to keep the decks.

Remove global box-sizing reset from bootstrap-grid.css

In bootstrap-grid.css, box-sizing was inherited which introduces this issue: #22872. On the other hand, setting the global box-sizing behaviour can introduce unexpected behaviour for custom CSS. By only adding box-sizing to the columns, we only apply the box-sizing to the elements where it's needed.

Examples

To do:

  • Change default gutter width to 1.5rem
  • Change gutter classes
  • Document breaking changes
  • Update grid documentation
  • Document stand alone .col-* utility classes
  • Update examples
  • .form-inline & .form-row are removed, document it
  • Update stretched link documentation (assumes col has position: relative)
  • Cleanup git

@ysds
Copy link
Member

ysds commented Mar 19, 2019

Umm... using universal selector is one of the most complex things of CSS, so it is very difficult to comment on this.

It looks like there is a dilemma, judging from the difference of the selectors between the 60 classes and the .no-gutters. At least, I agree with the performance problem of the .no-gutters selector. If we take consistency seriously, the changes will be right. There is no problem with the breaking change.

@MartijnCuppens
Copy link
Member Author

Umm... using universal selector is one of the most complex things of CSS, so it is very difficult to comment on this.

What exactly do you mean with complex?

Also just added an edit for .form-row, the same technique from .no-gutters was used there.

@ysds
Copy link
Member

ysds commented Apr 12, 2019

Sorry for late reply.

What exactly do you mean with complex?

It means that it's difficult to understand and use them effect. Universal selectors have the power to easily break other styles.

For example, expects the following case? (Aside from whether the such usage is good or bad):
https://codepen.io/anon/pen/oOwYQO

@max-ci
Copy link

max-ci commented Apr 12, 2019

From the docs:

In a grid layout, content must be placed within columns and only columns may be immediate children of rows.

@MartijnCuppens
Copy link
Member Author

For example, expects the following case? (Aside from whether the such usage is good or bad): codepen.io/anon/pen/oOwYQO

Aside from the fact that's not a good idea, the theming can easily be overridden with a class (even without !important because * has no effect on specificity: https://codepen.io/MartijnCuppens/pen/yroKYz

@mdo
Copy link
Member

mdo commented Apr 17, 2019

Haven't spent nearly enough time in Bootstrap work, but what about something akin to Primer? For example: https://styleguide.github.com/primer/objects/grid/#flexbox-grids. Primer has column classes that only set width in %s and then utilities are used for creating the flexbox behaviors you want.

@MartijnCuppens MartijnCuppens changed the title Simplify grid Simplify grid paddings Apr 18, 2019
@MartijnCuppens
Copy link
Member Author

@mdo, this PR only covers the way paddings are set. We could also change the grid so that the .col-* classes can be used without the need of a .row parent like Primer. This would also provide a solution for #21943.

Shall I extend the scope of this PR and also add that change here?

@mdo
Copy link
Member

mdo commented Apr 22, 2019

Perhaps! What are your thoughts on going that direction? It'd be one of the bigger changes to arguably one of our biggest and most well known features.

@MartijnCuppens
Copy link
Member Author

I've experimented with widths instead of flex-basis and it looks like we can drop the max-widths, gonna check some browsers and test some edge cases, but it looks promising. I'll tweak this PR further later this week!

@MartijnCuppens MartijnCuppens changed the title Simplify grid paddings Bootstrap 5 grid Apr 23, 2019
@jacobmllr95
Copy link
Contributor

Would also close #26512.

@MartijnCuppens MartijnCuppens force-pushed the master-mc-simplify-grid branch 2 times, most recently from 23c44aa to 2b16056 Compare May 21, 2019 17:35
@MartijnCuppens
Copy link
Member Author

Updated the PR and introduced a second grid. The second grid mimics the behaviour of the block gutters of display: grid. Documentation is still lacking, we'll probably need to have a look at how we combine this PR with #28450.

@mdo
Copy link
Member

mdo commented Aug 30, 2019

@MartijnCuppens Looking at this new secondary grid, maybe we just use display: grid? Support appears close enough https://caniuse.com/#feat=css-grid.

@MartijnCuppens
Copy link
Member Author

IE doesn't support auto placement or gutters. Can't do this yet.

Will update this PR soon. Not sure if we should go for the second grid (.grid) or if we should introduce gutter classes in x and y direction. What do you think, @mdo?

@mdo
Copy link
Member

mdo commented Sep 3, 2019

Ah, wasn't aware of those shortcomings. Vertical margins for gutters have been asked in the past, so if that saves on a few selectors (adding .grid to .row), I'm in favor.

@robrez
Copy link

robrez commented Sep 10, 2019

might anything be in the works for the negative margins on .row? I think those exist to offset grids being placed in a container, but the rows/cols otherwise work fine w/o container

@MartijnCuppens MartijnCuppens force-pushed the master-mc-simplify-grid branch 2 times, most recently from 8afbd13 to 80f1bc3 Compare September 21, 2019 15:20
@MartijnCuppens MartijnCuppens force-pushed the master-mc-simplify-grid branch 2 times, most recently from 55c3764 to f0c15c4 Compare September 28, 2019 14:18
Copy link
Member

@ysds ysds left a comment

Choose a reason for hiding this comment

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

@MartijnCuppens I revisited only the parts that I commented on in the previous review, but I haven't see other lines. Rebase does not leave the differences between before and after the review as independent commits, so need to recheck the all differences in the files. This is not a problem for files with few changes, but it is a big task to see all the files that have changed a lot. Keeping the commit log clean is important, but it is helpful to choose rebase or not depending on the situation.

@ysds
Copy link
Member

ysds commented Mar 10, 2020

@MartijnCuppens, @XhmikosR Can we merge this? Are there any remaining tasks?

@XhmikosR
Copy link
Member

This is quite a change and personally I haven't reviewed it. I'd say wait until we get an approval from @mdo before moving forward with this.

@MartijnCuppens
Copy link
Member Author

We might consider a switch to CSS grid now that we've descided to drop IE11 support. Not sure how to provide a solution for col-auto and col though.

Anyway, merging this would be a good base to build upon later on.

Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

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

Couple quick suggestions here after a final review.

site/content/docs/4.3/layout/grid.md Outdated Show resolved Hide resolved
site/content/docs/4.3/layout/grid.md Outdated Show resolved Hide resolved
site/content/docs/4.3/layout/grid.md Outdated Show resolved Hide resolved
@MartijnCuppens
Copy link
Member Author

:shipit:

@MartijnCuppens MartijnCuppens merged commit 2a2b0b5 into master Mar 10, 2020
@MartijnCuppens MartijnCuppens deleted the master-mc-simplify-grid branch March 10, 2020 19:30
@mdo
Copy link
Member

mdo commented Mar 10, 2020

🙌🏻👏🏻🎉

@michaelrobertson-fico-com

We're all looking forward to seeing IE11 disappear over the horizon. I'm repeating myself but I love the flexbox utility classes, and wish we had the same utilities available for CSS Grid. We're open to an asbstraction of CSS Grid but not sure we'd implement another approach to grids, even as an interim step. Our new design system is heavily invested in web standards, semantic markup and accessibility, and we have application developers already creating PoCs with CSS Grid. Appreciate everyone's work on making BS better.

// later on to override this initial width.
flex-shrink: 0;
Copy link

Choose a reason for hiding this comment

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

Hi all: while I understand the rational for dropping position: relative, can you comment on why you added flex-shrink: 0? If would be great if you could respond to the following discussion thread. Thanks!

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