Skip to content
Submitted about 2 years ago

Responsive QR card

Tora Wijaya•170
@Mirare23
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

Hellooo, i just finished my 4 project today. if anything wrong / have better solution please let me know : )

Community feedback

  • バレンタイン 😈•64,190
    @VCaramesPosted about 2 years ago

    Hey @Torawijaya, some suggestions to improve you code:

    • Your not using the picture and article element correctly.

    1. The picture element isnt needed for this challenge; you only have one image.

    2. The Article Element should wrap the entire card. the article event is meant for content that is reusable and can stand on its own. The text by itself can't stand on its own, since it needs a QR code to scan.

    <body>
  <main>
    <article></article>
  </main>
</body>

    • The QR image isnt decorative, it serves a purpose. So the Alt Tag description for the QR image needs to have a description.** it can't be blank.** Its needs to tell screenreader users what it is and where it will take them to when they scan it.

    • There's a lot of unnecessary CSS code being used, that is just bloating your code. Remove the following:

    main {
    display: grid;
    grid-template-columns: auto;
    margin: 2rem;
}

    img {
    height: auto;
    width: 100%;
}

    picture {
    max-width: 100%;
    padding: 1rem 1rem 0 1rem;
}

    article {
    display: flex;
    flex-direction: column;
}
    • Change the following:

    Old:

    main {
    margin: 2rem;
}

    New:

    main {
    padding: 1rem; 
}

    Old:

    article {
    padding: 2rem 1rem;
}

    New:

    article {
    padding: 0.63rem;
}

    Happy Coding!

    Marked as helpful
    0
    Tora Wijaya•170
    @Mirare23Posted about 2 years ago

    @vcarames Hello vcarames, thank a lot for the better solution, i alr remove the unnecessary css that you mention. but i want to ask some question, do we need to put some margin in <main> so it looks clean when the resolution under 320px. Thanks :)

    0
    バレンタイン 😈•64,190
    @VCaramesPosted about 2 years ago

    @Torawijaya

    For anything less than 375px, you will need a margin of maybe 10 - 16px...

    The lowest resolution that you need (but not required for this challenge) to is 320px. Anything after that your building a whole new site to accommodate smaller devices.

    Marked as helpful
    0
    Tora Wijaya•170
    @Mirare23Posted about 2 years ago

    @vcarames Thanks, it help me a lot.

    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