Skip to content
Submitted 11 months ago

QR Code Component using Flexbox, CSS, and HTML

Alex-coding-3420•40
@Alex-coding-3420
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

Thank you for looking at my solution for this project. Please share any insights or suggestions you have—I'm eager to learn and enhance my work further!

Community feedback

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

    Hi, you have done a decent job. Here is my feedback.

    HTML:

    • You need a <main>, this is important for accessibility. Since the card is the only "main" content on the page, the <main> could also be the card itself. I would change .container into a <main>, and remove section>, you don't need that element here.

    • The image must have alt text. This is essential for screen readers to understand the image. The alt text should be descriptive, and in this example, it also needs to say where it leads (frontendmentor.io).

    • I see you have commented out the .attribution. If you were to include it, it should not be a <div>, but a <footer>, with its text being wrapped in a <p>. But it's OK to omit it, too.

    CSS:

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

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

    • Do not set font-size: 62.5% on the html>.

    • Since all text is center-aligned, you can set text-align: center on the body, and remove it elsewhere.

    • You are mostly using rem for font sizes, that's good. If you include it, font-size on .attribution should also be in rem.

    • To center the card horizontally and vertically, use Flexbox on the body:

    display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
min-height: 100vh;
    • Hence, you don't need auto margin on .container - but change 5rem to 1rem, to prevent the card from touching the edge of the screen on smaller sizes.
    Marked as helpful
    1
    Alex-coding-3420•40
    @Alex-coding-3420Posted 11 months ago

    @Islandstone89 Thank you for the feedback, and thanks for linking the article regarding setting the font-size to 62.5% on the HTML. I'll take all this feedback for my next challenge attempt.

    1
  • amakura-411•60
    @amakura-411Posted 11 months ago

    Just an amateur's opinion, I seems like to see html's height is not fit.

    I checked the code you provided using developer tools, and the height of the screen's height is 800px, while the height of the <html> element is 569px.

    So, I've made some adjustments to the code, adding height: 100vh; display: flex, justify-content: center;, and align-items: center; to the body's style

    Here's the updated snippet:

    body {
  font-family: var(--font-family);
  font-size: var(--font-size);
  background-color: var(--light-gray);
    height: 100vh;
    display: flex;
    justify-content: center;
    align-items: center;
}

    Surely, it should work out fine! Good Luck ALEX!

    Marked as helpful
    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