Submitted over 1 year ago

QR code component

Kure-ru• 120

@Kure-ru

Preview site(opens in new window)View code(opens in new window)

This is a solution for...

QR code component

HTML
CSS

1newbie

View challenge

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,795

@vanzasetia

Posted over 1 year 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

Kure-ru• 120

@Kure-ru

Posted over 1 year 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 !

Vanza Setia• 27,795

@vanzasetia

Posted over 1 year ago

@Kure-ru

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

Have a nice day too! 🙌

Melvin Aguilar 🧑🏻‍💻• 61,240

@MelvinAguilar

Posted over 1 year 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

Kure-ru• 120

@Kure-ru

Posted over 1 year 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! 😊

Please log in to post a comment

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