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

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

wants to merge 1 commit into
base: main
Choose a base branch


Copy link

@julien-deramond julien-deramond commented Apr 6, 2024


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)


  • 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


Related issues

Closes #39798

@julien-deramond julien-deramond marked this pull request as ready for review April 8, 2024 06:10
@julien-deramond julien-deramond requested a review from a team as a code owner April 8, 2024 06:10
Copy link

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

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

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

Copy link
Member Author

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


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.

@julien-deramond julien-deramond marked this pull request as draft April 8, 2024 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet

Successfully merging this pull request may close these issues.

5.3.3 introduces breaking style bug in modal-header compared to 5.3.2
2 participants