Product-Preview-Card-Component

Community feedback

• 

  Elaine•11,400

  @elaineleungPosted about 2 years ago

  Hi Usman (or Dembele?), well done here, especially in using the shorthand flex property in the image and text, which not many people use as a solution. It's one I do recommend using for those who use flexbox because this is just one of the features of flexbox, and there's no need to set a width to try to make things even. So anyway, well done there!

  Two suggestions I have:

  1. I'm viewing this on a laptop, and in the mobile view, the top part is cut off. This is due to the flexbox properties on the body in mobile view, so I'd suggest you to remove the align-items: center for the body and also overflow: hidden as it prevents me from scrolling, and also change height to min-height: 100vh in the body. To have some spacing, you can add a bit of margin around the component as well, as right now, even with all these changes, the top of the card is touching the top.

  2. I think you can try a slightly bigger breakpoint (even just 20px or so higher) so that when the layout changes, there's some space around the component and so it wouldn't seem like the component is touching the sides of the browser.

  That's it, well done on the whole!

  Marked as helpful

  1

  ---

  

  Usman•350

  @awwsmanPosted about 2 years ago

  @elaineleung Good day Thanks alot for taking the time to go through my code Feedback like these makes me improve I hope to read more of your suggestions on my subsequent Project Submissions

  1
• 

  Lucas 👾•104,340

  @correlucasPosted about 2 years ago

  👾Hello Usman, Congratulations on completing this challenge!

  Your solution its almost done and I’ve some tips to help you to improve it:

  The easiest way to make the two columns have the same size is by using display: grid / max-width: 1000px and grid-template-column: 1fr 1fr (this makes each div have 50% of the total).

  You did a good work putting everything together in this challenge, something you can do to improve the image that needs to change between mobile and desktop is to use <picture> instead of <img> wrapped in a div. You can manage both images inside the <picture> tag and use the html to code to set when the images should change setting the device max-width depending of the device (phone / computer) Here’s a guide about how to use picture: https://www.w3schools.com/tags/tag_picture.asp

  ✌️ I hope this helps you and happy coding!

  Marked as helpful

  1

  ---

  

  Usman•350

  @awwsmanPosted about 2 years ago

  @correlucas Thank you so much for your going through my code I will surely put your tips into action in my next project.

  1