Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Flexbox Solution

@cj6046

Desktop design screenshot for the QR code component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


What are you most proud of, and what would you do differently next time?

I'm proud of getting my feet wet and jumping in.

What challenges did you encounter, and how did you overcome them?

I wasn't sure how to use flexbox in the vertical direction (column). Quick fix though.

What specific areas of your project would you like help with?

I'd like to make sure this code is readable and well organized.

Community feedback

P

@Islandstone89

Posted

Hi Chris, good job!

I've inspected your code, and these are my suggestions to improve your solution even further - I hope you find them helpful :)

HTML:

  • Remove role="main" - the <main> element has that role by default, so there is no need to declare it explicitly.

  • I would change the heading to a <h2> - a page should only have one <h1>, reserved for the main heading. As this is a card heading, it would likely not be the main heading on a page with several components.

  • Never have text in a <div> alone. "Scan the QR code" is a paragraph, so wrap it in a <p>. I would also remove the <div>, as it is not needed - divs are usually meant to group several elements for styling purposes.

  • .attribution should be a <footer>, and you should use <p> for the text inside. Move the <footer> outside of the <main>, as they are distinct landmarks.

CSS:

  • Including a CSS Reset at the top is good practice.

  • I recommend adding a bit of padding, for example 16px, on the body, to ensure the card doesn't touch the edges on small screens.

  • On the body, change height to min-height: 100svh - this way, the content will not get cut off if it grows beneath the viewport. Remove the margin, it's already set using the universal selector (*). Add flex-direction: column and gap: 2rem - the default value for flex-direction is row, which makes the footer appear besides the main. The gap creates breathing room between the main and the footer.

  • Remove the width and height in px on the card. We rarely want to give a component a fixed size, as we want it to grow and shrink according to the screen size.

  • We do want to limit the width of the card, so it doesn't get too wide on larger screens. To solve this issue, the card needs a max-width, which should not be in % but in rem. Around 20rem works well.

  • font-size must never be in px. This is a big accessibility issue, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead.

  • Paragraphs have a default value of font-weight: 400, so there is no need to declare it.

  • Since all of the text should be centered, you only need to set text-align: center on the body, and remove it elsewhere. The children will inherit the value.

  • Well done setting max-width: 100% on the image! It's also common to apply display: block.

  • Remove position: absolute, it is not needed.

1

Please log in to post a comment

Log in with GitHub
Discord logo

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