Mobile First Responsive Testimonials-Grid-Section | Build wit Css-Grid

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Hello, Ernest! 👋

  Great work on this challenge! Your solution looks pretty good! 😀 Also, good job with the alternative text for all images! 👏

  I would not recommend setting any font-size on the html element. It causes conflict with the user font size settings and causes issues when working with CSS framework. It's because all elements depend on the html font size, especially if you use rem as your main font size unit. So, simply set the font-size on the body element instead.

  Prefer unitless numbers for line-height values to avoid unexpected results. The MDN documentation explains it well.👍

  That's it! Hope this helps. 😊

  Marked as helpful

  0

  ---

  

  Ernest Push Gbaboh•460

  @gbabohernestPosted over 2 years ago

  @vanzasetia thanks for the feedback Always helpful. 😊✌

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @gbabohernest You're welcome! Glad it's helpful for you! 😄

  0
• 

  Zorah•20

  @ZorahScopePosted over 2 years ago

  This looks sweet. Can't be too in-depth on my feedback since I haven't learned CSS grid yet.

  But I'd say the only thing that stands out to me would be that the body or html element doesn't fill out the full viewport. Which explains the noticeable difference in the overall height when using the slider to compare the design vs your solution.

  body {
      min-height: 100vh;
      min-width: 100vw;
      /* other declarations stay the same */
  }
  
  main {
      margin: 10rem auto;
      /* other declarations stay the same */
  

  Something like this, but would center using flexbox or grid vs explicitly defining margin.

  Also went from H1 then skipped to H4, there's no H2 or H3. I'm guessing you opted for H4 for the smaller font size, but this can be don't by adjusting the font-size directly. Heading elements should follow as intended from "Most important > least important"

  0

  ---

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @elixy By default, the body element has already 100% width so, there's no need to set any min-width or even max-width. 🙂

  It's best to treat body element like the canvas paper, you can't limit or add more width to it. However, you can limit the element inside it whatever you like. 😉

  0

  

  Ernest Push Gbaboh•460

  @gbabohernestPosted over 2 years ago

  @elixy thanks for the feedback. As for the heading... yeah, that's what I did.. but I will look at it.. thanks again 😊 👍

  0