@Islandstone89
Posted
My feedback:
HTML:
-
Change the
.containerinto a<main>. Main landmarks are important for accessibility. -
As mentioned above, the image needs alt text. It should describe the content, and in this case, it also needs to say where it leads. Don't use words like "photo" or "image" - a screen reader will announce it as an image.
-
"Improve your frontend skills by building projects" should be a heading. Since it's the main heading on this page, you can use a
<h1>. -
.attributionshould be a<footer>. -
"Challenge by" and "Coded by" needs to be wrapped in
<p>. This will make them stack on top of each other, since<p>is a block element. If you want to have them side by side, put this on the.attribution:
display: flex;
gap: 1rem;
This would make the paragraphs flex items, and since the flex-direction of the parent (.attribution) is by default row, it means they will line up side by side.
CSS:
-
Performance-wise, it's better to link the fonts in the
<head>of the HTML, instead of using@import. -
Put
text-align: center;on the body. The children will inherit the value, so you get the same result, with one less line of code. -
You should have a CSS Reset at the top. Check out the one from Andy Bell.
-
Remove the fixed width on
.container, fixed dimensions are not recommended. -
max-widthshould be in rem instead of px. -
Remove all positioning, including
transform. You don't need it in this challenge. -
To center the card horizontally and vertically, put this on the body:
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
min-height: 100vh;
<img>should have amax-widthof 100%. This is included in the mentioned CSS Reset by Andy Bell.
Hope this helps, good luck :)
Marked as helpful
@DruxAMB
Posted
@Islandstone89 thanks man
