Skip to content
Submitted 4 days ago

QR Code Component

TheSecondChance•10
@TheSecondChance
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

Community feedback

  • Kamran Kiani•1,150
    @kaamiikPosted 4 days ago

    Hi. Congratulation for doing this challenge. Your HTML is brief and clean. I have some notes on your code I wanna mention:

    • You can check most of the pages and see they have a structure like this inside the body:
    <body>
  <header>...<header>
  <main>...<main>
  <footer>...<footer>
</body>

    • Based on your design you may have header and footer or not. But You should have a main element inside your page. So after your body always wrap all of your code inside a main element. In this design you do not need header but yourfooter is your .attribution.

    • Each Page should have a h1 heading. In this challenge though, Because in a real scenario the QR code is part of another page It does not need to have h1 and you can use a h2.

    • In most of the times there is no need to add br tag to wrap your text to the next line. Just use a max-width for your container and also add a proper padding to your container too. Then let the browser decide when wrap the text.

    • Try to use a proper CSS reset at the start of your CSS style. Andy Bell and Josh Comeau both have a good one. You can simply search on the internet to find them.

    • Use min-height: 100vh; instead of height:100vh;. height: 100vh strictly limits the height to the viewport size, potentially causing overflow issues if the content is larger than the viewport. On the other hand, min-height: 100vh allows your element to grow in height if the content exceeds the viewport size. And there is no need for calc.

    • Never limit your width and height in a container or element or tag that contains text inside. When you limit the width and height of elements containing text, you risk the text being cut off, overflowing, or becoming unreadable, especially on smaller screens or when the text dynamically changes. It's generally better to allow the container to adjust its size based on its content or set a flexible size that can adapt to different screen sizes and text lengths. You only need max-width here because it prevents elements from stretching beyond a certain point, keeping them visually appealing across different screen sizes. It ensures your design remains adaptive and doesn't get too wide on larger screens. And also no need for media query!

    • Your font-size and max-width should be in rem unit not px. You can read this article about it and why you should not use px as a font-size.

    • Add your text-align: center; to the body because all of your text has the center alignment.

    1
  • Psalmorg•50
    @PsalmorgPosted 4 days ago

    I don't have much understanding of Semantic HTML but i putting the last code at the bottom of the Picture in footer tag is good.

    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