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

Long buttons in modal-footer don't start on a new line #24800

Closed
anatolebeuzon opened this issue Nov 16, 2017 · 3 comments · Fixed by #25103
Closed

Long buttons in modal-footer don't start on a new line #24800

anatolebeuzon opened this issue Nov 16, 2017 · 3 comments · Fixed by #25103
Labels

Comments

@anatolebeuzon
Copy link

It looks like there is an issue with buttons when they are used in a modal-footer. If they are too long, they do not start on a new line like it would be expected. Instead, they appear to be out of their container:

bootstrap bug

The reduced case is pretty straightforward, simply copying and pasting the code snippet from the docs reproduces it.

(For the sake of the example, the buttons have very long text, but the bug is also observable with reasonably short button text on small displays such as iPhone 5).

I could observe this behaviour with Safari (mobile & desktop) and Chrome (desktop), so I suppose it is not browser-specific.

@gijsbotje
Copy link
Contributor

gijsbotje commented Nov 16, 2017

adding flex-wrap to the footer will make it go on a new line
note: by default they are positioned on the right
to position them left add justify-content-start to the footer (and to align the buttons add ml-0 to the last button)

demo of what i mean: https://codepen.io/gijsbotje/pen/POJBRp

@anatolebeuzon
Copy link
Author

Thanks @gijsbotje, I didn't know about flex-wrap!
Maybe we should make this the default behaviour for modal-footer?

@MartijnCuppens
Copy link
Member

MartijnCuppens commented Nov 23, 2017

flex-wrap could be added, but there would be an issue with margins between wrapped buttons. @gijsbotje added the .mt-1 class in his demo which works in this case, but this will cause a misalignment when the buttons aren't wrapped anymore.

Right now, this is the SCSS for the .modal-footer:
_modal.scss:

// Footer (for actions)
.modal-footer {
  display: flex;
  align-items: center; // vertically center
  justify-content: flex-end; // Right align buttons with flex property because text-align doesn't work on flex items
  padding: $modal-inner-padding;
  border-top: $modal-footer-border-width solid $modal-footer-border-color;

  // Easily place margin between footer elements
  > :not(:first-child) { margin-left: .25rem; }
  > :not(:last-child) { margin-right: .25rem; }
}

If this is changed to this, all cases are covered:
_variables.scss:

  • Add $modal-footer-margin-between variable
// Padding applied to the modal body
$modal-inner-padding:         1rem !default;

// Margin between elements in footer, must be lower than or equal to 2 * $modal-inner-padding
$modal-footer-margin-between:         .5rem !default;

_modal.scss:

  • Add flex-wrap: wrap; in .modal-footer
  • Remove some padding from .modal-footer
  • Add some margin to .modal-footer children
// Footer (for actions)
.modal-footer {
  display: flex;
  align-items: center; // vertically center
  justify-content: flex-end; // Right align buttons with flex property because text-align doesn't work on flex items
  flex-wrap: wrap;
  padding: $modal-inner-padding - $modal-footer-margin-between / 2;
  border-top: $modal-footer-border-width solid $modal-footer-border-color;

  // Place margin between footer elements
  > * {
    margin: $modal-footer-margin-between / 2;
  }
}

The problem is that a universal selector is used here because it is also possible that .modal-footer can contain other elements. Can we make an exception for this case?

Demo of the solution: https://codepen.io/MartijnCuppens/pen/bYMKXm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants