Testimonials-Grid-Section

Community feedback

• 

  Lucas 👾•104,340

  @correlucasPosted over 2 years ago

  👾Hello Youcef Boudour, congratulations for your new solution!

  Here's some tips for you:

  The proper size for this container is max-width: 1115px insert this inside the container and youll see a little change in the grid layout.

  To align the container properly vertical centered all you need is to use min-height: 100vh in the body:

  body {
      min-height: 100vh;
      box-sizing: border-box;
      margin: 0;
      padding: 0;
      background-color: var(--Light-grayish-blue);
      font-family: var(--main-font);
      font-size: 13px;
      display: grid;
      place-items: center;
  }
  

  To improve your semantics, you can replace the <div> that wraps each card with <article> and the paragraph with the quote with the property tag <blockquote> this way you'll wrap each block of element with the best tag in this situation. Pay attention that <div> is only a block element without meaning.

  👋 I hope this helps you and happy coding!

  Marked as helpful

  1
• 

  Elaine•11,400

  @elaineleungPosted over 2 years ago

  Hi Youcef, you've done great firstly in putting this together, and it looks pretty good, aside from the responsiveness right now. The main issue is, the sections have a fixed width, which prevent them from resizing, and your breakpoint is also really small at 400px, which means when it's past the 400px mark, the layout switches, and the contents are fully covered by the browser until the browser is larger, which is not quite an optimal view for users.

  Here are some suggestions on what you can do:

  1. First, you actually don't have a main tag right now, and that's actually why you have some accessibility issues in your report, so I'd firstly add a <main> tag that wraps around your <section> as a kind of container. On this main tag, add a responsive width of width: min(100% - 4rem, 1,280px) and a margin-inline: auto to keep things centered. The min() means find the smaller of the values inside; 100% - 4rem means 100% of the width minus 2 rem of right and left padding. More on min() can be found in this article by web.dev.

  2. I see that you have display: grid on the body selector, and while it's horizontally aligned, it's not vertically centered. All you need to do is to add a min-height: 100vh, and it should be vertically centered. That also means your footer would be right underneath the grid as well. If you want the footer at the very end, you'll need to use display: flex instead with flex-direction: column, and then give main a display: flex, align-items: center, and flex: 1.

  3. Lastly, I'd remove all the widths and heights on section, and I'd change the breakpoint to 900px in the media query. If you don't want the cards to be so wide when it gets close to the larger sizes, you can have the width in mobile view at min(100% - 4rem, 600px) and then change it to min(100% - 4rem, 1,280px) in the media query. This should make the component resizable!

  Hopefully this helps you out a bit!

  Marked as helpful

  1