Responsive feature cards using grid

Community feedback

• 

  David Dutra•230

  @dvdgdPosted 7 months ago

  Hey, great work completing this challenge, your HTML looks semantic and you achieve the main concern of placing the grid.

  Just one thing that I am noticing is there you are using px unit for your font-sizes. Prefer to use rem for your site be readable in larger screens and also small too. You can learn more about the rem unit and how to use it from here. Also, I always use a site to convert px values to a close rem, I can not recommend using it too.

  PLUS: You can set the max-width for your cards to ~23rem, this will prevent it to grow as the width grows too.

  Again, your HTML is pretty good, keep it up with this awesome work.

  Marked as helpful

  0
• 

  Maximilian Dybvik•200

  @RegexRiddlerPosted 7 months ago

  Your HTML markup is semantic and readable :)

  Your CSS is fine but if you where a little more consistent on your spacing and indentation it would be more readable.

  Last tip I have for you is about your CSS grid. You have 6 columns and 4 rows.

  section {
      grid-template-columns: repeat(6, 1fr);
      grid-template-rows: repeat(4, 1fr);
  }
  

  But you only need 3 columns and 2 rows, then use align-self: center on the left and right card.

  section {
      grid-template: repeat(2, auto) / repeat(3, auto);
  }
  
  article:nth-of-type(1), article:nth-of-type(4) {
      grid-row: span 2;
      align-self: center;
  }
      
  article:nth-of-type(2) {
      grid-area: 1 / 2;
  }
      
  article:nth-of-type(3) {
      grid-area: 2 / 2;
   }
  

  Here is my submition if you would like to compare:

  And my GitHub Repo

  0