Submitted almost 2 years ago

qr-code-component-main

Ranjana Mukhia• 150

@ranjanamukhia

Preview site View code

This is a solution for...

QR code component

HTML
CSS

1newbie

View challenge

Design comparison

SolutionDesign

Solution retrospective

I have revisited this Challenge and i would really like to get feedbacks for it. Some of the points i can think of :- Some of the points which i think of are:- 1.why my screenshot is not similar to the design screenshot..even though on browser mobile and desktop it looks almost same to me ? 2.is my media query breakpoint correct? 3.Any suggestion related to good practice is also welcome.

Community feedback

Alex• 1,990

@AlexKMarshall

Posted almost 2 years ago

This looks good to me. The one thing I'd change is to add some padding to the .card

That way you can remove the max-width from the text and the image, and a few of the margins.

In general, if you're looking to keep the children away from the sides of a parent, you want padding on the parent. It's the more common spacing property to use. Margin would come into play when adding gaps between siblings, like between the image and the heading, and the heading and the paragraph text. This can be achieved with a margin-top and margin-bottom on the heading

Marked as helpful

Ahmed Bayoumi• 6,800

@Bayoumi-dev

Posted almost 2 years ago

Hey Ranjana,

My suggestions:

Document should have one main landmark, Contain the component with <main>.

<main>
  <div class="white-card">
      //...
   </div>
</main>

Page should contain a level-one heading, Change h2 to h1 You should always have one h1 per page of the document... in this challenge, you will use h1 just to avoid the accessibility issue that appears in the challenge report... but don't use h1 on small components <h1> should represent the main heading for the whole page, and for the best practice use only one <h1> per page.
I suggest you center the component on the page with Flexbox, by giving the parent element <main> the following properties:

body {
    background-color:var(--Light-gray);

    /* The scrollbar will appear because `<body>` has a default `margin` ---- Remove `margin`*/
    margin: 0;
}
main {    /* <---- Add */
   display: flex;
   justify-content: center;
   align-items: center;
   min-height: 100vh;
 }  
.white-card{
    /* margin-left: auto;      <---Remove */
    /* margin-right: auto;      <---Remove */
    /* margin-top: 150px;      <---Remove */
    //...
}

Hope this is helpful to you... Keep coding👍

Marked as helpful

Please log in to post a comment

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