Skip to content
Submitted 11 months ago

QR Code Component

aristide5010•110
@aristide5010
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

This is my first challenge, any feedback would be appreciated.

Community feedback

  • Melvin Aguilar 🧑🏻‍💻•61,140
    @MelvinAguilarPosted 11 months ago

    Hello 👋. Congratulation on successfully completing your first challenge 🎉 ! !

    I have some recommendations regarding your code that I believe will be of great interest to you.

    • Wrap the entire main content of the page with the <main> tag.
    • Provide an alt attribute for the QR code image, explaining its purpose, e.g., QR code to frontendmentor.io.
    • Utilize box-sizing: border-box for more accurate sizing calculations. Details here 📘.
    • Avoid using position: absolute for centering as it may result in overflow on some screen sizes. Consider using flexbox or grid layouts. Learn more about centering in CSS here 📘.

    I hope you find it useful! 😄 Above all, the solution you submitted is great!

    Happy coding!

    Marked as helpful
    3
    aristide5010•110
    @aristide5010Posted 11 months ago

    @MelvinAguilar Thank you for checking my code and taking the time to comment! I will try to apply your recommendations on my next challenge.

    1
  • Øystein Håberg•10,710
    @Islandstone89Posted 11 months ago

    Hello, to add to the great advice from the fine gentlemen above:

    HTML:

    • "Improve your frontend skills" is a heading, not a paragraph. So make it a <h1>.

    CSS:

    • Performance-wise, it's better to link fonts in the <head> of the HTML then using @import.

    • It's good practice to include a CSS Reset at the top.

    • Font-size must never be in px. Use rem instead.

    • Remove all fixed widths and heights. Fixed dimensions easily cause issues with overflow.

    • Add a max-width of around 20rem to the card, to prevent it from getting too wide on larger screens.

    • The card should have padding on all sides, to create the space between its edges and the image.

    • Remove margin on the image, and also add a max-width: 100%.

    • Properties like text-align and font-family, when the same for all/most elements, should be set on the body.

    Marked as helpful
    2
    aristide5010•110
    @aristide5010Posted 11 months ago

    @Islandstone89 Thanks for the in-depth review, I've got some fixing to do!

    1
  • Daniel 🛸•44,310
    @danielmrz-devPosted 11 months ago

    Hello @aristide5010!

    Your project looks excelent!

    I have two minor suggestions for you to improve it:

    • Wrap the main content with the main tag instead of div. This won't change anything visually, but it makes your HTML more semantic.

    • If you add a box-shadow to the card, it'll look even closer to the original design.

    Other than those little details, you did a great job!

    Marked as helpful
    2
    aristide5010•110
    @aristide5010Posted 11 months ago

    @danielmrz-dev Thank you for your suggestions and encouragement, I appreciate it!

    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