Product preview card with Html and CSS

Community feedback

• 

  asdiAdi•130

  @asdiAdiPosted almost 2 years ago

  Hi Valarie Oyieke, it seems that the p margins inside the .price class are the root of the problem.

  • First, add a .price > p selector and add margin: 0 property inside it
  • Then add a margin-top: 32px property inside the .price class
  • Finally, you can change the align-items property into center inside the .price class

  I hope this helps you and Good luck with future challenges!

  Marked as helpful

  1

  ---

  

  Valarie Oyieke•60

  @ValarieOyiekePosted almost 2 years ago

  Hello Adi, Thank you so much. It worked 😃

  1

  

  T

  Grace•31,090

  @grace-snowPosted almost 2 years ago

  @asdiAdi @ValarieOyieke I do not recommend this approach at all. I will add feedback on the challenge below

  0
• 

  T

  Grace•31,090

  @grace-snowPosted almost 2 years ago

  There are quite a few issues I'm seeing on this, and I'll try to go through methodically

  Html

  • use the picture element not two img tags
  • remove the section element, you are misusing it. A div is fine as all you're trying to do is encapsulate the text part of the card for layout purposes.
  • alt text on meaningful images (like the product image) must be an actual description of the image. Tell me what it looks like briefly as if I can't see it
  • alt text on decorative images (like the cart icon) should be left intentionally blank
  • perfume is not a heading. It is a paragraph. It is just ribbon text. You can wrap that paragraph and the heading after it both inside one hgroup element if you want.
  • well done for using the del element and for using a button element on this!

  Css

  • to center the card on the screen use flex or grid properties along with min-height 100vh
  • remove all that position absolute 50% stuff. That is not how you'd center a component with Content inside. You'd rarely use that technique at all in modern development. At the moment it is causing your card to be cut off on mobile. You have to understand what positioning absolutely does by removing content from the dom
  • use classes on what you want to style. I've advised you change some of the html on this. See how you now will have to change css as well. Separate those concerns by using class selectors in css
  • use a modern css reset at the start and that will remove the browser styles like margins for you
  • never use px for letter spacing. Use em so it scales with the font size
  • you can set border radius on the whole component (the card) with overflow hidden so the corners of the img gets cropped to the radius. Then no need to set individual corner radius on images at each breakpoint
  • I don't see any issue with price alignment
  • the button can be flex or inline flex and also use align-items and gap properties instead of margin
  • the del element is not actually announced to screenreaders or other assistive technology, so you either need to add accessibly hidden text in spans on the html or in pseudo element content. This post explains how to add content to del

  Marked as helpful

  0

  ---

  

  Valarie Oyieke•60

  @ValarieOyiekePosted almost 2 years ago

  @grace-snow Thank you so much. Let me work on everything

  0
• 

  Brijen Makwana•160

  @BrijenMakwanaPosted almost 2 years ago

  Hey Valarie Oyieke, very nice. One suggestion from me is that to centre the .wrapper class, you can apply following styling.

  margin-left:auto;
  margin-right:auto;
  

  Good luck for the next challenges.

  0

  ---

  

  Valarie Oyieke•60

  @ValarieOyiekePosted almost 2 years ago

  @BrijenMakwana Thank you !

  0