Skip to content
Submitted about 2 years ago

Pure Html5 & Css3

Unzila Maqsood•60
@Unzila02
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

this is my second challenge solution and I improve a lot thanks for my mentor and also how guide me in comment. I face some difficultly but I solve it.

Community feedback

  • Ebere Ndukwu•180
    @eby-coderPosted about 2 years ago

    Hi👋. Congrats on completing this challenge. I'll like to point out a few things that I think may help.

    Firstly, it is the best practice to make your code as effective as possible. Therefore, any div or style that is not needed should be removed rather than put in comments.

      <div class="attribution">
    Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>. 
    Coded by <a href="unzila.dorik.io">Unzila02</a>.
  </div>

    It will be best to remove any unnecessary classes so that your code would look clean.

    In addition, while your desktop view is beautiful, the mobile view doesn't look nice. This is because you set the width of ur container in percentage.

    .container{
    margin: 10% auto;
    width:22%;
    background-color:hsl(0, 0%, 100%) ;
    border-radius: 15px;
}

    This width: 22% sets the width of the container to 22% of the screen's width, hence, while it may look okay when viewed with a laptop, when you use a phone, it looks bad.

    The best way to set your width is to use pixels, so that the width is fixed regardless of what the screen size may be.

    I hope this helps.😁

    1
    Unzila Maqsood•60
    @Unzila02Posted about 2 years ago

    @eby-coder sorry I just forgot to style. I style it later

    0
    Unzila Maqsood•60
    @Unzila02Posted about 2 years ago

    @eby-coder sorry I just forgot to style. I style it later and I learn bootstrap to make my site responsive so in next challenge I use bootstrap. Thank for guideline

    0
  • PhoenixDev22•16,950
    @PhoenixDev22Posted about 2 years ago

    Hi Unzila Maqsood,

    Congratulation on completing this challenge. Your solution looks great. I have some suggestions regarding your solution if you don’t mind:

    • You should use <main> to wrap the card. HTML5 landmark elements are used to improve navigation experience on your site for users of assistive technology.
    • In my opinion, the alternate is needed on this image. It should indicate where the Qr code navigate the user : like QR code to frontend mentor .
    • In order to center the card on the middle of the page , you can use the flex or grid properties and min-height: 100vh to the ``<body>`. Add a little padding to the body that way it stops the component from hitting the edges of the browser. Remove the position and the related properties.
    • Using percentages alone for width is not a good way to have a responsive layout as you are losing control of the layout. Consider using max-width to the card instead.
    • Remember a css reset on every project. That will make all browsers display elements the same.

    Hopefully this feedback helps.

    0
    Unzila Maqsood•60
    @Unzila02Posted about 2 years ago

    @PhoenixDev22 thanks for guiding be I just learning bootstrap and then I apply them I don't know yet.in week I will able to apply and then I upgrade this file

    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