Force left auto margin to be applied for modal and offcanvas header close buttons

[julien-deramond]

Description

This PR fixes the use case described in #39798 (comment) after a modification done in #39373.

IMHO, we don't need to reintroduce justify-content: space-between. Without justify-content: space-between, it offers more possibilities in terms of compositions for the modals and offcanvases headers. The issue seems to be limited to the special use case described in #39798 (comment).

Whenever $modal-inner-padding is set to 0 (or $modal-header-padding-y and $modal-header-padding-x are set to 0), the following shorthand margin rule doesn't apply the auto margin left rule:

margin: calc(-.5* var(--bs-modal-header-padding-y)) calc(-.5* var(--bs-modal-header-padding-x)) calc(-.5* var(--bs-modal-header-padding-y)) auto;

It works manually when doing:

margin: 0 0 0 auto;

I suspect an issue with the combination of calc and the actual values of the custom properties that could lead maybe to something kind of undefined or invalid. All the intermediate values in the shorthand rule are maybe undefined or invalid, making the entire rule invalid or equal to 0; auto not being applied for the left-margin. It is evaluated as something like margin: 0; in the end.

When replacing the shorthand rule by the longhand rule, it seems now to work with the special case setting a 0 value.

Still based on my suspicion, even if the calc rules (when setting a value to 0) are undefined or invalid, the longhand version allows having the right value for each margin-* rules:

// An equivalent `margin` shorthand is not used to ensure that the `auto` left margin is applied correctly
margin-top: calc(-.5 * var(--#{$prefix}modal-header-padding-y)); // Even if it's undefined or invalid, the browser is probably understanding that as `0`
margin-right: calc(-.5 * var(--#{$prefix}modal-header-padding-x)); // Even if it's undefined or invalid, the browser is probably understanding that as `0`
margin-bottom: calc(-.5 * var(--#{$prefix}modal-header-padding-y)); // Even if it's undefined or invalid, the browser is probably understanding that as `0`
margin-left: auto; // `auto` is always applied

/cc @twbs/css-review for other ideas or a better technical analysis

Type of changes

• Bug fix (non-breaking change which fixes an issue)

Checklist

• I have read the contributing guidelines
• My code follows the code style of the project (using npm run lint)
• N/A My change introduces changes to the documentation
• N/A I have updated the documentation accordingly
• N/A I have added tests to cover my changes
• All new and existing tests passed

Live previews

• https://deploy-preview-39873--twbs-bootstrap.netlify.app/docs/5.3/components/offcanvas/
• https://deploy-preview-39873--twbs-bootstrap.netlify.app/docs/5.3/components/modal/

Testing

• Reproduce locally 5.3.3 introduces breaking style bug in modal-header compared to 5.3.2 #39798 (comment) use case
• Play with all modal/offcanvas header Sass variables and CSS variables to double-check that every use case works as expected

Related issues

Closes #39798

[louismaximepiton]

As far as I can see, the same issue exists for the padding property above your changes.

For instance, if I'm setting $modal-header-padding-y: 0 !default; the x alignment is wrong because the padding of the button disappear just as the comment mentions.

Maybe if we want to resolve this issue, we should consider:

.btn-close {
  padding: calc(var(--#{$prefix}modal-header-padding-y) * .5) calc(var(--#{$prefix}modal-header-padding-x) * .5);
  margin: calc(-.5 * var(--#{$prefix}modal-header-padding-y)) calc(-.5 * var(--#{$prefix}modal-header-padding-x)) calc(-.5 * var(--#{$prefix}modal-header-padding-y)) auto;
  margin-left: auto; // Force the left margin of the close button to auto
}

[julien-deramond]

I don't have time to dig more into this right now, but there's also a possibility that our current version is correct (means that this PR should be closed) and that instead of setting $modal-inner-padding to 0, it must be set to 0px when using it like #39798 (comment):

$modal-inner-padding:  0px !default; // stylelint-disable-line length-zero-no-unit

Without unit added to 0, it's confusing for the browser to handle the calculation between a unitless value and a value. Like it would be an issue with min() and max() functions. See https://drafts.csswg.org/css-values-4/#comp-func where the spec mentions:

For all three functions, the argument calculations can resolve to any , , or , but must have a consistent type or else the function is invalid; the result’s type will be the consistent type.

And https://drafts.csswg.org/css-values-4/#css-consistent-type:

Two or more calculations have a consistent type if adding the types doesn’t result in failure. The consistent type is the result of the type addition.

I'll try to find more time to check that more in detail.

[mdo]

Seems fine to me. Could maybe drop to margin and margin-left for an override, but no pressure either way.