QR-Code-Component

Community feedback

• 

  Melvin Aguilar 🧑🏻‍💻•61,140

  @MelvinAguilarPosted almost 2 years ago

  Hello there 👋. Good job on completing the challenge !

  I have some suggestions about your code that might interest you.

  HTML 📄:

  • Use the <main> tag to wrap all the main content of the page instead of the <div> tag. With this semantic element you can improve the accessibility of your page.

  • Use the <footer> tag to wrap the footer of the page instead of the <div class="attribution">. The <footer> element contains information about the author of the page, the copyright, and other legal information.

  • The <br> tag is not a semantic element. Use margin-bottom to add space between elements.

  Alt text 📷:

  • The alt attribute should explain the purpose of the image. Uppon scanning the QR code, the user will be redirected to the frontendmentor.io website, so a better alt attribute would be QR code to frontendmentor.io

    If you want to learn more about the alt attribute, you can read this article. 📘.

  CSS 🎨:

  • Instead of using pixels in font-size, use relative units like em or rem. The font-size in absolute units like pixels does not scale with the user's browser settings. This can cause accessibility issues for users who have set their browser to use a larger font size. You can read more about this here 📘.

  • To center the component in the page, you should use Flexbox or Grid layout. You can read more about centering in CSS here 📘.

  I hope you find it useful! 😄

  Happy coding!

  Marked as helpful

  1
• 

  Hassia Issah•50,650

  @HassiaiPosted almost 2 years ago

  Replace<div class="card">with the main tag, <h2> with <h1> and <div class="attribution"> with the footer tag to fix the accessibility issues. click here for more on web-accessibility and semantic html

  Use the colors that were given in the styleguide.md found in the zip folder you downloaded.

  There is no need to give the body a padding value. Add a font-size of 15px and text-align center to the body, these will apply to both h1 and p, you don't have to give h1 and p any font-size after this.

  To center .card on the page, add min-height:100vh; display: flex; align-items: center: justify-content: center; or min-height:100vh; display: grid place-items: center to the body.

  To center .card on the page using flexbox:
  body{
  min-height: 100vh;
  display: flex;
  align-items: center;
  justify-content: center;
  }
  

  To center .card on the page using grid:
  body{
  min-height: 100vh;
  display: grid;
  place-items: center;
  }
  

  Give the img a max-width of 100% instead of a fixed width value. Give h1 and p the same margin-left, margin-right and margin-top values. Give p a margin-bottom value.

  Use relative units like rem or em as unit for the padding, margin, width values and preferably rem for the font-size values, instead of using px which is an absolute unit. For more on CSS units Click here

  Hope am helpful.

  Well done for completing this challenge. HAPPY CODING

  Marked as helpful

  0
• 

  Tee•190

  @thulanigamteePosted almost 2 years ago

  Congratulations on completing your first challenge.

  The text on the design comparison is centered rather than aligned to the left, try text-align: center, and it should fix that. When writing your HTML try to use landmarks, rather than using divs all the time.

  And I think it is better to have your css on a separate file from your html.

  0