qr-code component

Community feedback

• 

  P

  Øystein Håberg•10,390

  @Islandstone89Posted about 1 month ago

  HTML:

  • Every webpage needs a <main> that wraps all of the content, except for <header> and footer>. This is vital for accessibility, as it helps screen readers identify a page's "main" section. You don't need all those divs around the card, you only need a div that holds the card content, which should be placed inside a <main> landmark.

  • The alt text should be written naturally, without using - between the words. Write something short and descriptive, without including words like "image" or "photo". Screen readers start announcing images with "image", so an alt text of "image of qr code" would be read like this: "image, image of qr code". 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."

  • Text must not be in a <div> alone; you don't need to wrap the heading and paragraph in a div. The heading is a <h2>, and the paragraph is a <p>.

  • .attribution should be a <footer>, and you should use <p> for the text inside.

  CSS:

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

  • Move font-family to body, html doesn't need any styles. I would also move margin: 0 and padding: 0 to *. Remove width: 100vw and height:100vh.

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

  • Remove the margin on the card.

  • To center the card horizontally and vertically, with space between main and footer, I would use Flexbox on the body:

  display: flex;
  flex-direction: column;
  justify-content: center;
  align-items: center;
  min-height: 100svh;
  gap: 2rem;
  

  • Remove all widths and heights in px and %. Especially, setting a fixed px height for text elements is not recommended, as it can easily cause overflow if the text grows taller than the set size.

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

  • 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.

  • letter-spacing must also not be in px. You can use em, where 1em equals the element's font size.

  • 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, change height: 100% to height: auto, and add 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.

  Marked as helpful

  0