@Islandstone89
Posted
Hey, good job on creating this card! Here are some tips to make it even better :)
HTML:
-
I would remove the divs, as you don't need them here.
-
The image must have alt text. This is essential for screen readers to understand the image. The alt text should be descriptive, and in this example, it also needs to say where it leads (frontendmentor.io).
-
Footer text needs to be wrapped in a
<p>.
CSS:
-
Performance-wise, it's better to link fonts in the
<head>of the HTML then using@import. -
It's good practice to include a CSS Reset at the top.
-
max-widthon the card should be in rem. Around20remworks fine. -
Font-size must never be in px. Use rem instead, as you have done on everything except for
.attribution. -
Since all of the text is centered, put
text-align: centeron the body, and remove it elsewhere. The result will be the same but with less code. It works because of inheritance - the children inherit the value from their ancestor, thebody. -
Remove
margin: 150px autoon the card. A better way to center it horizontally and vertically, is to use either Grid or Flexbox, on its parent, which in this example is thebody:
Grid:
display: grid;
place-content: center;
min-height: 100vh;
Flexbox:
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
min-height: 100vh;
Marked as helpful
@AntonielAureliano
Posted
@Islandstone89 Thank you very much for the tips. They really helped me with any doubts I had and will help me write leaner code. I'm grateful for your feedback.
