Responsive Mobile First Result Summary Component

Community feedback

• 

  Hassia Issah•50,650

  @HassiaiPosted over 1 year ago

  Always avoid giving the body a height value, width, min-width or max-width value.

  Always use em/rem and other responsive unit for the width, height, max-width, max-height, min-width, padding, margin and other values.

  Scince you centered the main in the desktop design , there is no need to center it in the media query.

  Well done for completing this challenge you, did an awesome Job. Congrats!!!

  Marked as helpful

  2

  ---

  

  Jatin Poriya•180

  @InterestingPlusPosted over 1 year ago

  @Hassiai

  I had started learning with pixels.

  Now, I will turn to em / rem ✅

  Thanks to Guide me 😃

  0

  

  Jatin Poriya•180

  @InterestingPlusPosted over 1 year ago

  @Hassiai

  I have commented in the code in the GitHub repository why I gave height: 125vh to the body

  What's your opinion about this problem.

  0

  

  Hassia Issah•50,650

  @HassiaiPosted over 1 year ago

  @InterestingPlus , it is no needed, i saw it

  Marked as helpful

  1
• 

  Luka Glonti•3,440

  @lack21Posted over 1 year ago

  Great job 👍, but I have some recommendation!

  • Remove height: 125vh from the body, since you already have min-height: 100vh there's no need for first one.

  Marked as helpful

  1

  ---

  

  Jatin Poriya•180

  @InterestingPlusPosted over 1 year ago

  @lack21

  yes, but by giving height: 100vh the preview in solution is getting translated upwards. In this problem, I saw that the height of the body is not reaching 100% till the preview page, in other word not perfect match! so I have given height: 125vh.

  What about your opinion.?, Give me other advice 😃.

  0

  

  Luka Glonti•3,440

  @lack21Posted over 1 year ago

  @InterestingPlus If you want body to take full height adjust min-height: 100vh and add

  {
    display: grid;
    place-content: center;
  }
  

  Instead of this

  {
    display: flex;
    flex-direction: column;
    align-items: center;
    justify-content: center;
  }
  

  This should center the layout, if there's any problem let me know.

  Marked as helpful

  1

  

  Jatin Poriya•180

  @InterestingPlusPosted over 1 year ago

  @lack21 uhh, display: grid also not working as well as height: 125vh. what you say? how can we solve this problem in other way?

  0

  

  Luka Glonti•3,440

  @lack21Posted over 1 year ago

  @InterestingPlus Just remove height: 125vh from the body and add min-height: 100vh.

  If you want to make your code shorter, You can remove these from you're body

  {
    display: flex;
    flex-direction: column;
    align-items: center;
    justify-content: center;
  }
  

  And add these instead

  {
    display: grid;
    place-content: center;
  }
  

  This should fix the layout!

  Marked as helpful

  1