PhoenixDev22• 16,950
@PhoenixDev22
Posted
Hello Xoel Veiga,
Congratulation on completing your first frontend mentor challenge, I have some suggestions regarding your solution:
- You should use <main> landmark to wrap the body main content as landmarks allow screen reader users to navigate through sections of your website by skipping to content that interests them. Landmarks could be seen as the logical layout of the website's UI, which is divided into e.g. header, navigation, main content, and footer.
- The alternate text should indicate where the Qr code navigate the user : like
QR code to frontend mentornot describes the image.
- Page should contain
<h1>. In this challenge, you may use<h1>visually hidden withclass=”sr-only”. You can find it here. You can remove<b>.
- In order to center the card on the middle of the page , you can use the flexbox properties or grid properties and
min-height: 100vhfor the<body>add a little padding to the body that way it stops the card from hitting the edges of the browser. No need for position absolute.
Once you apply position: absolute to one element, you’ll usually find yourself applying it to everything else on the page. Because position: absolute takes the element out of the flow of the document, everything else has to be adjusted to compensate for it, to make sure that elements don’t overlap where you don’t want them to.
width: 275px;an explicit width is not a good way to have responsive layout . Consider usingmax-widthto the card inrem.
- Consider using rem for font size , it' not recommended to use px for font size as absolute units don’t scale for example 15px will always be 15px on the same device. Using pixels is a particularly bad practice for font sizing because it can create some accessibility problems for users with vision impairments.
Overall, your solution looks great. Hopefully this feedback helps.
0
