Product preview card component solution

Community feedback

• 

  Agata Liberska•3,995

  @AgataLiberskaPosted over 2 years ago

  Hi Eslam, I think it would be a good idea to review what html tags you're using here. It looks like you're using heading tags ust based on the styles you can see which isn't the best approach. Think about hierarchy here - it makes sense for the name of the perfume to be the main heading, but all other text could just be inside <p> tags or <span>s.

  I would also rethink your button. You've added hover styles to your green div which look nice but it would suggest to me that I can click this and something would happen - but only the text is an actual link. So Instead of using a div, you could just have an anchor tag and style it to look like a button:

  <a href="/"><img src="..."> Add to Cart</a>
  

  Make sure to take a look at the report above as well - the image urls should use / rather than \ and make sure to use landmark elements, <main> is definitely missing from here. I would also use <article> tag for the card.

  Also, for the perfume image I think adding a good alt text would also be very important, as this isn't a purely decorative image, it shows a product we're trying to sell.

  I also had a look at your css - I think in a simple project like this there is no need to combine your selectors, as you've only really got one thing with the class price or cart. Personally I think combining selectors really needs to be thought through as in a bigger project you may run into specificity conflicts and end up with a bunch of !importants which just isn't fun to work with :)

  Hope this help, happy coding!

  1

  ---

  

  Eslam Mohamed•110

  @eslammohamedtolbaPosted over 2 years ago

  @AgataLiberska First, I thank you for your help and I will rethink about your notes and I will modify it

  0