QR Code image using CSS display Flex

Community feedback

• 

  Marcos Travaglini•4,840

  @BlackpachamamePosted 10 months ago

  Good job!

  Here are some comments that may help you:

  • Use semantic tags such as main, footer, etc, this helps the accessibility and SEO of the site
  • The <div class="main"> should be <main class="main">
  • The <div class="attribution"> should be <footer class="attribution">. Place the footer outside the main
  • With these changes, the current main is no longer necessary
  • To center the elements in the center of the screen you can make the following adjustments in the body:

  body {
      font-family: 'Outfit', sans-serif;
      background-color: hsl(212, 45%, 89%);
      min-height: 100vh;
      display: flex;
      align-items: center;
      justify-content: center;
      flex-direction: column; /* Place one element below the other */
      gap: 20px;
      font-size: 15px;
  }
  
  .container {
      max-width: 360px;
      /* margin: 0 auto;  You no longer need to add margin to your main*/
  }
  

  • Apply display: block to the image to remove that annoying white space
  • To center the content you can also use display: grid + place-content: center + min-height: 100vh. Or you can use the margin: 0 auto, but the best and easiest way is with flex or grid
  • The align-items: center, centers the content on the vertical axis, to center it horizontally, you should use justify-content: center. All this with flex-direction: row. If you had a flex-direction: column, it would be the opposite.

  2

  ---

  

  DEVADE/•80

  @AdeMEDIAPosted 10 months ago

  Thanks man @Blackpachamame

  0
• 

  Melvin Aguilar 🧑🏻‍💻•61,140

  @MelvinAguilarPosted 10 months ago

  Hello there 👋. Good job on completing the challenge !

  I have some suggestions about your code that might interest you.

  • Use semantic elements such as <main> and <footer> to improve accessibility and organization of your page.

  • You can use the following styles to center the element effectively without using flexbox

    For Grid:

    body {
      min-height: 100vh;
      display: grid;
      place-items: center;
      /* Additional styles if needed */
    }
    

    There is another way to center using position: absolute, but in this type of page, it can cause unexpected behavior on mobile devices. You can read more about it in this link: The Complete Guide to Centering in CSS.

  I hope you find it useful! 😄

  Happy coding!

  1

  ---

  

  DEVADE/•80

  @AdeMEDIAPosted 10 months ago

  Thanks for this mate @MelvinAguilar

  0
• 

  Daniel 🛸•44,250

  @danielmrz-devPosted 10 months ago

  Hello @AdeMEDIA!

  Your solution looks great!

  I have a couple of suggestions for improvement:

  • For semantic reasons, and since that is the main title of the screen, you can replace the <h2> with <h1>.

  The <h1> to <h6> tags are used to define HTML headings. <h1> defines the most important heading. <h6> defines the least important heading. Only use one <h1> per page - this should represent the main heading/subject for the whole page. Also, do not skip heading levels - start with <h1>, then use <h2>, and so on.

  • Also, still about semantic HTML, you can replace your div.container with main.container.

  All these changes may have little or no visual impact but they make your HTML code more semantic and improve SEO optimization as well as the accessibility of your project.

  I hope it helps!

  Other than that, great job!

  0

  ---

  

  DEVADE/•80

  @AdeMEDIAPosted 10 months ago

  Thanks man @danielmrz-dev

  0