PhoenixDev22• 16,950
@PhoenixDev22
Posted
Hi Samarth Goyal,
Congratulation on completing this frontend mentor challenge. Your solution looks great. I have some suggestions regarding your solution:
- In my opinion, the image is an important content. The alternate text is needed on this image. The alternate text should indicate where the Qr code navigate the user : like
QR code to frontend mentornot describes the image. For future use, the alternate text should not be hyphenated, it should be human readable.
- Page should contain
<h1>. In this challenge , as it’s supposed to be a part of a whole page, you may use<h1>instead of<h4>.
- Adding
rel="noopener"orrel="noreferrer"totarget="_blank"links. When you link to a page on another site usingtarget=”_blank”attribute , you can expose your site to performance and security issues.
- In order to center the card on the middle of the page , you can use the flex or grid properties and
min-height: 100vhto the<body>. Add a little padding to the body that way it stops the component from hitting the edges of the browser.
width: 350pxan explicit width is not a good way to have responsive layout . Consider usingmax-widthto the card inrem.
Overall, Excellent work! Hopefully this feedback helps.
0
