QR Code Component using HTML and CSS

Community feedback

• 

  P

  Øystein Håberg•10,810

  @Islandstone89Posted 4 months ago

  Hello, well done!

  My feedback:

  HTML:

  • Remove the <header> - it is used for content that repeats on every page, like a logo and navigation bar.

  • Remove the <footer> if it doesn't have any content.

  • The alt text must also say where it leads(the frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website."

  • "Improve your" is a heading, not a paragraph. Change it to a <h2>.

  CSS:

  • Including a CSS Reset at the top is good practice.

  • I like to add 1rem of padding on the body, to ensure the card doesn't touch the edges on small screens.

  • Remove the html selector, it doesn't need any styles applied.

  • box-sizing: border-box is usually applied to all elements using the universal selector:

  *,
  *::before,
  *::after {
    box-sizing: border-box;
  }
  

  • On the body, change height to min-height - this way, the content will not get cut off if it grows beneath the viewport. It is also common to use viewport height instead of %, so change it to min-height: 100svh;.

  • Remove box-sizing: border-box on the card - that style is already applied through the universal selector (or inherited from the body if declared there).

  • Remove the fixed width and height in px on the card. Except for sometimes on icons, you rarely want to set fixed dimensions(especially heights), as this easily creates issues with responsiveness.

  • Add a max-width of around 20rem on the card, to prevent it from getting too wide on larger screens.

  • font-size must never be in px. This is a big accessibility issue, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead.

  • Since all of the text should be centered, you only need to set text-align: center on the body, and remove it elsewhere. The children will inherit the value.

  • Paragraphs have a default value of font-weight: 400, so there is no need to declare it.

  • On the image, add display: block and remove max-height: 100%, it is not needed.

  • Instead of using complex selectors like .qr p.sm, it is recommended to simply use the class:.sm.

  Good luck :)

  0