Responsive QR Code page

Community feedback

• 

  Martin Richter•160

  @Karag2006Posted over 1 year ago

  Also from me Gratulations to your first Project.

  I totally agree with the Centering above. Even if you did not learn about grid yet, just use those 3 lines and learn how it works for more complex stuff at your own time.

  I like that you used basicly only 4 html tags. and i tried to make the picture work without changing that or introducing any advanced things. And this should fix your picture:

  1. replace img { height: 50vh; border-radius: 10px; } with: img { width: 100%; border-radius: 10px; }
  2. remove the media query alltogether or if you want to use it change it so it does not make the component bigger then the screen is at that time. for example you could safely change it to @media screen and (min-width: 500px) .container { width: 400px; } The Challenge works perfectly fine without media query though.

  Thats it, now the picture stays where it should and you still only use 4 elements on your solution.

  0
• 

  Ecem Gokdogan•9,380

  @ecemgoPosted over 1 year ago

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

  I couldn't view your GitHub repository but I try to help you through the live site

  • If you want to make the card centered both horizontally and vertically, you'd better add flexbox and min-height: 100vh to the body

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

  • If you use flexbox in the body, you don't need to use margin in the .container to center the card
  • If you use max-width, the card will be responsive

  .container {
    /* width: 300px; */
    max-width: 300px;
    /* margin: 80px auto; */
  }
  

  • In addition to that above, in order to make the card responsive and the image positioned completely on the card, you'd better add width: 100% and height: 100% to the img

  img {
    /* height: 50vh; */
    width: 100%;
    height: 100%;
  }
  

  Hope I am helpful. :)

  0
• 

  Abdul Khaliq 🚀•72,560

  @0xabdulkhaliqPosted over 1 year ago

  Hello there 👋. Congratulations on successfully completing the challenge! 🎉

  • I have other recommendations regarding your code that I believe will be of great interest to you.

  ---

  CSS 🎨:

  • Looks like the component has not been centered properly. So let me explain, How you can easily center the component without using margin or padding.

  ---

  • We don't need to use margin and padding to center the component both horizontally & vertically. Because using margin or padding will not dynamical centers our component at all states

  ---

  • To properly center the component in the page, you should use Flexbox or Grid layout. You can read more about centering in CSS here 📚.

  ---

  • For this demonstration we use css Grid to center the component.

  body {
      min-height: 100vh;
      display: grid;
      place-items: center;
  }
  

  • Now remove these styles, after removing you can able to see the changes

  .container {
    margin: 80px auto;
  }
  

  ---

  ---

  • Now your component has been properly centered

  ---

  .

  I hope you find this helpful 😄 Above all, the solution you submitted is great !

  Happy coding!

  0
• 

  Zubin Nair•190

  @znair96Posted over 1 year ago

  Hi i have some feedback on your UI -

  1. Your image is not aligned and overflowing try to adjust it with max width and add image inside a div and use object fit property to align the image 2.Try to make the heading little bolder as it looks lighter .
  2. I would suggest you to use px and rem instead of vh or vw as it changes based on the viewport so please adjust the dimensions in pixels or rem.
  3. Use media queries to align the dimensions according to the screens Apart from that everything is good . Please try to implement those and these are my feedback.

  If my feedback helped you please mark it as helpful.

  0