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

Redo jumbotron padding #16477

Merged
merged 1 commit into from
May 14, 2015
Merged

Redo jumbotron padding #16477

merged 1 commit into from
May 14, 2015

Conversation

mdo
Copy link
Member

@mdo mdo commented May 14, 2015

Fixes #16374.

  • In general, there are very few instances that would require a jumbotron without a parent or child container.
  • Right now we account for that behavior though with some horizontal padding on the .jumbotron class.
  • This removes that horizontal padding as it narrows our grid classes unnecessarily and accounts for a super small use case.
  • It also improves consistency across breakpoints, in that padding isn't being added and removed.
  • In doing so, I also removed the shorthand padding property and went with the specific ones (because yay specificity).

/cc @rlindner81

- In general, there are very few instances that would require a jumbotron without a parent or child container.
- Right now we account for that behavior though with some horizontal padding on the .jumbotron class.
- This removes that horizontal padding as it narrows our grid classes unnecessarily and accounts for a super small use case.
- It also improves consistency across breakpoints, in that padding isn't being added and removed.
- In doing so, I also removed the shorthand padding property and went with the specific ones (because yay specificity).
@mdo mdo added the css label May 14, 2015
@mdo mdo added this to the v3.3.5 milestone May 14, 2015
mdo added a commit that referenced this pull request May 14, 2015
@mdo mdo merged commit 801d6bd into master May 14, 2015
@mdo mdo deleted the fix_16374 branch May 14, 2015 17:31
@cvrebert cvrebert mentioned this pull request May 14, 2015
@rlindner81
Copy link
Contributor

@mdo Awesome 👍

@inopinatus
Copy link

This was a surprise in 3.3.5. Removing the left padding on mobile devices leaves the jumbotron content looking unbalanced to me.

Case in point, see http://getbootstrap.com/components/#jumbotron with a narrow viewport or mobile device.

@rlindner81
Copy link
Contributor

@inopinatus Good point. There are 2 main usecases for jumbotrons (inner container, outer container). We made the first act consistently and sensibly relative to the grid layout, but the second was broken.

The fix is easy enough though, should amount to changing jumbotron.less line 28ff to

  .container &,
  .container-fluid & {
    border-radius: @border-radius-large; // Only round corners at higher resolutions if contained in a container
    padding-left:  (@grid-gutter-width / 2);
    padding-right: (@grid-gutter-width / 2);
/*
    padding-left:  (@jumbotron-padding * .6);
    padding-right: (@jumbotron-padding * .6);
*/
  }

I would personally prefer that the left/right padding depends on the grid's gutter variable rather than the jumbotron's, but that's up to the maintainers.

See http://jsbin.com/goquvahito/edit?css,output for how the compiled version acts in both cases.

@freddyboucher
Copy link

As @inopinatus said

This was a surprise in 3.3.5. Removing the left padding on mobile devices leaves the jumbotron content looking unbalanced to me.

screen shot 2015-07-07 at 12 42 09

@rlindner81
Copy link
Contributor

@freddyboucher did you read my response? It definitely is a bug. It's easy to fix though.

@freddyboucher
Copy link

@rlindner81
Yes thx, I've just posted a comment because I didn't see it in the 3.3.6 milestone https://github.com/twbs/bootstrap/milestones/v3.3.6

@mdo
Copy link
Member Author

mdo commented Jul 7, 2015

Please open a new issue with a JS Bin example demonstrating the bug and fix <3.

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 this pull request may close these issues.

Jumbotron occupies more horizontal space on smaller screens
4 participants