Frontend-Mentor project: QR code component

Hi @Ihdir,

Nice project! Here are a few suggestions:

Another fun way to center a <div> is by using position: relative on the parent and position: absolute on the content, with top: 50%, left: 50%, and transform: translate(-50%, -50%). For a challenge, you could also center it with Grid, but Flexbox is definitely the easiest method in this case.

I’d suggest improving your semantic structure a bit. For example, you’ve nested your component in many <div>s, when one main container for the card, combined with Flexbox on the body, would be sufficient. Using fewer <div>s makes your code easier to read and helps search engines better understand your page.

I noticed you’re using both <h1> and <h2>. While this isn’t necessarily wrong, the <h1> tag should be reserved for the page title and should only appear once. For card components or other sections, using <h2> for headings and <p> for paragraphs would be more appropriate.

Keep up the great work!

Best, Teodor

Hi, @Ihdir

I reviewed your solution and have a few thoughts:

• As Svitlana Suslenkova mentioned, you should apply min-height: 100vh to the body. This ensures that the body always covers the full height of the viewport, preventing it from being shorter than the screen and ensuring a consistent layout.
• Your main tag contains two elements: one with the .Element class and one with the .attribution class. I'd recommend giving .Element a more descriptive name. Good naming conventions improve code clarity, especially in larger projects. You can explore CSS naming conventions like BEM (Block, Element, Modifier) for better structure.
• You don’t need to use Flexbox for the card (.Element class, which is a block-level div). The child elements inside, except for the img, are also block-level, so they naturally stack on their own lines. Additionally, I’m not sure why you set width: 90% on the .Element class. On smaller viewports (under 37.5rem), it causes horizontal overflow. Simply removing this should resolve the issue.
• Lastly, the styles max-height: 43.75rem, height: auto, and overflow: auto on the card seem unnecessary as they don’t contribute much to the layout.

There are a few other areas for improvement, but I’ll keep this brief for now. 😉

It looks like you’re just starting out in front-end development—good job so far!

Hope this helps!

I see you added flex, but your project didn't align to the center. The problem is you should add to your flex also min-height: 100vh; Currently, the height of the body is the same as the component in it.

Hope you found this comment helpful :)