Design comparison
Solution retrospective
please correct my mistake
Community feedback
- @vanzasetiaPosted almost 2 years ago
Hi, Arbaz-79! 👋
I recommend removing all the
<div>
elements. You don't need any of them.- Make the
<body>
element as the flex container of the card instead of using the<main>
element. - Then, use the
<main>
element as the card. - You don't need to wrap the QR code and the text with
<div>
elements. - Add the
padding
to the card element to control the text wrapping.
Another recommendation I have is to remove the
height
property from the.container
. The card only needs amax-width
to prevent it from becoming too large. Also, it would be better if you userem
instead ofpx
unit for themax-width
.I hope this helps. Have fun coding! 😄
Marked as helpful0 - Make the
- @MouradisPosted almost 2 years ago
you dont have mistakes you can just remove the border from the image and i personally prefer that you put the <h1> and the <p> each in it own div and put those tow divs in a section for bigger porjects its better , again its not a mistake it is just my prefrences and you may benifit from them
Marked as helpful0@vanzasetiaPosted almost 2 years ago@Mouradis
It is not useful to wrap the
<h1>
and<p>
elements into their own<div>
. In this case, the HTML markup should be kept simple.Marked as helpful0 - @HassiaiPosted almost 2 years ago
The body has a wrong background-color. Use the colors that were given in the styleguide.md found in the zip folder you downloaded.
To center .container on the page using flexbox, replace the height in .wrap with min-height: 100vh.
Replace the height in .container with a padding value for all the sides, this will prevent the content from overflowing on smaller screens and its a responsive replacement.
padding:16px
.Give .qr-img a max-width of 100% and a border-radius value , the rest are not needed
Giv .qr-text a margin value for all the sides, text-align: center and a font-size of 15px which is 0.9375rem, this will be the font-size of both p and h1.
Use relative units like rem or em as unit for the padding, margin, width values and preferably rem for the font-size values, instead of using px which is an absolute unit. For more on CSS units Click here
Hope am helpful.
Well done for completing this challenge. HAPPY CODING
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