Design comparison
Community feedback
- @Islandstone89Posted 2 months ago
Hi, you've done a great job!
Some of the things I like about your solution:
- Using the
<main>
semantic element - Using Custom Properties
- Font sizes in rem instead of
px
, which is vital for accessibility - No fixed widths or heights in
px
. max-width
in rem on the card
A few suggestions:
HTML:
-
<main>
holds all of the main content on a page. As a card would likely not be the only component on a page, I would wrap the card content in a<div class="card">
inside of<main>
. -
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 would recommend adding
1rem
ofpadding
on thebody
, to ensure the card doesn't touch the edges on small screens. -
On the
body
, changeheight
tomin-height
- this way, the content will not get cut off if it grows beneath the viewport. -
An alternative 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;
-
Instead of
main p
andmain h2
, I would give the elements a class and use only that as the selector. Descendant selectors likemain p
have higher specificity than a single class selector, meaning they are harder to override. -
Move the styles on
main
to the<div>
you created that holds the card content. -
letter-spacing
must not be inpx
. You can useem
, where1em
equals the element's font size. -
It's common to add
display: block
on images - the defaultdisplay: inline
can cause unwanted space under an image - if an image is wrapped in a<div>
, it sometimes does not fill the entire height of that<div>
.
Well done, and keep up the good work :)
Marked as helpful1 - Using the
- @StroudyPosted 2 months ago
Hey, Your CSS is great, Impressed at this stage! Great job with this solution you should be proud, A few things I noticed,
- Missing a
<meta>
description tag for SEO purposes, - Setting a height and width attribute to your
<img>
will increase performance to reduce layout shifts and improve CLS, It reserves the space on the page for the image, - Having better
alt=""
descriptions for accessibility is a must check this out Write helpful Alt Text to describe images, - You should apply a full modern reset to make things easier as you build, check out this site for a Full modern reset
I hope you found some of this information helpful, You should give the articles a good read and I look forward to seeing some more from you, Happy coding! 💻
Marked as helpful1 - Missing a
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