Design comparison
Community feedback
- @Islandstone89Posted 3 months ago
HTML:
-
I would change the heading to a
<h2>
- a page should only have one<h1>
, reserved for the main heading. As this is a card heading, it would likely not be the main heading on a page with several components. -
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 the first<div>
into a<main>
, and removerole="main"
, as that is the implicit role of the<main>
element. I would also change the class of the second<div>
tocard
, meaning your structure should look like this:
<main> <div class="card"> <div class="image"> <img> </div> <div class="text"> <h2></h2> <p></p> </div> </div> </main>
- The alt text should be written naturally, without using
-
between the words. Write something short and descriptive, without including words like "image" or "photo". Screen readers start announcing images with "image", so an alt text of "image of qr code" would be read like this: "image, image of qr code". 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."
CSS:
-
Including a CSS Reset at the top is good practice.
-
I like to add
1rem
ofpadding
on thebody
, to ensure the card doesn't touch the edges on small screens. -
Remove the following from the card:
position: absolute; top: 50%; left: 50%; transform: translateX(-50%) translateY(-50%);
- A better way to center the card is to use Flexbox on the
body
:
display: flex; flex-direction: column; justify-content: center; align-items: center; min-height: 100svh;
-
Remove the
width
andheight
in px on the card. We rarely want to give a component a fixed size, as we want it to grow and shrink according to the screen size. -
We do want to limit the width of the card, so it doesn't get too wide on larger screens. Give the card a max-width of around 20rem to solve this issue.
-
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. -
Set
font-family
on thebody
, and remove it elsewhere. The children of the body will inherit the font. -
400
is not a valid value forfont-family
, I assume you meantfont-weight: 400
. However, this is not needed, as that is the default font weight for paragraphs. -
On the image, add
width: 100%
. -
Move
padding: 16px
fromsub.card
to.card
.
Marked as helpful0 -
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord