Skip to content

Commit

Permalink
Scope tablet classes to tablets range only
Browse files Browse the repository at this point in the history 
Fixes #9128
  • Loading branch information
@mdo
mdo committed Aug 6, 2013
1 parent 1d1e21a commit db45a60
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 7 deletions.
2 changes: 1 addition & 1 deletion dist/css/bootstrap.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,7 @@ pre code {
width: 100%;
}

@media (min-width: 768px) {
@media (min-width: 768px) and (max-width: 991px) {
.container {
max-width: 728px;
}
Expand Down
2 changes: 1 addition & 1 deletion dist/css/bootstrap.min.css
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion less/grid.less
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
.col-12 { width: 100%; }

// Small device columns (phones to tablets)
@media (min-width: @screen-tablet) {
@media (min-width: @screen-tablet) and (max-width: @screen-tablet-max) {

This comment has been minimized.

Sign in to view

Copy link
@plankguy

plankguy Aug 8, 2013

Was there major a reason for adding the max-width?
.container {
max-width: @container-tablet;
}
Expand Down
10 changes: 6 additions & 4 deletions less/variables.less
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,16 @@
@screen-medium: 992px;
@screen-desktop: @screen-medium;

// So media queries don't overlap when required, provide a maximum
@screen-small-max: (@screen-medium - 1);
@screen-tablet-max: @screen-small-max;

// Large screen / wide desktop
@screen-large: 1200px;
@screen-large-desktop: @screen-large;

// So media queries don't overlap when required, provide a maximum
@screen-small-max: (@screen-medium - 1);
@screen-tablet-max: (@screen-desktop - 1);
@screen-desktop-max: (@screen-large-desktop - 1);


// Grid system
// --------------------------------------------------

Expand Down

7 comments on commit db45a60

@eratzlaff
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry @mdo If I am bothering. But with this change whee lose the option to use sm and apply it to lg automatically.
example before the change.

<div>
  <div class="col-6 col-sm-4">col-6 col-sm-4</div>
  <div class="col-6 col-sm-8">col-6 col-sm-8</div>
</div>

Now to have the same effect I add col-lg

<div>
  <div class="col-6 col-sm-4 col-lg-4">col-6 col-sm-4 </div>
  <div class="col-6 col-sm-8 col-lg-8">col-6 col-sm-8</div>
</div>

If this is the intention its fine for me. I just liked more before.

@mdo
Copy link
Member Author

@mdo mdo commented on db45a60 Aug 6, 2013

Choose a reason for hiding this comment

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

I think the ranges make more sense, but we'll tweak that as needed before the final release.

@plankguy
Copy link

Choose a reason for hiding this comment

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

@eratzlaff +1. Normal use case is to define columns for sm & up, or lg & up. Now i'm having to add another class to achieve the same thing.

@Flydiverny
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this was to fix #9128 I'd say I prefer the col-*-offset-0 version instead. Either that or only scope the offset classes, though just scoping one of them seems inconsistent.

Although sure now you are "forced" to have a 'proper' column definition. But it feels like to much to always have to add all column classes.

@eratzlaff
Copy link
Contributor

Choose a reason for hiding this comment

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

@Flydiverny I'm agree. I was using the change on my page and y prefer how it was before. And I think if you add offset for sm I'm pretty sure you need some offset for lg too.

@beebauman
Copy link

Choose a reason for hiding this comment

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

Documentation needs to be updated to reflect this change. Submitted docs issue as #9302.

@eratzlaff
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer to find a way to update css and not update documentation. Make push pull and offset only to work on screen size range.

Please sign in to comment.