QR code using flexbox and CSS custom properties

Community feedback

• 

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 20 days ago

  Hello!

  I've inspected your code, and these are my suggestions. I hope they are clear and helpful to you!

  HTML:

  • <main> holds all of the main content on a page. As a card would likely not be the only component on a page, I would wrap the card content in a <div class="card"> inside of <main>.

  • I would remove the <section> wrapping the image, it's not needed.

  • 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. Headings must always be in order. I would make it a <h2> - a page should only have one <h1>, reserved for the main heading. As this is a card heading, it would likely not be the main heading on a page with several components.

  • I would replace the id with a class - here is an article that explains when we should use an id.

  • Wrap the footer text in a <p>, as text should never be in a <div> alone.

  CSS:

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

  • You must include the font in your HTML, or reference it via @font-face in the CSS.

  • Remember to specify a fallback font:font-family: 'Outfit',sans-serif;

  • I recommend adding a bit of padding, for example 16px, on the body, to ensure the card doesn't touch the edges on small screens.

  • Remove the margin on the card.

  • On body ,add the following:

  justify-content: center;
  min-height: 100svh;
  gap: 2rem;
  

  • max-width on the card should be in rem. Around 20rem will work fine.

  • Remove the max-height on the card - you should never limit the height of an element containing text! This will cause the text to overflow if it grows taller than the fixed size of the card.

  • 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, remove width and height. Instead, add height: auto, display: block and max-width: 100% - the max-width prevents it from overflowing its container. Without this, an image would overflow if its intrinsic size is wider than the container. max-width: 100% makes the image shrink to fit inside its container.

  • If you choose to include the image wrapper, remove its padding.

  • To create the space between the image and the edge of the card, set padding on all 4 sides of the card:padding: 16px;.

  Marked as helpful

  1

  ---

  

  Violeta•50

  @v-t-9Posted 20 days ago

  @Islandstone89 Thanks for the suggestions!

  1