Product-preview-card

Community feedback

• 

  Dylan de Bruijn•3,230

  @DylandeBruijnPosted 4 months ago

  @Jackson-zablon15

  Hiya! 👋

  Congratulations on your solution, it looks very close to the design! I can tell you put a lot of effort into it.

  Things I like about your solution 🎉

  • Use of semantic HTML elements
  • Clear descriptive CSS classes
  • Use of CSS custom properties

  Things you could improve ✍️

  • I suggest adding a bit of padding to your body element so the card has some space around it on smaller viewports.

  • You could add a min-height: 100vh to your body element so it takes up the full height of the viewport while still being able to grow when the content inside it grows.

  • Try using using relative CSS units like rem and em they make your layout more adaptable.

  • Be careful with setting a fixed width and height on your elements. If the content in these elements grows beyond these restrictions you’ll run into overflow issues. Keeping the height at auto - which block elements are by default - will be fine in most cases.

  • Try styling your elements using classes instead of ID’s. Most of the time they are the better choice. ID’s are mostly used to select elements using JavaScript, navigation on the page itself and to style unique elements.

  • You don’t have to wrap your image in a separate div, it’s possible to style it directly to achieve the same result.

  • Try using the picture element and making your solution more responsive.

  I hope you find my feedback helpful! 🌟

  Let me know if you have more questions and I'll do my best to answer them. 🙋‍♂️

  Happy coding! 😎

  Marked as helpful

  0

  ---

  

  Jackson-zablon15•150

  @Jackson-zablon15Posted 4 months ago

  @DylandeBruijn Thank you so much for the help. I'm going to use your tips and i know i will learn alot from them.

  0
• 

  Rui-Martins23•190

  @Rui-Martins23Posted 4 months ago

  Hi @Jackson-zablon15, Not sure if this may help, but try giving on the @media ccs part, 50% width to both "image" and <section>. This way both parts will only ocupy half of the container <main>, and I think you can then use padding as were you were trying to, on the <section> part.

  Concerning the scroll bar, you can use the following on the body element: body ::-webkit-scrollbar { display: none; }

  Enjoy coding!

  Marked as helpful

  0

  ---

  

  Jackson-zablon15•150

  @Jackson-zablon15Posted 4 months ago

  Hi @Rui-Martins23, Thank you for your help especially that part of scroll bar, I'm happy to learn it. Enjoy coding too!

  0
• 

  imranhossainemi•245

  @imranhossainemiPosted 4 months ago

  Great work! Your use of css variable and build approach mobile first well-thought-out. Some minor mistake here and there in the work, but I think you will fix it as you learn in the future. Good luck.

  0

  ---

  

  Jackson-zablon15•150

  @Jackson-zablon15Posted 4 months ago

  @imranhossainemi Thank you so much

  0
• 

  Jason Limas•50

  @jasonlimasPosted 4 months ago

  Hey! Awesome work! It looks really good! However, I think you are having a minor problem with the desktop version. There's a little gap at the bottom of the picture. I had this problem too. I fixed it and the fix might work for you too.

  Here's how you can do it: by setting display: block on either your .image class or your img selector. This might be the solution to the little gap at the bottom of the image.

  Good luck for your future Frontend development journey! Hope you are having a lot of fun!

  0

  ---

  

  Jackson-zablon15•150

  @Jackson-zablon15Posted 4 months ago

  @jasonlimas Thank you for noticing that problem. I will try your solution and try to learn from it.

  0