Simplify make-container()

[ffoodd]

By dividing in the custom property directly:

1. code is shorter,
2. a single stylelint-disable rule
3. easier override for paddings since we only have to change the custom property—without having to think about multiplying what we want by 2.

Any thoughts?

[MartijnCuppens]

Maybe rename the variable to --bs-gutter-x-half or something alike?

[ffoodd]

I don't think so: the value is half the gutter indeed, but we use it directly as a gutter and it's only used in containers. Overriding --bs-gutter-x-half wouldn't make any sense if we set it to $gutter for example.

I'd be in favor of keeping naming as-is, but any other arguments or ideas are welcome :)

[mdo]

Conceptually, I think we'd leave it as-is. The "gutter" is the combination of one column's right padding and the next column's left padding. We just have to divide it by 2 to apply it to both sides of those different columns.

But, if not, would we need to update the .row, too? I'm kinda fine keeping it as-is—does this have much of an impact to performance or anything? Or file size?

bootstrap/scss/mixins/_grid.scss

Lines 5 to 13 in 585b3ec

	@mixin make-row($gutter: $grid-gutter-width) {
	--bs-gutter-x: #{$gutter};
	--bs-gutter-y: 0;
	display: flex;
	flex-wrap: wrap;
	margin-top: calc(var(--bs-gutter-y) * -1); // stylelint-disable-line function-blacklist
	margin-right: calc(var(--bs-gutter-x) / -2); // stylelint-disable-line function-blacklist
	margin-left: calc(var(--bs-gutter-x) / -2); // stylelint-disable-line function-blacklist
	}

[ffoodd]

I don't think it impacts performance in any way; it should decrease file size a bit, but it's probably not valable for a few bytes only.

Your call, IMHO this PR allows simpler override but I don't have any other good argument.

[ffoodd]

@mdo FYI in Boosted gutter depends on breakpoint: https://github.com/Orange-OpenSource/Orange-Boosted-Bootstrap/blob/v5-dev/scss/mixins/_container.scss

If I kept the current implementation, I'd have to multiply $gutter by 2 to have the correct value above breakpoint, which would be like calc(#{$gutter * 2} / 2) which doesn't make any sense.

Anyone in need to customize the gutter should redeclare padding (which feels weird when using custom properties) or take the division into account to virtually get the expected result (which feels weird too).

Does my explanations make any sense?

[mdo]

I can dig it. Should we also do it on the .row though since we have the same calc() function happening there?

[XhmikosR]

FYI this is bigger than the current solution, check the bundlewatch output. Pretty small difference, but still, it doesn't reduce the file size :)

[MartijnCuppens]

Ow, I misunderstood what this PR was doing, the "gutter" term confused me a bit (padding-x was a bit clearer imo).

---

We should do the division by 2 in our _variables file. In the current situation, if I set $container-padding-x to 20px, it would actually be 10px.

bootstrap/scss/_variables.scss

Line 339 in 6c2abf3

$container-padding-x: $grid-gutter-width !default;

---

To save some bytes, we could also make use of the default value parameter:

  padding-right: var(--bs-gutter-x, #{$gutter});
  padding-left: var(--bs-gutter-x, #{$gutter});

[ffoodd]

What a topic :D

Moving the division in our variables seems a good idea.

Using the fallback value too, might be turned into a more generic approach about custom properties, we don't need to declare them actually and it'd still allows easy override while being lighter.

[XhmikosR]

@twbs/css-review so what's the plan here? Should we drop this or is it still valid?

[MartijnCuppens]

The current implementation is wrong, as I explained in #31735 (comment). We should fix this one way or another.

[ffoodd]

@MartijnCuppens I implemented your suggestions, is there anything else missing?

[MartijnCuppens]

Perfect, @ffoodd 👌

[XhmikosR]

This needs a rebase :)