Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
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

Responsive Product Card

P
Matthew 190

@MattJM1007

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


What are you most proud of, and what would you do differently next time?

Overall this was a fairly straight forward build.

What challenges did you encounter, and how did you overcome them?

I am still struggling with getting the spacing just right between elements with padding and margins. I feel like there should be an easier way to do it. I also find that the text doesn't look exactly like the design file, even though I put in the same font and properties that are in the design. I am not sure why this is.

What specific areas of your project would you like help with?

Can anyone provide tips on how to format/layout the text elements? Is there a better option than what I did? Also, how are my media queries? Anything I can improve there?

Community feedback

P

@JMBeltranDev

Posted

Note that to use different image sizes, you load them simultaneously, and in the media query, you use a display:none. This implies that if you open the page on a mobile device, the desktop-sized image is still loaded, which has an impact on performance. The following code is an example of how the image would be loaded depending on the screen size. <picture class="product__img"> <source media="(min-width:600px)" srcset="./images/image-product-desktop.jpg"> <img src="./images/image-product-mobile.jpg" alt="perfume imagen"> </picture>

Marked as helpful

2

P
Matthew 190

@MattJM1007

Posted

@JMBeltranDev thank you so much for the feedback. I added this to my updated code!

0
P
Koda👹 3,810

@kodan96

Posted

hi there! 👋👋

Instead of adding margin to all the child elements you can just add padding to the parent container, and that will line up your text nicely.

Also it's a good idea to set margins and paddings using relative units (em, rem, ch, etc), since usually their size depends on the element's size (headings for example usually have their own font-size as margins above and below them, unless they are extremely large)

And I wouldn't use hard-coded values (pixels) for setting width or height.Actually don't even set them. Parent elements will always include child element's content by default, it's better to let the child elements determine the height of the parent, hard-coding heights can cause overflow.

Hope this was helpful 🙏

Good luck and happy coding! 💪

Marked as helpful

2

P
Matthew 190

@MattJM1007

Posted

@kodan96 thanks! I will try applying this to my code. Appreciate the feedback

1
P
Matthew 190

@MattJM1007

Posted

@kodan96 I applied your feedback to my updated code and also switched to use grid to do the layout, and it was definitely a much easier time! I applied the padding to the parent container as you said and that worked out better. I did still end up using the min/max width for certain elements.

do you recommend using relative units for font size too? or are pixels okay?

1
P
Koda👹 3,810

@kodan96

Posted

@MattJM1007

You should avoid hard-coded values (pixels) most of the time. When you use these values you give up responsiveness(or you make it harder for yourself at least)..

Typically you will increase the font-size property with @media queries. If you have hard-coded values all over your CSS, you need to modify every element's font-size. On the other hand, if you use rem-s all you need to do is changing the font-size in your CSS :root selector and all your elements will have a new size based on that value.

I usually use em for padding and margin for text-based elements, since their margin usually based on their font-size, and again, when you change the font-size in :root these values will scale up as well without you touching them, making your job easier and your page maintainable.

If you are not familiar with the :root selector it's usually used to set custom properties that you can apply later. For example this is the :root selector for the challange I'm currently working on:

:root {
    --green: #4ee1a0;
    --black: #151515;
    --gray: #242424;
    --light-gray: #d9d9d9;
    --white: #ffffff;
    --light-font: 400;
    --bold-font: 700;
    font-size: clamp(18px, calc(3vw + 1vh), 20px);
}

Marked as helpful

1
P
Matthew 190

@MattJM1007

Posted

@kodan96 makes sense. Appreciate all the feedback!

0
P
Koda👹 3,810

@kodan96

Posted

Sorry, I actually lied (I mean, so-so). You can set min-width and/or max-width on your elements, so they won't shrink below or grow above a certain value. My bad 🙇‍♂️

2
rafi b 280

@raficode2303

Posted

as @kodan96 said, try to not use fixed sizes to solve ptoblem. it make the site look like an fixed image and not responsive for all devices, remeber css is responsive by default. by adding fixed sizes you make it un-responsive. you use codes like margin: 19px 19px 19px; in many tags, padding to the parent container is best practice as @kodan96 mentioned. using css flex/grid is more modern soultion. you will achive more with less code. keep to improve and keep to build 👷

1

P
Matthew 190

@MattJM1007

Posted

@raficode2303 appreciate the reply! I switched to grid for this project and applied the other feedback and I think it came out better!

1

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