2

I'm not sure why the images are taking up so much space and causing an overflow. If the imgs were replaced with a text it looks fine.

for (let i = 0; i < 15; i++)
{
  $("#grid").append(`
    <div class="item">
      <img src="http://via.placeholder.com/150x350" />
    </div>
  `);  
}
.flex {
  border: 1px solid black;
  display: flex;
  flex-direction: column;
  height: 100vh;
}
.footer {
  height: 20%;
}
.upper {
  flex: 1;
  overflow: auto;
}

#grid {
  border: 1px solid black;
  display: grid;
  grid-template-columns: 1fr 1fr 1fr 1fr 1fr;
  grid-template-rows: 1fr 1fr 1fr;
  height: 100%;
  position: relative;
  max-width: 100%;

}

.item {
  display: inline-block;
  text-align: center;
}

img {
  max-height: 100%;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js">
</script>
<div class="flex">
  <div class='upper'>
    <div id="grid">
    </div>  
  </div>
  <div class="footer">
    footer
  </div>
</div>
Asons
  • 84,923
  • 12
  • 110
  • 165
A. L
  • 11,695
  • 23
  • 85
  • 163

3 Answers3

2

You may use images as background, it will be easier to control and avoid overflow. Also don't forget the border in the height calculation so you may add box-sizing:border-box :

for (let i = 0; i < 15; i++) {
  $("#grid").append(`
    <div class="item" style="background-image:url(http://via.placeholder.com/150x350)">
     
    </div>
  `);
}
.flex {
  border: 1px solid black;
  display: flex;
  flex-direction: column;
  height: 100vh;
}

.footer {
  height: 20%;
}

.upper {
  flex: 1;
  overflow: auto;
}

#grid {
  border: 1px solid black;
  display: grid;
  grid-template-columns: 1fr 1fr 1fr 1fr 1fr;
  grid-template-rows: 1fr 1fr 1fr;
  height: 100%;
  position: relative;
  max-width: 100%;
  box-sizing:border-box;
}

.item {
  display: inline-block;
  text-align: center;
  background-size: auto 100%;
  background-position:center;
  background-repeat:no-repeat;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js">
</script>
<div class="flex">
  <div class='upper'>
    <div id="grid">
    </div>
  </div>
  <div class="footer">
    footer
  </div>
</div>
Temani Afif
  • 245,468
  • 26
  • 309
  • 415
  • I generally don't want the overflow, which is hwy I'd like the img's height to be based on the grid. – A. L Nov 12 '17 at 08:56
  • ah right, using the img as the background, how could I forget, I'll give a whirl. – A. L Nov 12 '17 at 09:12
1

The problem here is:

  • height: 100% on #grid combined with border: 1px and not using box-sizing: border-box

  • the classical inline white space on item and image

The fix:

  • add box-sizing: border-box on #grid (or use calc, height: calc(100% - 2px))

  • remove inline-block on item and add display: block on img

For the centering of the image, and as being a block, I also added margin: 0 auto and removed text-align: center as it has no effect anymore.

Stack snippet

for (let i = 0; i < 15; i++)
{
  $("#grid").append(`
    <div class="item">
      <img src="http://via.placeholder.com/150x350" />
    </div>
  `);  
}
.flex {
  border: 1px solid black;
  display: flex;
  flex-direction: column;
  height: 100vh;
}
.footer {
  height: 20%;
}
.upper {
  flex: 1;
  overflow: auto;
}

#grid {
  border: 1px solid black;
  display: grid;
  grid-template-columns: 1fr 1fr 1fr 1fr 1fr;
  grid-template-rows: 1fr 1fr 1fr;
  height: 100%;
  position: relative;
  max-width: 100%;
  box-sizing: border-box;                  /*  added  */
}

.item {
  /*display: inline-block;                     removed  */
  /*text-align: center;                        removed  */

  min-height: 0;                           /*  Firefox fix  */
}

img {
  display: block;                          /*  added  */
  margin: 0 auto;                          /*  added  */
  max-height: 100%;
  max-width: 100%;           /*  might want this too  */
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js">
</script>
<div class="flex">
  <div class='upper'>
    <div id="grid">
    </div>  
  </div>
  <div class="footer">
    footer
  </div>
</div>
Asons
  • 84,923
  • 12
  • 110
  • 165
  • I'm using Vue and I'm not sure why it's completely ignoring the `max-height` I set. I'll have to keep dabbling it a bit. – A. L Nov 12 '17 at 12:52
  • @A.Lau If you follow up the styles Vue use/set you might find it, still, the answer I gave show the reason why the scroll appears. If you have a link to a working sample using Vue, I'll have a look. – Asons Nov 12 '17 at 12:57
  • I'll try making a public repo of the part where it's having this weird issue – A. L Nov 12 '17 at 20:40
  • The thing is, I'm trying to make a card game, so I'd like the grids just to space them out evenly, while putting another div inside so it's as large as the image, while having some text on top (using `position: absolute`) – A. L Nov 12 '17 at 22:30
  • I've decided to open a new question since this one was a bit ambiguous and already has answers for it. https://stackoverflow.com/questions/47255747/css-nested-div-in-grid-does-not-respect-grid-sizes – A. L Nov 13 '17 at 01:02
  • @A.Lau As you said yourself in the first comment (before you edited it), my answer explains the real reason of the issue you have in the posted code, and still you choose to accept an answer that does not? ... I find that odd, don't you? – Asons Nov 13 '17 at 06:53
  • yeah I think it does help, so I'm going to change my approach, changed answer to yours. – A. L Nov 13 '17 at 09:05
  • @A.Lau Thanks .... I also made a comment at your new question, with a link, to show what it takes – Asons Nov 13 '17 at 09:10
  • I just realised this solution doesn't work on firefox – A. L Nov 14 '17 at 00:36
  • @A.Lau Will have a look later today – Asons Nov 14 '17 at 06:49
  • Dw, I asked another question and found that you need to add `min-height: 0` – A. L Nov 14 '17 at 09:43
  • @A.Lau That is correct, to make Firefox behave...just updated my answer with that. `min-height` defaults to `auto`, and prevents items to be smaller than its content, and in some case it need to be set to `0` allowing them to shrink and fit its parent – Asons Nov 14 '17 at 09:53
0

Here's one way to shrink grid items which contain elements with intrinsic dimensions such as images - while retaining aspect ratio

img { object-fit: contain; }

for (let i = 0; i < 15; i++)
{
  $("#grid").append(`
    <div class="item">
      <img src="http://via.placeholder.com/150x350" />
    </div>
  `);  
}
.flex {
  border: 1px solid black;
  display: flex;
  flex-direction: column;
}
.footer {
  height: 20%;
}
.upper {
  flex: 1;
  overflow: auto;
}

#grid {
  border: 1px solid black;
  display: grid;
  grid-template-columns: 1fr 1fr 1fr 1fr 1fr;
  grid-template-rows: 1fr 1fr 1fr;
  height: 100%;
  position: relative;
  max-width: 100%;
}

.item {
  display: inline-block;
  text-align: center;
}

img {
  max-height: 100%;
  max-width: 100%;
  object-fit: contain;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js">
</script>
<div class="flex">
  <div class='upper'>
    <div id="grid">
    </div>  
  </div>
  <div class="footer">
    footer
  </div>
</div>
Danield
  • 121,619
  • 37
  • 226
  • 255
  • I don't think it's quite the issue. I've opened another question that properly corresponds to my problem since this one was poorly worded. https://stackoverflow.com/q/47255747/6554121 – A. L Nov 13 '17 at 01:03