Submitted 9 months ago

QR Code Component

aristide5010• 110

@aristide5010

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

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

Community feedback

Melvin Aguilar 🧑🏻‍💻• 61,220

@MelvinAguilar

Posted 9 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

aristide5010• 110

@aristide5010

Posted 9 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.

Øystein Håberg• 9,450

@Islandstone89

Posted 9 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

aristide5010• 110

@aristide5010

Posted 9 months ago

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

Daniel 🛸• 44,230

@danielmrz-dev

Posted 9 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

aristide5010• 110

@aristide5010

Posted 9 months ago

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

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