QR code component

Community feedback

• 

  P

  Øystein Håberg•10,710

  @Islandstone89Posted 3 months ago

  HTML:

  • I would change the heading to 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.

  • 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. Change the first <div> into a <main>, and remove role="main", as that is the implicit role of the <main> element. I would also change the class of the second <div> to card, meaning your structure should look like this:

  <main>
      <div class="card">
          <div class="image">
              <img>
          </div>
          <div class="text">
              <h2></h2>
              <p></p>
          </div>
      </div>
  </main>
  

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

  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 following from the card:

      position: absolute;
      top: 50%;
      left: 50%;
      transform: translateX(-50%) translateY(-50%);
  

  • A better way to center the card is to use Flexbox on the body:

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

  • Remove the width and height in px on the card. We rarely want to give a component a fixed size, as we want it to grow and shrink according to the screen size.

  • We do want to limit the width of the card, so it doesn't get too wide on larger screens. Give the card a max-width of around 20rem to solve this issue.

  • 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 never be in px. You can use em, where 1em equals the element's font size.

  • Set font-family on the body, and remove it elsewhere. The children of the body will inherit the font.

  • 400 is not a valid value for font-family, I assume you meant font-weight: 400. However, this is not needed, as that is the default font weight for paragraphs.

  • On the image, add width: 100%.

  • Move padding: 16px from sub.card to .card.

  Marked as helpful

  0
• 

  SemahCODE•20

  @SemahCODEPosted 3 months ago

  well done

  0