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

Using SASS as pre-processor

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


Margin is difficult, but I try my best to learn it...

Community feedback

Elaine 11,400

@elaineleung

Posted

Hi Obhasa, great job at attempting this challenge! As mentioned by @PPechmann above, you can try centering the component with flexbox using his method, and if you do that, you can remove the margins you have for the container.

Some other comments I have:

  1. Right now the layout changes to desktop view with a lot of space around the component; I'd try making the breakpoint smaller so that there won't be so much space.

  2. Your component isn't so responsive right now, unforunately! That means it doesn't really resize or "respond" when the browser width changes, and the width is fairly fixed. I usually would say, just change the width to max-width, which is a fairly easy solution. ln your case, because you have the images as background images, instead of using max-width, you can try width: min(100%, 37.6875rem) instead. The better solution, however, is just to change the background images to the picture element, since this is a product image and is central to the content, therefore you'd want the screen reader to pick up on the image (which it can't if you're using a background image). To do that, try this in your code:

    <picture>
       <source media="(min-width: 64em)" srcset="image-desktop.png" />
       <img src="image-mobile.png" alt="Glass perfume bottle placed against a leafy background" />   
    </picture>
    
  3. To make the image width more or less equal with the text, try adding flex: 1 0 50% to the item__image.

Hope some of this can help you out!

Marked as helpful

3

@obhasa12

Posted

@elaineleung Hallo Elaine, thanks for the feedback, you give me an improvement to become better in the future. now I starting considered about screen reader more. I'll try my best in the future...

1
Patrick 800

@PPechmann

Posted

Hi @obhasa12,

congrats on the challenge!

I really like your approach and the code looks pretty clean too, well done 💪

As a small improvement, I suggest centering the card properly with flexbox. For it to work, you need to give the body a min-height: 100vh, to cover the full viewport, and then simply the following (also on the body):

display: flex;
align-items: center;
justify-content: center;

Hope this is helpful and I am looking forward to more completed challenges 😉

Happy coding!

Patrick

Marked as helpful

1

@obhasa12

Posted

@PPechmann, thank u @PPechmann, that's very helping me to solve the problem. maybe I still dont get used to for "viewport height", even I have watched so many video but that's very helpful.

Happy coding 😉

Obhasa

1
Chris 1,030

@BlackSun225

Posted

Good debut, with practice you will be better. Congratulations

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