@Islandstone89
Posted
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 a page's "main" section. Wrap the card in a<main>
. -
Do not use words as "picture" or "image" in the alt text, as screen readers announce that automatically. The alt text must also say where it leads(frontendmentor website).
-
Headings should always be in order, so you never start with a
<h4>
. Since this is a component heading, it should be a<h2>
.
CSS:
-
Including a CSS Reset at the top is good practice.
-
Add around
1rem
ofpadding
on thebody
, so the card doesn't touch the edges on small screens. -
On the
body
, changeheight
tomin-height
- this way, the content will not get cut off if it grows beneath the viewport. -
Remove all widths and heights in
px
. -
Add a
max-width
of around20rem
on the card, to prevent it from getting too wide on larger screens. -
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. -
Since all of the text should be centered, you only need to set
text-align: center
on the body, and remove it elsewhere. The children will inherit the value. -
I would use
px
instead of%
forborder-radius
. -
On the image, add
display: block
andmax-width: 100%
- the max-width prevents it from overflowing its container. Remove the margin. -
To create the space between the image and the edge of the card, set
padding
on all 4 sides of the card:padding: 16px
. -
Whenever the left and right margins are identical, you can use the shorthand:
padding-inline: 32px
.
Marked as helpful
@i-prkr
Posted
@Islandstone89 Amazing feedback, just the kind of CSS guidance I was hoping for. Thanks for reasoning and links - appreciate your time!