@Islandstone89
Posted
Hi, well done!
Some of the things I like about your solution:
- Including a proper CSS Reset
- Use of Custom Properties for colors, typography and spacing
- Font sizes in rem instead of
px
, which is vital for accessibility.
All of these are things that 90% of people miss out on, so great work!
A few suggestions:
HTML:
-
I would wrap the card in a
<div class="card">
inside of<main>
. The<main>
holds all of the main content on a page. Since a card is rarely the only component on a page, it makes sense to have it in its own<div>
. -
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."
-
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.
CSS:
-
I would add
1rem
ofpadding
on thebody
, to ensure the card doesn't touch the edges on small screens. -
Remove
height: 100%
onhtml, body
, and instead usemin-height: 100svh
on thebody
. -
Instead of setting a fixed width, we want the card to grow and shrink according to the viewport. However, we do want to limit the width of the card, so it doesn't get too wide on larger screens. To solve these issues, change
width
tomax-width: 20rem
. -
letter-spacing
should not be inpx
. You can useem
, where1em
equals the element's font size. -
Instead of writing
.card .text
, I would just write.text
,as the former creates more specificity than needed.
Kepp up the good work :)
Marked as helpful