Design comparison
Solution retrospective
This was by far the easiest one
Leave Your Thoughts
Community feedback
- @PhoenixDev22Posted over 2 years ago
Hi @Naleeka,
Congratulation on completing this frontend mentor challenge. Your solution looks great. I have some suggestions regarding your solution:
- You should use
<main>
landmark to wrap the card. HTML5 landmark elements are used to improve navigation experience on your site for users of assistive technology.
- In my opinion, the image is an important content. The alternate text should indicate where the Qr code navigate the user: like
QR code to frontend mentor
not describes the image.
- Adding
rel="noopener"
orrel="noreferrer"
totarget="_blank"
links. When you link to a page on another site using target=”_blank” attribute , you can expose your site to performance and security issues.
- Consider using rem and em units as they are flexible, specially for font size better to use rem. If your web content font sizes are set in absolute units, such as pixels, the user will not be able to re-size the text or control the font size based on their needs. Relative units “stretch” according to the screen size and/or user’s preferred font size, and work on a large range of devices.
Overall, Excellent work! Hopefully this feedback helps.
Marked as helpful1@NaleekaPosted over 2 years ago@PhoenixDev22 thanks for the reply I will make the changes ASAP ❤
1 - You should use
- @correlucasPosted over 2 years ago
👾Hello NALEEKA, congratulations for your new solution!
You've a perfect solution here, the only thing you can improve is the html structure that can be reduced, for example, you dont need a div to wrap the
main
you delete this div an manage all content only with the<main>
you already have to keep all the content inside, and nothing more. The ideal structure is thediv
and only the image, heading and paragraph.See a good structure for this challenge
<body> <main> <img> <h1></h1> <p></p> </main> </body>
Give a margin to the main like
margin: 24px
will avoid the container to touch the screen edges while scaling.The rest is just perfect!
👾My rating for this solution: ⭐⭐⭐⭐⭐
👋 I hope this helps you and happy coding!
Marked as helpful1
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