QR-Code Component with CSS Grid and media query

Community feedback

• 

  Adriano•42,820

  @AdrianoEscarabotePosted over 2 years ago

  Hello Mohsin, how are you? Welcome to the community of front-end mentors! I truly loved your project's outcome, however I have some advice that I hope you'll find useful:

  To align some content in the center of the screen, always prefer to use display: flex; it will make the layout more responsive!

  body {
      margin: 0;
      padding: 0;
      display: flex;
      align-items: center;
      flex-direction: column;
      justify-content: center;
      min-height: 100vh;
  }
  

  A document ought to have one primary landmark, and the absence of a main tag around the page's primary content is the root of this issue. On this page, there is no other element that is more important than the one that this challenge is based on, so to solve it, wrap all the content in the'main' tag.

  It's always a good idea to pay close attention to the proper use of semantic html elements because they are crucial for screen reader users to understand what the main content of the page is in the case of the'main' tag!

  The remainder is excellent.

  I hope it's useful. 👍

  Marked as helpful

  1

  ---

  

  Mohsin•60

  @Mohsin-93Posted over 2 years ago

  Thank you for your insightful feedback Adriano.

  1
• 

  tan•640

  @tan911Posted over 2 years ago

  Hello @Mohsin-93, maybe this will help and improve your code:

  • Wrap your content with "main" element or use a "role" attribute for your div with the value of "main", so it should look like this: <div role="main" class="card"></div>.I suggest you should use main element as sematic html for accessibility and best practices.
  • Use "h1" instead of "h2", you should not skip h1, therefore don't use h2 without h1 and same with h3-h6. - Do not use pixels for font sizes instead use rem for that, so that your audience can be able to see the text base on their preferred font sizes.

  Hope it helps, Thanks.

  Marked as helpful

  1