PhoenixDev22• 16,950
@PhoenixDev22
Posted
Hello Akram Adjab,
Congratulation on completing this challenge. Your solution looks great. I have some suggestions regarding your solution if you don’t mind:
- You should use
<main>
to wrap the card and<footer>
for the attribution. HTML5 landmark elements are used to improve navigation experience on your site for users of assistive technology.
- In my opinion, the alternate text should indicate where the Qr code navigate the user : like
QR code to frontend mentor
.
- Page should contain
<h1>
. In this challenge, you may use<h1>
visually hidden with class=”sr-only”. You can find it here
- 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.
width: 30rem
an explicit width is not a good way to have a responsive layout. Consider usingmax-width
to the card instead.
- There are so many arguments against setting the root
font-size: 62%
it state that you should never change the root font size because it harms accessibility.
hopefully this feedback helps.
Marked as helpful
0
Akram Adjab• 380
@akramAdjab
Posted
@PhoenixDev22 Thank you so much for your feedback, I'll be more careful next time about these things!
0
PhoenixDev22• 16,950
@PhoenixDev22
Posted
@akramAdjab
You're welcome. Happy coding!
0