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

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

Open
3 tasks done
bbottema opened this issue Mar 18, 2024 · 7 comments · May be fixed by #39873
Open
3 tasks done

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

bbottema opened this issue Mar 18, 2024 · 7 comments · May be fixed by #39873

Comments

@bbottema
Copy link

bbottema commented Mar 18, 2024

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

Copy link
Contributor

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.

@XhmikosR
Copy link
Member

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

@maleeb
Copy link

maleeb commented Mar 22, 2024

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

@justin-oh
Copy link

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

@XhmikosR
Copy link
Member

XhmikosR commented Apr 5, 2024

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

@julien-deramond
Copy link
Member

julien-deramond commented Apr 5, 2024

I was waiting for a reproducible example so far. I'll try to look into it as soon as possible manually based on #39798 (comment). 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).

@justin-oh
Copy link

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs review
Development

Successfully merging a pull request may close this issue.

5 participants