bootstrap icon indicating copy to clipboard operation
bootstrap copied to clipboard

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

Open bbottema opened this issue 1 year ago • 7 comments

Prerequisites

Describe the issue

Due to differences between NPM versions with me and our colleagues, we accidentally discovered a breaking style bug between 5.3.2 and 5.3.3, which is that .modal-header { } is missing the property justify-content: space-between;, causing headers in our modal dialogues to collapse. Pinning the version to 5.3.2 fixes it.

5.3.2:

.modal-header {
    display: flex;
    flex-shrink: 0;
    align-items: center;
    justify-content: space-between; /* missing in 5.3.3 */
    padding: var(--bs-modal-header-padding);
    border-bottom: var(--bs-modal-header-border-width) solid var(--bs-modal-header-border-color);
    border-top-left-radius: var(--bs-modal-inner-border-radius);
    border-top-right-radius: var(--bs-modal-inner-border-radius);
}

5.3.3:

.modal-header {
    display: flex;
    flex-shrink: 0;
    align-items: center;
    padding: var(--bs-modal-header-padding);
    border-bottom: var(--bs-modal-header-border-width) solid var(--bs-modal-header-border-color);
    border-top-left-radius: var(--bs-modal-inner-border-radius);
    border-top-right-radius: var(--bs-modal-inner-border-radius);
}

Reduced test cases

N/A

What operating system(s) are you seeing the problem on?

Windows

What browser(s) are you seeing the problem on?

Chrome

What version of Bootstrap are you using?

5.3.3

bbottema avatar Mar 18 '24 14:03 bbottema

Hello @bbottema. Bug reports must include a live demo of the issue. Per our contributing guidelines, please create a reduced test case on CodePen or StackBlitz and report back with your link, Bootstrap version, and specific browser and Operating System details.

github-actions[bot] avatar Mar 18 '24 16:03 github-actions[bot]

This was introduced in #39373. /CC @mdo @julien-deramond

XhmikosR avatar Mar 19 '24 14:03 XhmikosR

I currently use justify-content-between on modal-header as a workaround.

maleeb avatar Mar 22 '24 11:03 maleeb

This broke for me in potentially a different way when I went from 5.3.2 to 5.3.3.

These are the pertinent variables as set by Bootstrap:

$modal-inner-padding:               $spacer !default;
$modal-header-padding-y:            $modal-inner-padding !default;
$modal-header-padding-x:            $modal-inner-padding !default;
$modal-header-padding:              $modal-header-padding-y $modal-header-padding-x !default;

This is how I overwrote those variables when using 5.3.2:

$modal-inner-padding:               0;
$modal-header-padding:              $spacer;

I found I had to make the following update:

$modal-inner-padding:               0;
$modal-header-padding-y:            $spacer;
$modal-header-padding-x:            $spacer;

I don't understand CSS enough to know why, but if either of --bs-modal-header-padding-x or --bs-modal-header-padding-y are 0, then the following doesn't work:

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;

In my case both were 0, which should make the above effectively margin: 0 0 0 auto. Again, I don't understand why, but if I override the property to literally margin: 0 0 0 auto, it works.

You can see this in action by fiddling around in the inspector of https://getbootstrap.com/docs/5.3/components/modal/.

justin-oh avatar Apr 05 '24 17:04 justin-oh

@mdo @julien-deramond if you guys don't have time, perhaps we could just revert the offending patch next week and release a new version.

XhmikosR avatar Apr 05 '24 17:04 XhmikosR

I was waiting for a reproducible example so far. I'll try to look into it as soon as possible manually based on https://github.com/twbs/bootstrap/issues/39798#issuecomment-2040301613. FYI, as we tried to have a consistent approach with modals and offcanvases, it might be more than a revert. I'll keep you in touch (or @mdo if he manages to check this out before me).

julien-deramond avatar Apr 05 '24 17:04 julien-deramond

I can confirm through the same means (fiddling around in the inspector of https://getbootstrap.com/docs/5.3/components/offcanvas/) that the offcanvas button position also fails if either of --bs-offcanvas-padding-x or --bs-offcanvas-padding-y are 0.

But please check for yourself!

justin-oh avatar Apr 05 '24 18:04 justin-oh