Skip to content
Submitted about 2 years ago

Basic HTML and CSS solution for desktop and mobile.

Samuele•170
@frasconi00
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

Feedback welcome!

Community feedback

  • Adriano•38,390
    @AdrianoEscarabotePosted about 2 years ago

    Hi Samuel, how are you?

    Welcome to the front-end mentor community.

    I really liked the result of your challenge, but I have some tips that I think you will like:

    1- Document should have one main landmark, you could have put all the content inside the main tag click here

    I noticed that there is a scrollbar on the page, and the content is not centered, we can arrange both things like this:

    I removed this one:

    .container { /* margin: 20vh auto; */ }

    I added these:

    body { display: flex; align-items: center; justify-content: center; min height: 100vh; flex-direction: column; }

    This will make the content centered on any resolution.

    • { margin: 0; padding: 0; box-sizing: border-box; }

    This will remove the scrollbar, adding margin and padding 0 to all elements on the page.

    Hope it helps... 👍

    The rest is great!!

    Marked as helpful
    1
    Samuele•170
    @frasconi00Posted about 2 years ago

    @AdrianoEscarabote Thanks a lot for the feedback and advice, especially for the scrollbar removing tips! Will implement this for sure in my future projects 👍

    1
  • Lucas 👾•104,340
    @correlucasPosted about 2 years ago

    👾Hello SAMUELE, congratulations for your first solution!👋 Welcome to the Frontend Mentor Coding Community!

    Great solution and great start! By what I saw you’re on the right track. I’ve few suggestions to you that you can consider to add to your code:

    You’ve used px as the unit for sizes but the problem with pixels is that its not optimized for multiple devices and screens. So a good fit its to use rem or em that have a better performance and make your site more accessible between different screen sizes and devices. REM and EM does not just apply to font size, but to all sizes as well.

    Something I've noticed in your code is that in many occasions you've added some <div> to wrap contents that don't really need to be inside of a div block. Note that for this challenge all you need is a single block to hold all the content, can be <div> or <main> if you want to use a semantic tag to wrap the content, the cleanest structure for this challenge is made by a block of content with div/main and all the content inside of it (img, h1 and p) without need of any other div or something. See the structure below:

    <body>
<main>
<img src="./images/image-qr-code.png" alt="Qr Code Image" >
 <h1>Improve your front-end skills by building projects</h1>
<p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>
</main>
</body>

    ✌️ I hope this helps you and happy coding!

    Marked as helpful
    1
    Samuele•170
    @frasconi00Posted about 2 years ago

    @correlucas You're absolutely right, thank you for your tips! I really appreciate feedback as this is my first ever project. Hope to improve soon 💪

    1

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