@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. Change.container
into a<main>
. -
The alt text must also say where it leads(frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website."
-
As this is a card heading, it would probably not be the main heading on a webpage. Hence, I would change it to a
<h2>
.
CSS:
-
I'm not sure how much
normalize.css
is used nowadays. I like to use this CSS reset from Andy Bell. -
You don't need the
.clear
or.clearfix
, so remove those. -
I like to add
1rem
ofpadding
on thebody
, to ensure the card doesn't touch the edges on small screens. -
I would remove the margin on the card, and center the card horizontally using Flexbox. Add
justify-content
on.container
.height
should bemin-height
- we never want to limit the height of text elements, as that would cause overflow if we have enough text. -
Remove all widths.
-
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. -
On the image, add
display: block
andmax-width: 100%
- the max-width prevents it from overflowing its container. Removemargin-top
. -
To create the space between the image and the edge of the card, set
padding
on all 4 sides of the card:padding: 1rem
.
Marked as helpful