Design comparison
Community feedback
- @Islandstone89Posted about 1 month ago
Great job, Raphael!
Here are my suggestions for an even better solution. I hope you find them helpful :)
HTML:
- I would change the heading to a
<h2>
- a page should only have one<h1>
, reserved for the main heading. As this is a card heading, it would likely not be the main heading on a page with several components.
CSS:
-
Including a CSS Reset at the top is good practice.
-
Remove the styles on
main
, they are not needed. -
max-width
on the card should be in rem. Around20rem
will work fine. -
Remove
width: 100%
, as block elements take up the full width by default. -
font-size
must never be in px. This is a big accessibility issue, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead. -
On the image, remove
max-height
and changemax-width
to100%
- the max-width prevents it from overflowing its container. Without this, an image would overflow if its intrinsic size is wider than the container.max-width: 100%
makes the image shrink to fit inside its container.
Keep up the good work!
Marked as helpful1@perfidevPosted 30 days ago@Islandstone89 Thank you very much for the suggestions. I've already updated my code. 😄
1 - I would change the heading to a
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