Skip to content
Submitted about 2 years ago

QR code

Noseriouszv•90
@Noseriousv
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

Best practices

Community feedback

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

    Hi Noseriouszv,

    Congratulation on competing your first frontend mentor challenge.

    Well done! I have some suggestions regarding your solution if you don't mind:

    • You should use<main>landmark to wrap the card as landmarks allow screen reader users to navigate through sections of your website by skipping to content that interests them. Landmarks could be seen as the logical layout of the website's UI, which is divided into e.g. header, navigation, main content, and footer. So the usage makes sense in any case.
    • Page should contain <h1> . The <h1> is most commonly used to mark up a web page title. This challenge is supposed to be one component of a web page. To tackle the accessibility issue in the report , you may use an <h1> visually hidden with class=”sr-only”. You can find it here.
    • In my opinion, the image is an important content. The alternate text is needed on this image. The alternate text should indicate where the Qr code navigate the user : like QR code to frontend mentor not describes the image.
    • You should use <p> instead of <h4> . For future use , consider using the headers in a chronological order. How you order headings dictates how a screen reader will navigate through them. As you go down a level, the number should increase by one, like a numbered list within an outline.
    • In order to center the card on the middle of the page , you can use the flexbox properties and min-height: 100vh for the <body> add a little padding to the body that way it stops the card from hitting the edges of the browser. Then you can remove.Container-1.
    • height: 35.625rem;It's not recommended to set fixed height to component, you almost never want to set it. let the content of the component define the height
    • width: 21.875rem; an explicit width is not a good way to have responsive layout . Consider using max-width to the card instead.
    • Consider using rem for font size , it' not recommended to use px for font size as absolute units don’t scale for example 15px will always be 15px on the same device. Using pixels is a particularly bad practice for font sizing because it can create some accessibility problems for users with vision impairments. Remember a modern css reset on every project that make all browsers display elements the same.
    • Include a git ignore. This came with your starter files and it is extremely important as you move onto larger projects with build steps.
    • Last, Don’t Repeat Your CSS is a good general principle to follow and eliminating duplication of css code should naturally be part of coding journey.

    Hopefully this feedback helps.

    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