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

Perfumes Preview Card

@WiaterKa

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


Had problems with a proper positioning of the image (please see the bottom line).

Any feedback concerning the above (and other aspects) hugely appreciated :))

Community feedback

@kanuos

Posted

Hi Kamil, congrats on your submission. Your code looks pretty decent to be honest but like they say - there's always room for improvement.

Here's a few tips on how to improve your code

  1. Your code is missing the landmark wrapper. Easy fix - convert the div.container to main.container
  2. Looking at your class names, try to follow a standard - eg. BEM. Look up BEM naming convention. That'll help you isolate the UI into minute re-usable components - hence leading to DRY-er code.
  3. Always use an alt attribute to your img element.
  4. Try to make the img styling more generic. The starter kit provided by FEM is kind enough to resize and optimize the images. What if the images were out of shape/aspect ratio? Use object-fit object-position etc props on img elements. Setting the display to block helps prevent a lot of unnecessary bugs.
  5. If you look at the design, you'll see the font weight of the heading element is not optimal in your code. Fix it when you can. While at it, fix the font size of the card description too.
  6. You probably missed out the hover effect on the button. As a pointer, I'd recommend using a <a> tag instead of the button element. Also, decrease the border-radius of the button to make it look identical to the design's button.
  7. If you look at the desktop design, the image on the left and the card info on the right are of the same width. In your solution, the image looks a bit thinner compared to the card info. Also, the padding on the text content is a bit more in the design.
  8. Try using relative units like rem, %, em etc instead of absolute units like px.

Hope this helps. Hope to see a lot more from you in the future. Happy coding :)

Marked as helpful

1

@WiaterKa

Posted

@kanuos your feedback is impressing! I partially managed to introduce those changes to the project. I will bear in mind your helpful tipis for the future projects :))

I really appreciate your time and effort - it helped a lot! Hope to see you around here in the future.

Have a nice day and happy coding!

1
Anna P. M. 390

@annapmarin

Posted

Good job! Maybe you can erase the text-align: justify to fit the original design. Also, regarding the image, you can heighten the image: height: 455px; to fit in better, or you can try adding new properties to the div you named as "img_container".

Also (but this is less important) you can add more right margin to the <h1> and more line-height and margin-right to the product description <p> to fit more the original design.

Marked as helpful

1

@digigrrl525

Posted

It's looking good. I don't know that I would have used an SVG for the button, but that is a personal choice. Also, you have "Add do Cart" instead of "Add to Cart" as your button CTA.

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