Responsive QR Code component using media queries

Solution retrospective
I would use flexbox next time.
What challenges did you encounter, and how did you overcome them?I faced challenges mainly with making the QR code component responsive with smaller screen sizes. I solved it by adding a width property along with max-width and using a bunch of media queries.
What specific areas of your project would you like help with?I know it would have been better if I used flexbox, but I feel the code I wrote could have been written better and more succinctly. I would appreciate if someone could take a look and let me know where I can improve on.
Please log in to post a comment
Log in with GitHubCommunity feedback
- P@Islandstone89
Hi, good job.
Here are some suggestions - I hope my feedback is clear and helpful!
HTML:
-
You don't need to include "image" in the alt text, as "qr code image" would be read by screen readers as "image, qr code image". Also, the alt text must 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. -
Wrap the footer text in a
<p>
. -
The
<footer>
must be outside of the<main>
- both should be direct children of the<body>
.
CSS:
-
Make a habit of including a modern CSS Reset at the top of the stylesheet.
-
I recommend adding a bit of
padding
, for example16px
, on thebody
, to ensure the card doesn't touch the edges on small screens. -
Move
box-sizing: border-box
to*, *::before, *::after
. Remove the remaining styles onhtml
, as it's not common to set styles on thehtml
. NB: Do not change thefont-size
to62.5%
. -
It's common in a CSS Reset to set the
line-height
onbody
to1.5
. -
Remove
height: 100%
on thebody
. -
Remove all positioning and transform properties, as these are not the best options for centering components.
-
To center the card horizontally and vertically, with some space between the
<main>
and the<footer>
, I would use Flexbox on the body:
display: flex; flex-direction: column; justify-content: center; align-items: center; min-height: 100svh; gap: 1rem;
-
Remove all widths in
%
. -
I would adjust the
max-width
on the card to around20rem
. -
Remove the
height
on the card. You should never limit the height of elements containing text - it will cause overflow if the text grows taller than the fixed size. Let the content decide the height, which it does by default. -
Since you've removed
font-size: 62.5%
, the text will probably look a bit too big - I would removefont-size
on the heading, paragraph and the footer, as their default font size looks alright. -
Since all of the text should be centered, you only need to set
text-align: center
on the body, and remove it elsewhere. The children will inherit the value. -
Paragraphs have a default value of
font-weight: 400
, so there is no need to declare it. -
Descendant selectors like
.card-content > p
increase specificity, making the styles harder to override. It's best practice to instead give elements a class, and use that as the selector. -
On the image, add
display: block
,height: auto
andmax-width: 100%
- the max-width prevents it from overflowing its container. Without this, an image would overflow if its intrinsic size is wider than the container.max-width: 100%
makes the image shrink to fit inside its container. -
As the design doesn't change, there is no need for any media queries. When you do need them, they should be in
rem
orem
, notpx
. Also, it is common practice to do mobile styles first and use media queries for larger screens.
Good luck :)
Marked as helpful -
- @dilip43
the ui is looking good
Join 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