Skip to content
Submitted 8 months ago

Product preview card component

WB52•150
@WB52
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Product preview card component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

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

I tried to get it as close to the design as I could, it's not 100% the same but fairly close I think...

Next time I'd probably spend more time trying figure out how the picture and source tags worked and use them in the code.

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

I found changing the images from mobile to desktop more challenging than I thought it would be. I found various ways this can be done, setting the images as a background images and changing with CSS but realised it's not really a good practice if the image is considered part of the content.

Also discovered the picture tag, source tag and the srcset attribute which can be used to select images based on screen width and more, but apparently not all browsers support this and also read about browser cache issues and not loading the alternative image/s when needed?

In the end I went for putting both images in the html and using display:none; on the desktop image when in mobile size and then toggling them when needed within the media query.

Seems to work but not sure if this was the best solution?

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

Any feedback, pointers and suggested improvements are welcome... Thanks.

Community feedback

  • Bryan Li•3,530
    @Zy8712Posted 8 months ago

    Hi there! Your site and code look excellent!

    If I really had to nitpick at one thing I'd change, it would be how you implemented the border radius.

    For your container you already did border-radius: 10px 10px 10px 10px;. Instead of adding border radius to the images, you could just add overflow: hidden to your #container. This will automatically hide the parts of the image that normall sticks out of the container.

    In summary:

    • #container already has border-radius: 10px 10px 10px 10px;, so just add overflow: hidden
    • with that you can get rid of border-radius: 0.625rem 0.625rem 0rem 0rem; and border-radius: 0.625rem 0rem 0rem 0.625rem; from your classes for the images

    Hope this helps 👍

    Marked as helpful
    0
    WB52•150
    @WB52Posted 8 months ago

    @Zy8712 Thanks for taking the time to look over my site and code. I haven't used overflow that much as yet but I totally understand what you mean. It makes sense and uses less code for the same result which is more efficient and saves needless code. I'm going to give it a try...

    I'm finding comments a great help as I wouldn't of thought about the overflow otherwise, much appreciated...

    0

Please log in to post a comment

Log in with GitHub

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