Design comparison
Community feedback
- @PhoenixDev22Posted over 2 years ago
Greeting @dabinderudhan ,
I have few suggestions regarding your solution:
-
No need to mention
image
in the alt text as It’s going to be obvious to either a person or a machine when something they're accessing is alt text. Read more how to write an alt text -
Use min-height: 100vh; instead of
height: 100vh
/using vh (viewport height) units allows the body to to grow taller if the content outgrows the visible page./ -
width: 350px;
an explicit width is not a good way . Remove the width from the main component and change it tomax width
instead. That will let it shrink a little when it needs to. -
You should use
em
andrem
units .Bothem
andrem
are flexible, Usingpx
won't allow the user to control the font size based on their needs.Never usepx
for font size , read more here -
In this challenge , you don't need media query
@media (max-width: 375px)
. Even though it is too late to start the mobile layout. Start it as soon as there is room for the layout.
Last , you can generate another report for your solution after you corrected the accessibility issues.
Overall , your solution is good. Hopefully this feedback helps.
0 -
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