Frontend-Mentor-Preview-Card

Community feedback

• 

  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.

  ---

  iMAGES 📸:

  • You can use the <picture> tag instead of setting images to background when you have different versions of the same image. The <picture> HTML element contains zero or more <source> elements and one <img> element to offer alternative versions of an image for different display/device scenarios.

  ---

  • Using the <picture> tag will help you to load the correct image for the user's device saving bandwidth and improving performance. You can read more about this here.

  Example :

  <picture>
     <source media="(max-width: 500px)" srcset="./images/image-product-mobile.jpg">
     <img src="./images/image-product-desktop.jpg" alt="alt text goes here">
   </picture>
  

  ---

  .

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

  Happy coding!

  Marked as helpful

  0
• 

  Petros Devrikis•380

  @PetrosdevriPosted over 1 year ago

  Hey there, nice work and congrats for completing the project!

  A few notes on your solution:

  • Try changing the width of your main element to a smaller value in order to ensure that both images and text display in harmony. By setting it to auto you basically enable the browser to render it with issues (for example the image won't appear well if you zoom to 100%). I personally chose to wrap these elements inside a .product-preview div after the main element and set a width of 550px and a height of 400px, which I of course changed for smaller devices by reducing width and increasing height respectively.

  In conclusion, your project seems to be decent and by working on various details and practicing frequently you will be able to achieve even better results.

  Marked as helpful

  0

  ---

  

  P

  visualdennis•8,375

  @visualdennissPosted over 1 year ago

  @Petrosdevri You should almost never set fixed heights or max-heights, especially when it is text containing element. Data might change, resulting in content overflow. User might change base font-size, resulting text overflow. And so on, in short, it can cause many issues, best is simply let the content determine the height of the container, tweak it with paddings/margins instead. At most use a min-height when needed. Also try to avoid px, instead use responsive units like rem/em.

  In this particular challenge, only a max-width for the container is needed, and the children (left side with img and right side with text container) can be adjusted with width: 50%, img does not need any height here, only width: 100% display: block is enough. Rest will take care of itself automatically

  Marked as helpful

  0