Preview card

Community feedback

• 

  P

  beowulf1958•1,450

  @beowulf1958Posted 3 months ago

  Great job on completing this challenge! Your desktop design looks great, your css is very organized and readable, and you made good use of css variables and flexbox.

  However, the mobile design needs a little tweak. The heart of this challenge is to switch the image from image-product-desktop.jpg to image-product-mobile.jpg as the screen size changes. This is an important skill, as many of the challenges require switching images with screen size. Reviewing other peoples' solutions, there are several ways of switching images. The simplest way may be using <picture> element with the "srcset" attribute. When reviewing your project, I simply removed the image and replaced it with a <picture> :

      <article class="container">
        <picture>
          <source
            media="(min-width:600px)"
            srcset="/images/image-product-desktop.jpg"
          />
          <img
            src="/images/image-product-mobile.jpg"
            alt="product-img"
            class="perfumeImg"
          />
        </picture>
  

  When I did that (and removed an unnecessary max-height: 80% from the container) it looked beautiful. A helpful article here explains how to use the picture element.

  Hope this helps. And keep on coding!

  Marked as helpful

  3

  ---

  

  Dei•60

  @deidalopezPosted 3 months ago

  @beowulf1958 Thank you so much for pointing out the fact that I almost missed out on a learning lesson by not updating the image source depending on the screen size. Also thanks for the resources and your feedback!

  0
• 

  ricardoychino•190

  @ricardoychinoPosted 3 months ago

  Hi,

  Nice job with the solution

  Regarding the image, let me explain: you added the object-fit: cover correctly. It indeed makes the image fit within the container. But it is not being applied. This is because when using object-fit, the dimensions available must be clear and the overflow: hidden in the image is messing a little with the sizing of it. Also, the width should be 50% since it is supposed to have the half of the parent .container:

  .perfumeImg {
    width: 50%;
    object-fit: cover
  }
  

  That's enough to define the sizing of the image as half of the card, fitting inside that space.

  Now, as for having the same corners, the thing is that there's nothing wrong. It is fitting the space available, which is a rectangle without rounded corners! The rounded corner exists only on the parent, which is a completely different and independent element. The corner is visually altered, but it is there and the img element corner is matching the same position. So technically, it is correctly fitting the space available.

  So there are 2 options:

  1. Set the border on the image (as you did)
  2. Apply overflow: hidden on parent element, since they are the ones that have the rounded corner

  .container {
    border-radius: 1rem;
    overflow: hidden;  <-- add this
    ...
  }
  

  But keep in mind that sometimes the approach 1 will be better (if for some kind of design choice you NEED elements that get out and be visible outside parent div), and in others the 2 will have a best fit. In case of this solution, I'd go with the approach 2 so that we don't have to repeat code.

  Hope this helps.

  Marked as helpful

  1

  ---

  

  Dei•60

  @deidalopezPosted 3 months ago

  @ricardoychino This was super helpful, thank you! I set overflow:hidden and was able to remove redundant code!

  1