Skip to content
Submitted over 2 years ago

Qr code component using html and css

Abdullah Khallaf•60
@Abdullah-khallaf
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

Community feedback

  • PhoenixDev22•16,950
    @PhoenixDev22Posted over 2 years ago

    Greeting , I have few suggestions regarding your solution:

    • Use <main> to wrap the body content (card ) . HTML5 landmark elements are used to improve navigation .

    • I would use any <section> in this challenge. section is not meant to be used anytime you feel tempted to use a div . section is for a bigger chunk of content often titled by <h2> Read more aout usage notes.

    • To tackle the accessibility issues : Document should have one main landmark , you can add a <h1> with class="sr-only" (Hidden visually, but present for assistive tech).

    • To center the card on the middle of the page , you can use the flexbox properties and min-height: 100vh for the <body>( no need for position absolute .)like this:

    
body{
display:flex;
align-items: center;
justify-content: center;
width: 100%; 
min-height: 100vh;

    • You should use em and rem units .Both em and rem are flexible, Using px won't allow the user to control the font size based on their needs.

    • Never style on ID'S

    Overall , your solution is good. Hopefully this feedback helps.

    Marked as helpful
    1
    Abdullah Khallaf•60
    @Abdullah-khallafPosted over 2 years ago

    @PhoenixDev22 Thank you, these accessibility notes are so helpful i wasn't have any idea about them especially h1 and section for big chunks.

    I will take your tips in consideration next time.

    Thank you again ❤️🙏

    1
  • Travolgi 🍕•31,400
    @denieldenPosted over 2 years ago

    Hi Abdullah, I took some time to look at your solution and you did a great job!

    Also I have some tips for improving your code:

    • add main tag and wrap the card for Accessibility
    • Centering a section with absolute positioning is now deprecated, it uses modern css like flexbox or grid
    • add width: 25rem to container id
    • try to use flexbox to the body for center the card. Read here -> best flex guide
    • after add min-heigth: 100vh to body because Flexbox aligns to the size of the parent container

    Overall you did well :)

    Hope this help and happy coding!

    Marked as helpful
    1
    Abdullah Khallaf•60
    @Abdullah-khallafPosted over 2 years ago

    @denielden Thank you, I didn't know the main issue in accessibility so i will use it in the future. I used to use main when i have multiple sections in the page not only one like this challenges. I will take your tips in consideration next time. Thank you 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