Skip to content
Submitted over 1 year ago

qr-code-component

Mohd Arbaz•80
@Arbaz-79
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...QR code component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

please correct my mistake

Community feedback

  • Vanza Setia•27,775
    @vanzasetiaPosted over 1 year 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 a max-width to prevent it from becoming too large. Also, it would be better if you use rem instead of px unit for the max-width.

    I hope this helps. Have fun coding! 😄

    Marked as helpful
    0
    Mohd Arbaz•80
    @Arbaz-79Posted over 1 year ago

    @vanzasetia thanks i remember next time

    1
  • Mohamad Mourad•190
    @MouradisPosted over 1 year 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 helpful
    0
    Vanza Setia•27,775
    @vanzasetiaPosted over 1 year 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 helpful
    0
  • Hassia Issah•50,650
    @HassiaiPosted over 1 year 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 helpful
    0

Please log in to post a comment

Log in with GitHub

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