Design comparison
Community feedback
- @Islandstone89Posted 11 months ago
HTML:
-
You need a
<main>
, this is important for accessibility. The<main>
element should wrap all of the content on a page, except for<header>
and<footer>
. As the card is the only "main" content, you should change.card
into a<main>
. -
The alt text needs to say where it leads (frontendmentor.io). It must also be written naturally, so instead of
qr-code
, you would writeqr code
. -
There is no need to wrap the text in a
<div>
, so I would remove it. -
Headings should always be in order, so you never start with a
<h4>
. Change it into a<h1>
.
CSS:
-
The styling should be written in a separate file, usually called
style.css
. You then link to it in the<head>
section. -
Use the style guide to get the correct font.
-
It's good practice to include a CSS Reset at the top.
-
The
body
needs amin-height
instead of aheight
. This way, content wouldn't be cut off if it grows beneath the viewport. -
font-size
must never be in px. This is bad for accessibility, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead. -
Remove all widths and heights in percentages. You rarely need to set widths or heights, especially heights - the content should determine the tallness of a component.
-
Add a
max-width
of around 20rem on the card, to prevent it from getting too wide on larger screens. -
Remove
text-align: center
everywhere except for the one on.card
. Descendants inherit typographic properties (among others) from their ancestors. -
The image should have a
border-radius
similar to the one on the card. Remove themargin
, and change themax-width
to100%
. It's also common for images to havedisplay: block
. -
Add
padding
on all 4 sides of the card, to create the space between its edge and the image. -
As the design doesn't change, there is no need for any media queries. When you do need them, they should be in rem, not px. And it is common practice to do mobile styles first and use media queries for larger screens.
Marked as helpful1 -
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