qr-code-component-challenge (html,css)

Community feedback

• 

  brunokzz•40

  @brunokzzPosted over 1 year ago

  Hello! Welcome to Frontendmentor. Congrats on your very first project!! I hope you keep achieving new challenges. Here are some recommendations due to your accesibility reports:

  • "Document should have one main landmark" --> It's a really good practice (overall for screen readers) to use semantic HTML5 landmarks (like header, nav, main, etc). You could replace the div with the "main-box" class, you could set a main tag that includes all the content of the page.

  • "Page should contain a level-one heading" --> It's important that you add an h1 tag, mostly because screen reader users usually use keyboard shortcuts to navigate directly to the first h1, which represents the main content of the web page. If there is no h1, or if the h1 appears somewhere other than at the start of the main content, screen reader users will waste valuable time trying to understand your page.

  • "All page content should be contained by landmarks" --> In order for it to be well organized, it is recommended that you divide the page content between semantic HTML tags. You can find more information in the link next to the accesibility report.

  Finally, here are some recommendations about your code:

  • For centering the component you could set a display:flex to the body, including justify-content: center and align-items: center so that the card is centered horizontally and vertically.

  • The margin property allows you to establish cross-axes value and main-axes value separately. So, to center the qr code inside the card you could set a margin: 5px auto, since auto will center it horizontally.

  However, even with these little things that I'm sure will help you in the future, your desing is very well written and accomplished. Hope to see more of you, and keep coding!!

  Marked as helpful

  2

  ---

  

  Hadi homaei•190

  @FisherSHADOWPosted over 1 year ago

  @brunokzz thank you for help and tips, it was helpful a lot.<3

  0
• 

  Ecem Gokdogan•9,380

  @ecemgoPosted over 1 year ago

  Some recommendations regarding your code that could be of interest to you.

  CSS

  • If you want to center it correctly, you can add min-height: 100vh to the body:

  body{
      min-height: 100vh;
  }
  

  • after adding min-height, you can remove margin-top from the .main-box to center the card:

  .main-box {
      width: 300px;
      height: 450px;
      background-color: rgb(255, 255, 255);
      border-radius: 20px;
      align-items: center;
      /* margin-top: 200px; */ //you don't need it because of flexbox you used in 'body'
  }  
     
  

  Hope I am helpful. :)

  Marked as helpful

  0

  ---

  

  Hadi homaei•190

  @FisherSHADOWPosted over 1 year ago

  ty for tips it was helpful, <3 @ecemgo

  1

  

  Ecem Gokdogan•9,380

  @ecemgoPosted over 1 year ago

  @FisherSHADOW I'm happy to help!

  0