Overall this is really good. There's just a few suggestions for improvement and one critical issue you need to fix
- The image should ideally have slightly more meaningful alt text like "QR code to FrontendMentor.io"
- It's less performant to load the css reset as a separate file
- The order of the css is very strange in this. On larger projects you definitely won't want repeating selectors in different places. Usually it's reset, base styles, fonts, then component by component
- Avoid complex css selectors or wildcard selectors unless they are absolutely necessary. Eg instead of
.qr-container > *
you can place text center higher in the DOM and it would be inherited anyway. Complex css selectors increase specificity making the styles harder to manage, especially on large projects. And wildcard selectors are poor for browser performance so should be used sparingly - This is the big critical bug that is essential to change. NEVER change the font size on
:root
orhtml
, and never set font size in pixels anywhere. That one line of CSS means this fails minimum accessibility standards. Read more here
Marked as helpful
@jumiranda5
Posted
Thanks a lot, Grace! I thought I had to set the root font size... but now I see I was completly wrong. I'll do my best to apply all of this on the next challenge. (Actually, I'll fix this before moving to the next) @grace-snow
@jumiranda5
Posted
I've just finished refactoring the css and improving the qr code alt text as you suggested. Thanks again! @grace-snow