QR code challenge using Flexbox

Community feedback

• 

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  Hi DSoluk, great work on this challenge! 😉

  Here are a few tips for improve your code:

  • remove height from img and main tags, it is not necessary because it will take the necessary space according to the size of the elements
  • use h1 for the title of card and not simple p element
  • remove max-width from body otherwise on larger screens it will not be centered
  • to make it look as close to the design as possible remove outline from main tag and set width: 20rem
  • remove height from html and body element and add min-height: 100vh to body because Flexbox aligns child items to the size of the parent container
  • instead of using px or % use relative units of measurement like rem -> read here

  Overall you did well 😁

  Hope this help and happy coding!

  Marked as helpful

  0

  ---

  

  DSoluk•120

  @dsolukPosted over 2 years ago

  @denielden Thank you for the great feedback..thanks to your efforts and well organized instructions, I was able to make the changes and was very happy with results. I definitely over complicated my solution but am better off with feedback received. Thanks again!!

  On to the next challenge!

  0
• 

  Abdelrhman Kamal•450

  @AbdelrhmanX7Posted over 2 years ago

  hi DSoluk first you did a great job but there is some error i found you did it 1- in main you can remove height: 80% and use height: fit-content 2- i prefer you to use a specific instead of use percentage width because the percentage width change when the web width change example if the web width: 1000px and you use width: 30% in your element in this point your element width will be 300px it will be okay but when the screen will increase or reduce your element width will increase or reduce the web width => 1500px now your element width will be 450px and this will not be a better thing you do 3- if you want to make your element in the middle of the page you can check this article it include all ways to make an element in the middle of the page (https://blog.hubspot.com/website/center-div-css)

  Marked as helpful

  0

  ---

  

  DSoluk•120

  @dsolukPosted over 2 years ago

  @AbdelrhmanX7 Thanks so much for the feedback...very helpful...the whole sizing thing is a bit confusing but expect with more experience it'll settle in...again, thanks for taking the time for the feedback!!!

  0
• 

  T

  Grace•31,270

  @grace-snowPosted over 2 years ago

  Hi

  I'm afraid this looks totally broken for me viewing on mobile. You've already had lots of blood advice above so I won't repeat it. Just make sure you refractor code before moving on to anything else

  I'll add a few more

  • you're missing a heading element in the html. There is not 2 paragraphs in the design. There is a heading and paragraph
  • img alt text is not code, it's a human readable description of the image. It needs to say something like "QR code for frontendMentor.io" " font sizes must never be in px. Use rem. This will allow text to scale no matter what settings a user sets on their browser or device
  • this challenge should not have a media query. You don't need to change font sizes

  I hope this helps

  1