@mkboris
Posted
Great work @Erika-codes, here are a few things to review
- It's not good practice wrapping every element in
div
because divs aren't semantic. Use more meaningful semantic elements likemain
,section
, andfooter
. Wrap amain
tag around the .card and afooter
for the attribution. - Use heading tags
h1
,h2
for the title instead of wrapping it in ap
tag. This makes the structure more accessible. - Your image needs a more descriptive alt attribute like so
alt="Qr Code to Frontendmentor.io"
- To properly center the card, add this on your
body
and remove the all the margin properties from the .card
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
min-height: 100vh;
- Avoid setting fixed
heights
andwidths
on elements, as this can create problems with responsiveness and content fit. Instead, let the content and padding determine the element’s size. If necessary, usemax-width
ormin-height
, and prefer relative units like rem for better adaptability. Change the width of the .card tomax-width
and it should be defined inrem
. - The link to the github repo seems to have an issue, you might want to check that
Hope this helps, Good luck!
Marked as helpful