CSS Flexbox

Community feedback

• 

  P

  Jake Godsall•1,390

  @jakegodsallPosted 8 months ago

  Hi 👋

  Great work on this project!

  It really looks great at both mobile and desktop layouts 😁

  The main issue I see is with larger viewports. I think it would make sense to talk about the width and height of the cards separately.

  Width:

  Right now you are using a vw-based flex-basis to determine the width of the columns in your flex container. This is fine for standard viewports, but with wider viewports the cards grow indefinitely, leading to lots of empty space within the card. I would recommend adding a max-width with a fixed pixel value to each of the cards to stop this behaviour.

  .card {
      flex-basis: 20vw;
      max-width: 300px;
  }
  

  Height:

  Something similar is happening with the height becase of the vh-based value for height. Although the problem is the same, the solution is not. generally, it is best practice to not apply a fixed-value height, because it can lead to content overflow, especially when our containers have dynamic content. CSS is responsive by default and will determine the correct height for your component based on the size of the content, as well as padding, border etc, depending on the box-sizing property you use.

  I would recommend to leave the default height: auto on your cards and use a padding-bottom to add the desired whitespace below the content.

  Hope this helps! 😁

  Marked as helpful

  0

  ---

  

  Kenn-eth•40

  @Kenn-ethPosted 8 months ago

  @jakegodsall Thank for your feedback.

  0

  

  Kenn-eth•40

  @Kenn-ethPosted 8 months ago

  @jakegodsall I tried the solution but I got different result. I guess I followed the wrong approach from the start. I should have used grid instead of flexbox.

  That said, I will keep your feedback in mind while working on the next challenge. Thank you 😃😃

  0