Skip to content
Submitted almost 2 years ago

QR code component

Kure-ru•120
@Kure-ru
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

I would really appreciate any advice and feedback regarding my code.

I hard a hard time with sizing. Do you have any tips?

Thank you in advance 😊

Community feedback

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

    Hi, Kure-ru!

    It's okay if you don't get the size exactly like the original design. As long as it looks similar to the original design then that's already good. 🙂

    I recommend focusing on code quality instead. You can improve your solution by:

    • Adding alternative text to the QR code. It is the main content of the image. So, it should have alternative text to be visible by screen readers.
    • Removing </div>. There is no <div> so there should not be </div>.
    • Using <body> as the page element instead of the <html> element. It's better to not change styling on the <html> element since it can overwrite the user's settings.
    • Using rem or em instead of px for font sizes. Relative units such as rem and em can adapt when the users change the browser's font size setting.

    I hope you find this useful.

    Marked as helpful
    0
    Kure-ru•120
    @Kure-ruPosted almost 2 years ago

    @vanzasetia Hi Vanza!

    Thank you so much for taking the time to look at my code! Your remarks are very helpful. 😁

    Have a great day !

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

    @Kure-ru

    You are welcome! Happy to hear that was helpful! 😊

    Have a nice day too! 🙌

    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.

    HTML 📄:

    • Since this component involves scanning the QR code, the image is not a decoration, so it must have an alt attribute. The alt attribute should explain its purpose. e.g. QR code to frontendmentor.io

    CSS 🎨:

    • You should use the box-sizing: border-box property to make the width and height properties include the padding and border of the element. This will make it easier to calculate the size of an element. You can read more about this here 📘.

    • Centering an element with position: absolute or fixed would make your element behave strangely on some screen sizes, "there's a chance the content will grow to overflow the parent". You can use Flexbox or Grid to center your element. You can read more about centering in CSS here 📘.

      main {
    display: flex;
    align-items: center;
    justify-content: center;
    min-height: 100vh;
}

.centered {
    /* position: fixed; */
    /* top: 25%; */
    /* left: 50%; */
    /* margin-top: -50px; */
    /* margin-left: -140px; */
    /* right: 50%; */
}

article {
    max-width: 250px; /* Use max-width instead of min-width */
    /* min-width: 250px; */
}
    • You should use a CSS reset to remove the default browser styles and make your page look the same in all browsers.

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

    Happy coding!

    Marked as helpful
    0
    Kure-ru•120
    @Kure-ruPosted almost 2 years ago

    @MelvinAguilar Hi Melvin, thank you so much for your feedback! I didn't know about CSS reset, it seems very useful. I am going to correct my code with your suggestions. Thank you again! 😊

    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