Skip to content
Submitted about 1 year ago

QR Component using Flexbox

Matias•50
@matiaslagoevia
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

This was a fun challenge to start out with! I've included more info in the repo's README, but here's a summary of the questions I'd have.

  • I wasn't comfortable with having the "title" of the card use an h* tag and changing it's size. I'm starting to think that it could be more semantic to indicate it's a title, and that this would be ok?
  • Are the placement techniques/thought process I describe in the repo ok? Was there another way I could've addressed the spacing/centering main content w/ footer problem?
  • I followed the Figma spec to have the card and font sizes be accurate, but some of the spacing within the card I wasn't able to get exact. Rather than tinker with them manually to try to get them to match the design pixel-for-pixel, I decided it'd be better to let the browser pick an appropriate padding based on the font size and element type. Would you agree with this approach? Should I have done it differently?

Thanks for your time and feedback!

Community feedback

  • Øystein Håberg•11,380
    @Islandstone89Posted about 1 year ago

    Hi, hope this feedback is helpful.

    HTML:

    • It's essential that the image has alt text. The alt text should describe the content, and in this case, where it leads.

    • "Improve your front-end skills by building projects" should definitely be a heading. And remove the <b>, it's not much used these days.

    • "Liked it? See more of my stuff" needs to be marked up as a paragraph.

    CSS:

    • You need a CSS Reset at the top.

    • Height on body should be min-height.

    • To center the card, I prefer using Flexbox on this challenge. Add this to the body:

    justify-content: center;
align-items: center;

    • Then you can remove flex: 1 on the <main>.

    • Both <main> and <footer> has centered text, so you can set text-align: center on the body. Same result, one less line of code.

    • Font-size should, contrary to popular belief, never be in px. Use rem instead.

    • Remove the fixed width on the card. You rarely want to set fixed widths/heights.

    • Do add a max-width in rem, to prevent the card from getting too big at larger screens.

    • Add some spacing between the card and the footer. Since they are both flex children of the body, you can add a gap: on the body. Again, get in the habit of using relative units like rem or em.

    Good luck :)

    Marked as helpful
    1
    Matias•50
    @matiaslagoeviaPosted about 1 year ago

    @Islandstone89 Thanks a lot for taking the time to write such a detailed reply, it was most certainly helpful!

    I believe I understood most of the feedback, and it should be addressed in the v1-feedback branch. Please let me know if I misunderstood anything!

    The items I don't understand are related to your approach about centering w/ the footer. Would you mind making a PR against v1-feedback with the changes that you don't see addressed in there already so that I can take a look and see if I understand you better? I'd be happy to merge your PR directly once I get a better understanding for the decision.

    Thanks again!

    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