QR Code Component made using only HTML

Community feedback

• 

  Evandee Silva•40

  @EvandeeMoSPosted 3 months ago

  First of all, your solution was perfect, congrats!

  My only tips for you are:

  • Put your css into a different file
  • Write a good Readme

  Why put css into a different file? Because it will be easier to maintain, this project is not complex enough to be a problem have the css and html into a single file, but write it into another file is a good pratice.

  Why write a good Readme? Sometimes people will se only your github, and is important that there they have a good explanation or a preview of what the project is.

  That is it! Continue the good work.

  Marked as helpful

  1
• 

  Sabine•40

  @SabineEmdenPosted 3 months ago

  Hello there!👋 Good job on completing the challenge.

  Here are some suggestions how you could improve your code:

  HTLM

  • Add a <main> element to your code. This is very important for web accessibility. It allows users who use assistive technology to access the main content of the page faster. You don’t need an extra HTML element for this, simply replace <div class=“card-container”> with <main>.
  • Remove <div class=“card-detail>. It isn’t needed here.
  • Replace <p class=“header-text”> with an <h2> or <h1> element. As this project is a component that will probably be used on a page that already has an <h1> heading, I would use an <h2>.
  • Put the attribution back into your code. Give credit where credit is due. And wrap the attribution in a <footer> element. It does not belong inside <main>, but needs its own semantic HTML element.
  • Count your </div> closing tags and remove any extras.

  CSS

  • Move the CSS code to a separate style sheet.
  • Add a modern CSS reset to the top of your style sheet. I like the one by Josh Comeau. Andy Bell has another good one.
  • Add some padding to body.
  • Convert all values for font-size from px to rem. This is very important for web accessibility. It allows users who have selected a larger font size in the setting for their browser or their operating system to view your page with a larger font size.
  • Convert max-width for .card-container to rem. This is needed for the design to work with larger fonts sizes.
  • Remove hight from img. If you use a CSS reset, it’ll include max-width:100% for the image. That’s all you need.
  • Replace the padding you had on .card-detail with margins on .header-text and .detail-text.

  I hope this helps. Let me know if you have any questions.

  Happy coding! 😎

  0