Testimonial Grid

Community feedback

• 

  T

  Grace•31,230

  @grace-snowPosted over 3 years ago

  Hi

  You'll never want this on html and body:

  width: 100vw;
  height: 100vh;
  

  Viewport units don't account for scrollbars or things like phone notches, so this code would cause scrollbars to be added even when you don't need them. And you want height to be able to grow with the content.

  So: Leave HTML alone. No width on the body (it's 100% width anyway). Optionally, add a min-height 100vh to body.

  Also add some padding to the body or one of your wrapper elements so content can't hit the sides

  1

  ---

  

  T

  Grace•31,230

  @grace-snowPosted over 3 years ago

  Overall, on this I reccomend you change the HTML structure almost completely. You have loads of nested flex items, most of which aren't needed, and are trying to build out rows of content.

  Ideally, this is a CSS grid challenge. So all cards can be direct children of one element set to display grid. The grid-template properties dictate the size and position of all cards.

  I also recommend bumping up the heading levels. h4 is very low to start from, I can't imagine content like this would have 3 heading levels above it. I would also change verified graduate to be a paragraph, maybe with a small tag inside, and not use negative margin at all.

  In fact, those card headers are the only part of this design I would recommend using flexbox for.

  If you're not familiar with css grid yet, maybe have a go at some of the other beginner UI challenges first and come back to refactor this one later on

  I hope this helps you

  1

  

  sylvamadu•100

  @sylvamaduPosted over 3 years ago

  Wow, I'm happy for this wonderful feedback of yours. I actually didn't know that I was suppose to use display grid for those cards. I figured I was pretty much familiar with flex and not grid. But with this feedback, I will look into learning grid and absolutely work on this challenge again, thanks Grace.

  0