Responsive Card using CSS Grid and Flexbox

Community feedback

• 

  Plinsinga•310

  @codedforfreePosted over 2 years ago

  Look pretty good indeed. Some things i would change, besides the points of @DavidMorgade

  First off all when i see your code on github. All html is above the opening body ;). Next to that i would try to turn the images in your card__header into a picture element with srcset and give then. both the same class.

  I would probably also get rid of the card__body div in HTML and in CSS. There is no need to add flexbox to get the content inside this div stacked. This is standard behaviour.

  But some thing are also just a personal preference ;) So keep up the good work and enjoy your projects!

  Marked as helpful

  1

  ---

  

  P

  Freddy Santana•190

  @CreixzPosted over 2 years ago

  @codedforfree Thank you my friend, I didn't notice that my html code was above body's tag, my mistake, but thank you for your time and your advice. I'll improve my code.

  0
• 

  David•8,000

  @DavidMorgadePosted over 2 years ago

  Hello Freddy congrats on finishing the challenge!

  Yes!, you did a pretty good job using the BEM methodology, but you need to take advantage of modifiers, in this project you have a pretty good example: the prize inside the card.

  Instead of defining two classes, card__actual-prize and card__previous-prize try to use a modifier here, for example, you could have card__prize and card__prize--discounted, this is a minor change but for larger projects it can help you a lot organizing your classes!

  Also don't forget to add a cursor: pointer to your add to cart button for a better user experience.

  Hope my feedback helped you!, good job!

  Marked as helpful

  1

  ---

  

  P

  Freddy Santana•190

  @CreixzPosted over 2 years ago

  @DavidMorgade Thanks for your advice, I will use what you recomment me for the next projects that I'm going to do.

  1