QR code component

Community feedback

• 

  Adriano•38,080

  @AdrianoEscarabotePosted about 2 years ago

  Hello!

  You did a good job on this challenge. your project has some html issues:

  1- Document should have one main landmark, you could have pasted all the main content inside a main tag to make the code more semantic [click here](https://dequeuniversity.com/rules/axe/4.3 /landmark-one- main?application=axeAPI)

  2- Page should contain a level-one heading click here to read about it

  3- I changed the display of the grid container to flex-box to have a better responsiveness and I changed the height:100vh; for min-height:100vh;

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

  4- I removed the width of the img-container div, because it was not necessary.

  5- I removed the width of the card-text div because it was causing the layout to break when it reached lower resolutions, preventing a good layout responsiveness.

  6- I put a media query to adapt the layout to smaller resolutions and changed the measurement unit to percentage

  @media (max-width:400px) { .container { padding: 1rem; } .card { width: 90%; } #qr-code { width: 100%; } }

  One thing I noticed was that you put width in the parent divs of the elements, for example in the img #qr-code and in the card-text, this is not necessary since every time we put sizes for the elements that are inside the div it makes the divs adapt to it, so it's not needed in this case.

  Make the changes I made to your code to see a better responsiveness of the layout

  But the rest is very good congratulations on the result. Congratulations 🥳

  Marked as helpful

  0
• 

  FedericoSpina1•60

  @FedericoSpina1Posted about 2 years ago

  You can use Flexbox or Grid instead of width in the .card, also you can create a box-shadow around the .card. Good job, though.

  0