Design comparison
Community feedback
- @Islandstone89Posted 10 months ago
Hi there. Here is some feedback - hopefully it will help you :)
HTML:
-
Every webpage needs a
<main>
that wraps all of the content, except for<header>
andfooter>
. This is vital for accessibility, as it helps screen readers identify the "main" section of a page. Change.box
into a<main>
. -
The image at the top is decorative, so its alt text must be empty:
alt=""
. -
The author image has meaning, so it must have a short and descriptive alt text. Do not include words like "image" or "photo". Screen readers start announcing images with "image", so an alt text of "image of author" would be read like this: "image, image of author".
-
I would wrap the date in a
<time>
tag, like this:<time datetime="2023-12-21">Published 21 Dec 2023</time>
CSS:
-
It's good practice to include a CSS Reset at the top.
-
I would add around
1rem
ofpadding
on thebody
, so the card doesn't touch the edges on small screens. -
font-size
must never be in px. This is bad for accessibility, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead. -
Change
height
tomin-height
- this way, the content will not get cut off if it grows beneath the viewport. -
Remove the
width
on the card - generally, you rarely want to set fixed dimensions, whether it's widths or heights. -
Do add a
max-width
in rem on the card - this makes sure it doesn't stretch too wide on larger screens.
Marked as helpful1@CaaspitaPosted 10 months ago@Islandstone89 Thank you very much for taking the time to review the code and provide me with constructive feedback. It helps me a lot in my development.
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