-
Notifications
You must be signed in to change notification settings - Fork 441
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
Margin and padding shim for bootstrap 5 migration #13625
Conversation
$infix: breakpoint-infix($breakpoint, $grid-breakpoints); | ||
|
||
@each $prop, $abbrev in (margin: m, padding: p) { | ||
@each $size, $length in $spacers { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a reference, $spacers
is defined here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With bootstrap 5, those will have to be updated to 861f819, but that's not needed here
@@ -0,0 +1,16 @@ | |||
@each $breakpoint in map-keys($grid-breakpoints) { | |||
@include media-breakpoint-up($breakpoint) { | |||
$infix: breakpoint-infix($breakpoint, $grid-breakpoints); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
breakpoint-infix
is defined upstream in Bootstrap.
In the commits, it would be nice to link to the Bootstrap documentation where those changes are mentioned: https://getbootstrap.com/docs/5.0/migration/#utilities |
@@ -0,0 +1,16 @@ | |||
@each $breakpoint in map-keys($grid-breakpoints) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did you come up with this Sass code? It looks like something from upstream which was slightly adapted. I'm asking to understand and also to compare with how Bootstrap does this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed I just copied over code from https://github.com/twbs/bootstrap/blob/v4-dev/scss/utilities/_spacing.scss and modified it to replace r and l to e and s
55ce1e2
to
78f12d7
Compare
This PR will need to be rebased to fix conflicts. |
cb44445
to
f387ce7
Compare
f387ce7
to
c0c2924
Compare
This is a part of #13602, updating all existing margin and padding to use s and e instead of l and r