Skip to content
Submitted almost 2 years ago

QR Code Component using Grid

Deepali Panchal•310
@Deepali25-K
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...QR code component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

Would you please have a look at my code and offer any suggestions for improvements?

Community feedback

  • Vanza Setia•27,775
    @vanzasetiaPosted almost 2 years ago

    Hi, Deepali Panchal!

    There are a few changes you can do to make this solution better.

    • Use <img> element instead of <section class="image"></section> for the QR code.
    • Remove all <section> elements. You don't need to wrap the text with a <section>.
    • Add rel="noopener" to all links with target="_blank". It helps protect users of legacy browsers from security issues. Read more — Links to cross-origin destinations are unsafe
    • Don't change the <html> or the :root font size to 4px. It can cause huge accessibility implications for those users with different font sizes or zoom requirements.
    • Never use px unit for font sizes. Use rem or em instead. Relative units such as rem and em can adapt when the users change the browser's font size setting.
    • Remove height: 124rem; from the <main> styling. Let the content controls the height of it. Also, it is rare to specify a specific height on an element.
    • Put the QR code image on the GitHub repository instead of hosting it somewhere else. Self-host your asset. It will make your site loads faster.

    I hope this helps. Happy coding!

    Marked as helpful
    1
    Deepali Panchal•310
    @Deepali25-KPosted almost 2 years ago

    @vanzasetia Thank you so much for your feedback. I really appreciate the information you shared with me about areas where I can improve; it will be useful when I approach my next project.

    0
    Vanza Setia•27,775
    @vanzasetiaPosted almost 2 years ago

    @Deepali25-K

    No problem!

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

    • Since this component involves scanning the QR code, the image is not a decoration. You must not use the background-image property to add the QR code image. Instead, use the <img> tag to add the image. Use the background-image property only for decorative images that do not add any information to the page.
    • html { font-size: 4px; }: The font size is too small. 4px is not a recommended font size for webpages. There is no benefit to setting a font size so small, it ends up using a lot of units, for example: width: 81rem.

    I hope you find it useful! 😄

    Happy coding!

    Marked as helpful
    1
    Deepali Panchal•310
    @Deepali25-KPosted almost 2 years ago

    @MelvinAguilar

    I appreciate you taking the time to provide me with detailed and constructive feedback. :)

    1

Please log in to post a comment

Log in with GitHub

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord