Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Request path contains unescaped characters
Request path contains unescaped characters
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Frontend-Mentor-Preview-Card

EmrePWβ€’ 150

@EmrePW

Desktop design screenshot for the Product preview card component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


I have struggled with the images and I cant seem to fit them very well. Can anyone lend me a hand with that ?

Community feedback

Abdul Khaliq πŸš€β€’ 72,660

@0xabdulkhaliq

Posted

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

@Petrosdevri

Posted

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

@visualdenniss

Posted

@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

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord