Skip to content
Submitted 11 months ago

Qr-code-component

raya•2,870
@rayaatta
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

Qr-code-component Project repeat finished I decided to repeat this project in order to re-write the whole code without using pixels as the base size units All feedback welcomed

Community feedback

  • Øystein Håberg•10,710
    @Islandstone89Posted 11 months ago

    Hey, good job on re-doing the challenge after getting feedback - this is vital for the learning process. You are getting there, only a couple of tweaks left. Keep it up :)

    HTML:

    • You don't need to wrap the card content in a <section>, so I would remove it.

    • The alt text also needs to say where it leads (frontendmentor.io).

    • I would add the attribution class to the <footer>, and delete the div, as you don't need it.

    • Footer text needs to be wrapped in a <p>

    CSS:

    • Performance-wise, it's better to link fonts in the <head> of the HTML than using @import.

    • It's good practice to include a CSS Reset at the top.

    • It's common to set text-align and font-family on the body.

    • Remove the percentage width on the card. It only needs a max-width of around 20rem, to prevent it from getting too wide on larger screens.

    • The image should not have width but a max-width: 100% - the max-width prevents it from overflowing its container. Also, add display: block.

    • Finally, remove all of the media queries - as the design doesn't change, there is simply no need for media queries. And you don't want as many as 7 - 1, or 2 is usually enough. When you do need media queries, remember that they should not be in px, but in rem.

    Marked as helpful
    1
    raya•2,870
    @rayaattaPosted 11 months ago

    Thanks a lot but I don't think I like the idea of using someone else's reset as it affects the styles am adding Instead I normally use the

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

    And then struggle with the rest. That aside your feedback is invaluable and has helped me get better. Thanks a lot. @Islandstone89

    1
  • Daniel 🛸•44,310
    @danielmrz-devPosted 11 months ago

    Hello @rayaatta!

    Your project looks great!

    I have one suggestion for you:

    • Since this project is not responsive (desktop and mobile versions are the same), avoid using percentage values for the card's width. You can use fixed values in this case so you prevent the card from overgrowing. You don't even need to use media queries for this one.

    I hope it helps!

    Other than that, excelent job!

    2

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