Design comparison
Solution retrospective
Comments and feedback are welcome
Community feedback
- @PhoenixDev22Posted over 2 years ago
Hello cyrilkodjo ,
Congratulation on completing this challenge,
I have some suggestions regarding your solution:
HTML
-
For any decorative images, each img tag should have empty
alt="" as you did
andaria-hidden="true"
attributes to make all web assistive technologies such as screen reader ignore those images . In this challenge , all the images are decorative. -
don't capitalise in html, let css text transform take care of that. Remember screen readers won't be able to Read capitalised text as they will often read them letter by letter thinking they are acronyms.
CSS
-
To make the card perfectly in the middle of the page, you can make the body element as a flexbox container and give it
min-height: 100vh
and only a little padding to stop the content from hitting the edges. (then you can remove the margin that was given to the .container) -
Using an explicit width
width: 1000px;
is not really a good way , consider usingmax-width
in rem instead, That will let the component grow up to a point and be limited. Then a little margin on the component or padding on the body to stop it hitting screen edges. -
height: 480px;
It's rarely ever a good practice to set heights on elements . Let the content inside the card element dictate the height of it. -
border-radius
andoverflow hidden
to the main container that wraps the three cards so you don't have to set it to individual corners. -
In
line-height: 3rem;
use unitless value for theline-height
, this is the preferred way to set line-height and avoid unexpected results due to inheritance.Read more in MDN.
And it is essential that interactive elements have
focus-visible
styles as well as hover styles. These need to be really clear and obvious as they are needed to help a keyboard user know where is focused on the page.General point : Really important to keep css specificity as low/flat as possible. The best way to do styling is single class selectors.
- It's recommended to use
em
andrem
units .Bothem
andrem
are flexible, Usingpx
won't allow the user to control the font size based on their needs.
Last , you can add the hover effect on the links.
Overall, your solution is good. hopefully this feedback helps.
Marked as helpful1@cyrilkodjoPosted over 2 years ago@PhoenixDev22 thanks @PhoenixDev22. This is very helpful. I'll revise my solution with your feedback. Grateful.
1 -
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