Skip to content
Submitted about 1 year ago

QR code component challenge result using CSS Flexbox

Adri Vasarhelyi•90
@adrivasarhelyi
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 on Frontend Mentor, I am very very newbie so I'd appreciate any kind of feedback. Thank you!

Community feedback

  • Muhammad Desai•640
    @momorocks111Posted about 1 year ago

    It doesn't apply here, but in larger projects, in the css file I personally place a :root property so I can declare variables for each color and font size. It's faster for doing the css. Hope this helps :)

    2
    Adri Vasarhelyi•90
    @adrivasarhelyiPosted about 1 year ago

    @momorocks111 Every tips are helpful so thanks I'll try when I'll start doing bigger projects:)

    1
  • Øystein Håberg•10,650
    @Islandstone89Posted about 1 year ago

    Congratulations on your first project :)

    My feedback:

    HTML:

    • As mentioned above, it needs a <main>. You can put it on the .container.

    • Alt text should also tell where scanning QR Code leads.

    • "Scan the QR code..." should be a paragraph.

    CSS:

    • Remove the width and height on the img. Add max-width: 100%, so it doesn't overflow the card.

    • Remove width on .container - it's a <div>, which means it's a block element, and they have 100% width as default.

    • Height should be min-height.

    • Remove width and height on .wrapper. You rarely want to set fixed dimensions! Consider adding a max-width in rem if card gets too big at larger screens.

    • You only need to put text-align: center on the <body> - the heading and paragraph will inherit the value from its ancestor.

    1
    Adri Vasarhelyi•90
    @adrivasarhelyiPosted about 1 year ago

    @Islandstone89 Thank you very much for your detailed feedback! They were very helpful I corrected my code according to them. :)

    1
    Øystein Håberg•10,650
    @Islandstone89Posted about 1 year ago

    @adrivasarhelyi Glad to help! Don't forget to upload your files again to the repo :)

    0
    Adri Vasarhelyi•90
    @adrivasarhelyiPosted about 1 year ago

    @Islandstone89 I won't, thank you. :)

    1
  • Bryan Li•3,530
    @Zy8712Posted about 1 year ago

    Your website is pretty solid, the only thing I'd really add in is a <main> tag to wrap around all of your stuff. This is primarily for accessiblity purposes. Besides that your solution is very close to the original design. Nice work 👍

    1
    Adri Vasarhelyi•90
    @adrivasarhelyiPosted about 1 year ago

    @Zy8712 Thank you! I was thinking about doing what you're saying but next time I'll add it to my code, thanks!

    0

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