@Islandstone89
Posted
HTML:
-
The alt text must also say where it leads(the frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website."
-
.attribution
should be a<footer>
, and you should use<p>
for the text inside.
CSS:
-
Including a CSS Reset at the top is good practice.
-
I would recommend adding
1rem
ofpadding
on thebody
, to ensure the card doesn't touch the edges on small screens. -
I usually set
font-family
on thebody
instead of:root
. -
I would move the properties on
main
tobody
, this way you can center both the main and the footer. Addflex-direction: column
andgap: 2rem
and changeheight
tomin-height
- this way, the content will not get cut off if it grows beneath the viewport. Also, removewidth: 100%
- a block element likebody
takes up the full width by default. -
It's recommended to use
px
instead of%
forborder-radius
. -
Well done for giving the card a suitable
max-width
, so it doesn't stretch indefinitely. However, it's best practice to use rem, so change it tomax-width: 20rem
. -
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. -
letter-spacing
must also never be inpx
. You can useem
, where1em
equals the element's font size. -
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. NB:text-align: center
doesn't apply to images. -
On the image, add
display: block
and changewidth
tomax-width: 100%
- 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.
Marked as helpful