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.

Sign up for GitHub

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

Jump to bottom

overflow of .twelve.columns div in fixed width tablet portrait view #28

Closed
dirkolbrich opened this issue Apr 28, 2017 · 6 comments
Closed

overflow of .twelve.columns div in fixed width tablet portrait view #28

dirkolbrich opened this issue Apr 28, 2017 · 6 comments

Comments

@dirkolbrich
Copy link
Contributor

dirkolbrich commented Apr 28, 2017 •
edited
Loading

A .twelve .column.wide <div> is too wide in iPad portrait screen. It overflows the surrounding .container.

Following html setup. Theme is Freshwith $is-fluid: false;setup:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <title>sample title</title>
    <link rel="stylesheet" href="/css/main.css">
</head>

<body>
    <!-- .container is main centered wrapper -->
    <div class="container">
        <div class="row">
            <div class="twelve columns">
                <p>This column is too wide for surrounding container in browser width 960 - 1100px</p>
            </div>
        </div>
    </div>
</body>

</html>

Problem

Right in between the sweet spot of 960 - 1100px browser width (iPad is 1024px on portrait), the .twelve.columsdiv is wider than the surrounding .container div. I think the reason is .twelve.columshas no max-widthset at that point.

This is caused by the @media only screen and (min-width: $tablet-width) and (max-width: $base-width - 1) {} query in /themes/fresh/_grid.scss:34 as it stops at 959px browser width.

/* TABLET (PORTRAIT) */
@media only screen and (min-width: 768px) and (max-width: 959px) {
  .container {
    width: 768px;
  }
  /* ... */
  .container .twelve.columns {
    width: 748px;
  }
}

From there on, the other smaller media queries kick in:

/* For devices larger than 400px */
@media (min-width: 400px) {
  .container {
    width: 85%; /* results to 816px width at 960px browser, needs browser at 1100px width until it is wide enough for .twelve.columns */
  }
}

/* For devices larger than 550px */
@media (min-width: 550px) {
  .container { /* no width set, is by 85% */ }
  .container .twelve.columns {
    width: 940px;
  }
}

Proposed solution

Give all columns a max-width: 100% in /core/_dependencies.scss:96:

.#{numToString($i)}.columns { 
  width: $colWidth + (($colWidth + $gutterWidth) * ($i - 1));
  max-width: 100%;
}

Yet I don`t know for sure, if this would cause any side effects on other settings.
@dirkolbrich dirkolbrich changed the title overflow of .twelve.columns div in fixed width tablet portrait view, /themes/fresh/_grid.scss:34 overflow of .twelve.columns div in fixed width tablet portrait view, /core/_dependencies.scss:96 Apr 28, 2017
@dirkolbrich
Copy link
Contributor Author

better solution to set max-widthfor all .columns at /core/_dependencies.scss:83:

@mixin _fixedGrid($width, $colWidth, $gutterWidth, $colCount) {
	.container {
		.column,
		.columns {
			max-width: 100%;
			margin: {
				left: $gutterWidth / 2;
				right: $gutterWidth / 2;
			};
		}
	}
	/* The Grid */
       /* ... */
}

@atomicpages
Copy link
Owner

@dirkolbrich can you help me out with an exact repro? Here are some good sites for sharing code:

@dirkolbrich
Copy link
Contributor Author

Ok, here is the CodePen testing skeleton-sass fixed grid.

Setup is right from the skeleton-scss README, no additional styles, except the gray-boxes.
max-width 960px, 12 columns

There are several issues with the base fixed-grid:

  1. at screen width 320px
  2. at screen width 400px
    • .container div has no padding, below width 400px a padding: 20px;, this gives a little jump at changing screen size
  3. screen width between 960px und 1129px

@dirkolbrich
Copy link
Contributor Author

added basic layouts and offset to the CodePen.

Following issue in addition to the above:
4. .one-half columns have no css class assigned

@atomicpages atomicpages changed the title overflow of .twelve.columns div in fixed width tablet portrait view, /core/_dependencies.scss:96 overflow of .twelve.columns div in fixed width tablet portrait view May 7, 2017
atomicpages pushed a commit that referenced this issue May 7, 2017
3.1.4
f6b3396 
* Addressing issue #31
    * Adding styles for `.one-half`
    * Adding new selectors to fixed grid:
        * `.one-third.columns`
        * `.two-third.columns`
        * `.one-half.column`
        * `.one-half.columns`
        * `.full-width.column`
* Addressing issue #28
* Fixing configuration issues when `$base-gutter-width` is something other than `20px` in fixed grid mixin
* Enforcing `body` margin styles to fixed overflow issue in `960px - 1129px`
    * Applies only to `fresh` theme
@atomicpages
Copy link
Owner

@dirkolbrich thanks for all your hard work identifying the issues! I alongside our small Skeleton Sass community greatly appreciates it! I believe all issues you identified are resolved in release 3.1.4

@dirkolbrich
Copy link
Contributor Author

Thanks. I am just finger pointing. You did the work.
Thank you for this repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@atomicpages @dirkolbrich