Product preview card component

Community feedback

• 

  Atif Iqbal•3,200

  @atif-devPosted almost 2 years ago

  Hi Muhammad Ehtisham. You did well. You need to work on following points:

  User-interface:

  1.Fix the position of crossed text and crossed line.(Use margin-top property)

  2.Fix alignment of button text(text should be in one line.)

  3.Fix container position(container should be in center), for centering use:

  body {
      min-height: 100vh;
      display: grid;
      place-content: center;
  }
  

  or learn centering from https://moderncss.dev/complete-guide-to-centering-in-css/

  Code:

  use main tag and wrap everything inside it to overcome frontendmentor accessibility report issue "All page content should be contained by landmarks". And try to overcome issues generated by frontendmentor Report Generator.

  Hope you find this Feedback Helpful.😇

  0
• 

  John Denver•290

  @jbidzPosted almost 2 years ago

  Hi there

  I noticed that you're using absolute height to your component which causes the add to cart button to overflow. You can omit the height property and let the content dictate it's height.

  Also, to make your component adapt to any screen sizes wrap it inside a div and add necessary styles to center it:

  div {
       max-width: 600px;
       margin: 0 auto;
       display: flex;
       flex-direction: column;
       align-item: center;
       justify-content: center;
  }
  

  That's it. Congratulations on making a step on this challenge. If you have any questions feel free to reply on this comment, and if you think this helps you a little please consider to marked this comment as helpful . Happy coding!!!

  0
• 

  Hassia Issah•50,650

  @HassiaiPosted almost 2 years ago

  Replace <div class="container"> with the main tag to fix the accessibility issue. Remove <div class="icon"> and <div class="icon"> to fix the error issue, a div cannot be a child element to a button. for more on semantic html visit https://web.dev/learn/html/semantic-html/

  Give .container / main a height value of auto to stop to overflowing of the button.

  To center .container on the page, add min-height:100vh; display: flex; align-items: center: justify-content: center; to the body. instead of giving .container position: absolute and it properties.

  Give .image-section a width of 50% and .text-section a width of 50% instead on giving them a width and height value. Give the img a width or max-width of 100% instead of a fixed width and height value.

  Hope am helpful HAPPY CODING

  0