Qr code component main

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted about 2 years ago

  Hi Sef Palima,

  Congratulation on completing this challenge. Your solution looks great. I have some suggestions regarding your solution if you don’t mind:

  • You can use <main> for the card and <footer> for the attribution. HTML5 landmark elements are used to improve navigation.

  <main>
  </main>
  <footer>
  </footer>
  

  • In my opinion, the alternate text should indicate where the Qr code navigate the user : like QR code to frontend mentor.

  • Add min-height: 100vh instead of height: 100%to the body that let the body grows taller if the content outgrows the visible page instead.

  • width: 330px an explicit width is not a good way to have a responsive layout. Consider using max-width to the card in reminstead.

  • height: 500px It's not recommended to set height to component, let the content of the component define the height.

  Hopefully this feedback helps.

  Marked as helpful

  0

  ---

  

  Sef Palima•60

  @sef1210Posted about 2 years ago

  @PhoenixDev22 I'm having a hard time when I'm not setting the height but I will try next time :)

  Thank you so much for your feedback highly appreciated :)

  1
• 

  Baliram•260

  @0ME9APosted about 2 years ago

  Hi @sef1210 !!!

  Your design looks almost the same, and as a developer, I'm happy you did this. So now let's come to the point

  There are several ways to center an element(s).

  1. display: flex, justify-content: center, align-items: center --(inside parent div/element)
  2. position: absolute, left: 50%, top: 50%, transform-translate: (-50%, -50%) --(apply in that div which you want to center and important apply position: relative ---------in parent div)
  3. margin: auto --(Horizontal align)

  There are still more ways to center elements but I mostly use these css. #HappyCoding🧑‍💻

  Marked as helpful

  0

  ---

  

  Sef Palima•60

  @sef1210Posted about 2 years ago

  @0ME9A thus the margin: auto will not work on vertical align?

  thank you so much for your feedback highly appreciated :)

  0
• 

  David•8,000

  @DavidMorgadePosted about 2 years ago

  Hello Sef, congrats on finishing the solution to this challenge!

  I think that using flex-box in this case is one of the best options you can use, other option could be using display-grid and align-content: center and justify-content: center on your body, but it would get the same result!

  Great job, there are few people that gets to center this component on the first try!

  Hope my answer helped you!

  Marked as helpful

  0

  ---

  

  Sef Palima•60

  @sef1210Posted about 2 years ago

  @DavidMorgade thank you so much for your feedback highly appreciated :)

  0