testimonials-grid-section-main

Community feedback

• 

  Adriano•42,800

  @AdrianoEscarabotePosted 4 months ago

  Hi Salva, hope you're doing well! I loved how your project turned out, but I’ve got a few suggestions that could be useful:

  Using Flexbox or Grid on the body to center elements ensures a more responsive and adaptive layout, fitting different screen sizes seamlessly. It avoids manual calculations and constant adjustments needed with margin, padding, or absolute positioning. These techniques provide more consistent alignment and simplify the code.

  flexbox:

  body {
      display: flex;
      justify-content: center;
      align-items:  center;
      min-height: 100vh;
  }
  

  grid:

  body {
      display: grid;
      place-content: center;
      min-height: 100vh;
  }
  

  The rest is fantastic.

  Hopefully, you'll find it helpful. 👍

  0
• 

  Salva•280

  @salva-itPosted 4 months ago

  Hi KapteynUniverse, thank you for reviewing my project. I have made the changes and added another 1fr column.

  i know quote image is decorative I didn’t want to leave the alt attribute empty because of SEO.

  0
• 

  KapteynUniverse•2,900

  @KapteynUniversePosted 4 months ago

  Hey Salva, nice job,

  You should add another 1fr to the column grid-template-columns: 1fr 1fr 1fr; to the container for desktop layout. With 3 column, it looks like grid making the fourth one automaticly because of the positioning of "Kira Whittle" but since that column created automaticly i think its width setting based on its content and that is pushing other articles and making them shrink. Also changing max-width of the container might be better. You can remove all the max-width from article after this too.

  @media screen and (min-width: 1024px) {
      .container {
          display: grid;
          grid-template-columns: 1fr 1fr 1fr 1fr;
          grid-template-rows: 1fr;
          max-width: 1024px;
      }
  }
  

  Also quote image is decorative, you can leave alt text empty.

  0