Skip to content
Submitted about 2 years ago

QR code round 2

jom•130
@Jmlsoteyza
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

Actually, i'm having a problem to center a div, that's why I try to make myself another try. When I looked at others solution I was impressed and sad at the same time. But on the other hand, I learned from my mistakes although this 2nd try is still not perfect but it looks much better than my 1st try. So, if you guys have advice or see any mistakes on my code just tell me, I'll appreciate it and consume it to make me a better in coding.

Community feedback

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

    Hi jom,

    Congratulation on completing this frontend mentor challenge. Your solution looks great. I have some suggestions regarding your solution:

    • <header> is used to define the site’s header. It typically includes a logo and navigation elements, generally at the top of a page. So you should remove the <header> .

    The HTML needs a little refactoring:

    <main>
<div class=”Cardbg”>
<img>
<h1>...</h1>
<p>.....</p>
</div
</main>
    • In my opinion, the image is an important content. The alternate text is needed on this image. The alternate text should indicate where the Qr code navigate the user : like QR code to frontend mentor not describes the image.
    • In order to center the card on the middle of the page , you can use the flexbox properties and min-height: 100vh for the <body> add a little padding to the body that way it stops the card from hitting the edges of the browser.
    • width: 350px; an explicit width is not a good way to have responsive layout . Consider using max-width to the card in rem instead .
    • height: 530px;It's not recommended to set height to component, let the content of the component define the height.
    • Consider using rem and em units as they are flexible, specially for font size better to use rem. If your web content font sizes are set in absolute units, such as pixels, the user will not be able to re-size the text or control the font size based on their needs. Relative units “stretch” according to the screen size and/or user’s preferred font size, and work on a large range of devices
    • Remember a css reset on every project. That will do things like set the images to display block and make all browsers display elements the same.

    Overall, Excellent work! Hopefully this feedback helps.

    Marked as helpful
    1
    jom•130
    @JmlsoteyzaPosted about 2 years ago

    @PhoenixDev22 Thank you for telling me my mistakes it's such a helpful, I'll keep in mind your advice. Appreciate it thank you bro!!

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

    @Jmlsoteyza Glad to help. Happy coding!

    0
  • Emeghiebo Chizoba•250
    @chizobaemeghieboPosted about 2 years ago

    Hello there jom, Great work on completing the project. I just wanted to add a little something to what @PhoenixDev22 said already, in addition to setting a max-width, you should also add a default width e.g max-width: 350px; width: 90% This helps the content not start at the edge of the screen for screens lower than 350px. Good luck with further projects.

    Marked as helpful
    0
    jom•130
    @JmlsoteyzaPosted about 2 years ago

    @chizoba-009 Thank you for that, I've googled it to understand more and get it what you mean. Appreciate it!!

    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