Rsponsive QR Code

Community feedback

• 

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 12 months ago

  Hey, good job on the challenge. Here are some things you can do to make it even better.

  HTML:

  • You need a <main>, this is important for accessibility. I would change the structure of the HTML a little bit. You can remove the <section>, that element is not needed here. You also don't need any divs. Add <main class="card"> and have the image and the text inside of it - this is all you need.

  • The alt text also needs to say where it leads (frontendmentor.io).

  • "Improve your frontend skills" is not a paragraph, but a heading. Change it into a <h1>.

  CSS:

  • It's good practice to include a CSS Reset at the top.

  • The max-width on the card works nice, but it should be in rem. 320px equals to 20rem.

  • I would change the margin: 0 1em to margin: 1em, this will give all sides some sort of margin. This can be helpful on smaller screens, just to make sure the card doesn't touch the edges of the screen.

  • You don't need height: 100% on the image.

  • The image should have a max-width: 100% instead of a width. This prevents the image from overflowing its container. It's also common to set display: block - all this is included in the mentioned CSS Reset above.

  • Set text-align: center on the body, as all the text should be centered. Do the same with the font-family.

  All that said - well done :)

  Marked as helpful

  1

  ---

  

  haseeb6111•70

  @haseeb6111Posted 12 months ago

  @Islandstone89 Thanks for your helpful review. I'll continue to improve my skills so please look into my future solutions as well if you've got time.

  1