Design comparison
Solution retrospective
This is my first challenge, any feedback would be appreciated.
Community feedback
- @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 helpful3@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 - Wrap the entire main content of the page with the
- @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 around20rem
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 amax-width: 100%
. -
Properties like
text-align
andfont-family
, when the same for all/most elements, should be set on the body.
Marked as helpful2@aristide5010Posted 11 months ago@Islandstone89 Thanks for the in-depth review, I've got some fixing to do!
1 - "Improve your frontend skills" is a heading, not a paragraph. So make it a
- @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 ofdiv
. 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 helpful2@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 GitHubJoin 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