search

MLH-Fellowship / march-2022-prep-portfolio

Portfolio for March 2022 Prep
https://prep-march-2022.netlify.app
MIT License
2 stars 16 forks source link

fix: 🐛 profile picture layout by using height: fit-content for container #15

Closed Minh-ctrl closed 2 years ago

Minh-ctrl commented 2 years ago

✅ Closes: #12

Please, go through these steps before you submit a PR.

netlify[bot] commented 2 years ago

✔️ Deploy Preview for prep-march-2022 ready!

🔨 Explore the source changes: fa8a9b38a0964264aa2d1f4d2628d1650a87471b

🔍 Inspect the deploy log: https://app.netlify.com/sites/prep-march-2022/deploys/622a1778fc4f18000bda8914

😎 Browse the preview: https://deploy-preview-15--prep-march-2022.netlify.app

Minh-ctrl commented 2 years ago

I've set the height of the .profile and .profile-picture to fit-content and also set a specific max-width and height of the images to be 350px which fixed the issue

image image image
Minh-ctrl commented 2 years ago

Hey @Minh-ctrl , this looks great 💯

I had one small thing to brainstorm and get your views from design standpoint:

  • In smaller screens, would it be better if we can make images' width smaller. Basically the image size can be made responsive. Reason: It can be better to have 2X2 sort of Images in the Smaller Screens as 4 Images vertically in one column might look a little bit long for smaller screens. What do you think?

Thank you!

Hey @sohamsshah!

Thanks for the feedback! I didn't think about this at all when I implemented the solution 😆 .

For the solution i think it's a simple media query implementation. At a certain width size we can adjust the image to a certain size so that the users would have less to scroll. I've set the media query to be around 400px Here's the code: @media only screen and (max-width: 400px) { img { width: 150px; height: 150px; } }

Thanks for your suggestion, i think this issue will really matter once we add all 20 of us onto the portfolio so it's good to catch it early 😄 .