First QR code challenge

Community feedback

• 

  Lucas 👾•104,340

  @correlucasPosted over 2 years ago

  👾 Hello Donn, congratulations for your solution!

  Your design elements are just perfect, but as you said you can have a better way to align this container to the center.

  I know that you've used height: 20vh and works, but a best practice in this case is use display: flex; and min-height: 100vh in the body to align the child element. The child align to the body with min-height: 100vh because the child element use its parents size to get the alignment, so the body has 100% of the viewport the child will ever align to the screen center. Hope this was a good explaination, but you can write me if dont.

  You can also improve even more your solution making your component responsive, you can not that if you scale down the window, the component will not behave responsive due its fixed width to fix it you need only to replace width with max-width inside the container and you'll not that the text elements will start to broke lines and the image scale properly.

  Hope it helps and feel free to ask me any question.

  Happy coding!

  Marked as helpful

  0

  ---

  

  Donn O'Lochlainn•20

  @donnol87Posted over 2 years ago

  @correlucas hi there. Thanks for your feedback. Quick question, which section do I need to move from a fixed width to a max-width? I had a look back over my code and it seems I used max-width from the beginning? Although, my container doesn't scale correctly so I might have gone wrong.

  0

  

  Lucas 👾•104,340

  @correlucasPosted over 2 years ago

  @donnol87 I've check your css file and I think is the image that's not allowing the container to be responsive due the width: auto;

  #image{ 
       height:250px; 
       width: auto; 
       border-radius: 20px;
  

  Use instead max-width: 100%; to make the image grow 100% the container, remove the heightand setdisplay:block ` to the img for a better scaling.

  0
• 

  BADHRI KESAVA RAJA S M•515

  @BadhrikrPosted over 2 years ago

  Hello your solution is superb,

  My only suggestion is ,Apply these to your stylesheet

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

  Remove the margin top on the box1

  Use main tag as your box1 to remove accesibility issues

  Happy coding.

  Marked as helpful

  0

  ---

  

  Donn O'Lochlainn•20

  @donnol87Posted over 2 years ago

  @Badhrikr thank you SpiderMan!!

  1