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

Preview card

Dei 60

@deidalopez

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?

I like the structure of the project since I think its readable, and easy to follow. I am also glad to have found the CSS Reset from Josh Comeau https://www.joshwcomeau.com/css/custom-css-reset/.

However, I didnt spend too much time with the font scaling since it was readable regardless, so maybe in a future project I'll implement it.

I also struggled because I didnt have the Figma files, but I'm still proud about how close it got.

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

Working with images has been a challenge for me, so it was a lot of troubleshooting to get the image to fit correctly... I dont know how close I got, but it got to a better place than it was haha.

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

I guess if anyone has any resources for image styling?

For example, I wanted the image to fit within the container, hoping that I wouldnt need to manually set the border radius for the corners of the image since I set the border radius on the parent div.

Community feedback

P
beowulf1958 1,170

@beowulf1958

Posted

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

@deidalopez

Posted

@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

Posted

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

@deidalopez

Posted

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

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