Responsive Product Card

Community feedback

• 

  P

  Martín Beltrán•260

  @JMBeltranDevPosted 7 months ago

  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•200

  @MattJM1007Posted 7 months ago

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

  0
• 

  P

  Koda👹•3,830

  @kodan96Posted 7 months ago

  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•200

  @MattJM1007Posted 7 months ago

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

  1

  

  P

  Matthew•200

  @MattJM1007Posted 7 months ago

  @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,830

  @kodan96Posted 7 months ago

  @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•200

  @MattJM1007Posted 7 months ago

  @kodan96 makes sense. Appreciate all the feedback!

  0
• 

  P

  Koda👹•3,830

  @kodan96Posted 7 months ago

  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

  @raficode2303Posted 7 months ago

  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•200

  @MattJM1007Posted 7 months ago

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

  1