product card

Community feedback

• 

  Adriano•38,040

  @AdrianoEscarabotePosted about 2 years ago

  Hi Kelvin Boateng, how are you?

  I really liked the result of your project, but I have some tips that I think you will like:

  1- Document should have one main landmark, you could have put all the content inside the main tag [click here to read about it](https://dequeuniversity.com/rules/axe/4.3/landmark-one-main ?application=axeAPI)

  2- I noticed that the div was not so centralized I made some changes in the code, to solve this:

  body { min-height: 100vh; align-items: center; display: flex; }

  The rest is really good! Hope it helps... 👍

  Marked as helpful

  0

  ---

  

  Kelvin Boateng•90

  @Kellycypher98Posted about 2 years ago

  @AdrianoEscarabote thanks a lot my guy

  0
• 

  Kelvin Boateng•90

  @Kellycypher98Posted about 2 years ago

  thanks a lot my guy

  1
• 

  David•8,000

  @DavidMorgadePosted about 2 years ago

  Hello Kelvin, great job finishing the challenge! congratulations.

  I would like to suggest you some changes on your code.

  If you want to get your component centered, try using flex-box on the parent element, instead of using margin on your container class, to archieve this you just have to remove that margin and add to the body:

  body {
    background-color: hsl(30, 38%, 92%);
    display: flex;
    justify-content: center;
    align-items: center;
    min-height: 100vh;
  }
  

  With this, your body takes full screen thanks to min-height: 100vh and with the rest of flex-box you center the child element wich is the container.

  Well now that we got your card centered, lets try to see what's happening with that image!

  Try giving some relative dimenstions to your container with rem, maybe width: 35rem and height: 25rem should do the work!. Also set each flex item in the container to take 50% of the space of it!

  Hope my feedback helps you, if you need any help or you don't understand something of this, I can also help you with a pull request on GitHub, just ask!

  0

  ---

  

  Kelvin Boateng•90

  @Kellycypher98Posted about 2 years ago

  @DavidMorgade thanks. i made changes with respect to your suggestions and it worked

  1