Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Bootstrap 5 grid#28517

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
MartijnCuppens merged 10 commits intomasterfrommaster-mc-simplify-grid
Mar 10, 2020
Merged

Bootstrap 5 grid#28517

MartijnCuppens merged 10 commits intomasterfrommaster-mc-simplify-grid
Mar 10, 2020

Conversation

MartijnCuppens
Copy link
Member

@MartijnCuppensMartijnCuppens commentedMar 16, 2019
edited
Loading

Set gutter width inrem instead ofpx

The gutter width is now1.5rem instead of30px. This way we 'll be able to use the spacing utilities to align things with the grid.

The container paddings are now set to1rem.

Fixes#27072

Change approach on how column paddings are set

The col classes were extended lead to this list of60 selectors inv4. 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 towidth instead offlex-basis andmax-width

Just using width has the benefit the.col-* classes can be used without the need of a.row class. This is also how theprimer 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.

Removeposition: 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 globalbox-sizing reset frombootstrap-grid.css

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

Examples

To do:

  • Change default gutter width to1.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
  • Updatestretched link documentation (assumes col hasposition: relative)
  • Cleanup git

klihelp, josephfusco, afuno, skeets23, tkrotoff, mikepyson, robrez, bibs2091, johannschopplich, adrianjguerrero, and 6 more reacted with thumbs up emojirjoly and yuhengliang reacted with hooray emojiJohann-S, afuno, dvmuccillo, andresgalante, and gavin310 reacted with heart emoji
@ysds
Copy link
Contributor

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
MemberAuthor

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 withcomplex?

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

@ysds
Copy link
Contributor

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

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
MemberAuthor

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 commentedApr 17, 2019
edited
Loading

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 setwidth in%s and then utilities are used for creating the flexbox behaviors you want.

klihelp reacted with thumbs up emojiGlenn-Domin reacted with thumbs down emojiwellington1993 reacted with hooray emojiwellington1993 reacted with heart emoji

@MartijnCuppensMartijnCuppens changed the titleSimplify gridSimplify grid paddingsApr 18, 2019
@MartijnCuppens
Copy link
MemberAuthor

@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 commentedApr 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
MemberAuthor

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!

klihelp reacted with hooray emoji

@MartijnCuppensMartijnCuppens changed the titleSimplify grid paddingsBootstrap 5 gridApr 23, 2019
@jacobmllr95
Copy link
Contributor

Would also close#26512.

MartijnCuppens reacted with thumbs up emoji

@MartijnCuppensMartijnCuppensforce-pushed themaster-mc-simplify-grid branch 2 times, most recently from23c44aa to2b16056CompareMay 21, 2019 17:35
@MartijnCuppens
Copy link
MemberAuthor

Updated the PR and introduced a second grid. The second grid mimics the behaviour of the block gutters ofdisplay: 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 commentedAug 30, 2019

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

@MartijnCuppens
Copy link
MemberAuthor

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 commentedSep 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

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

@MartijnCuppensMartijnCuppensforce-pushed themaster-mc-simplify-grid branch 2 times, most recently from8afbd13 to80f1bc3CompareSeptember 21, 2019 15:20
@MartijnCuppensMartijnCuppensforce-pushed themaster-mc-simplify-grid branch 3 times, most recently fromf0c15c4 toead1e64CompareSeptember 28, 2019 14:30
@ysds
Copy link
Contributor

@twbs/css-review I have read through all the changes! Please see#28517 (comment)

@MartijnCuppensMartijnCuppensforce-pushed themaster-mc-simplify-grid branch 2 times, most recently from0dd036f tofad767bCompareFebruary 21, 2020 22:37
Copy link
Contributor

@ysdsysds 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.

MartijnCuppens reacted with thumbs up emoji
@ysds
Copy link
Contributor

@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
MemberAuthor

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

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

ysds, mohe2015, and ffoodd reacted with thumbs up emojimohe2015, michaelrobertson-fico-com, and ffoodd reacted with heart emoji

Copy link
Member

@mdomdo left a comment
edited
Loading

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.

@MartijnCuppens
Copy link
MemberAuthor

:shipit:

@MartijnCuppensMartijnCuppens merged commit2a2b0b5 intomasterMar 10, 2020
@MartijnCuppensMartijnCuppens deleted the master-mc-simplify-grid branchMarch 10, 2020 19:30
@mdo
Copy link
Member

mdo commentedMar 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.

MartijnCuppens, mohe2015, bbugh, and kerryj89 reacted with heart emoji

// always setting `width: 100%;`. This works because weuse `flex` values
// always setting `width: 100%;`. This works because weset the width
// later on to override this initial width.
flex-shrink:0;
Copy link
Contributor

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 droppingposition: relative, can you comment on why you addedflex-shrink: 0? If would be great if you could respond to the following discussion thread. Thanks!

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

@mdomdomdo left review comments

+3 more reviewers

@chalinchalinchalin left review comments

@ysdsysdsysds approved these changes

@auvipyauvipyauvipy approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
14 participants
@MartijnCuppens@ysds@max-ci@mdo@jacobmllr95@robrez@midzer@florianlacreuse@michaelrobertson-fico-com@voltaek@micka-fdz@XhmikosR@chalin@auvipy

[8]ページ先頭

©2009-2025 Movatter.jp