Simplified solution

Community feedback

• 

  Ecem Gokdogan•9,380

  @ecemgoPosted over 1 year ago

  Some recommendations regarding your code that could be of interest to you.

  • If you want to make the card centered both horizontally and vertically, you'd better add flexbox and min-height: 100vh to the body
  • For the color of the screen, you can use the recommended color in the body

  body {
    background-color: hsl(212, 45%, 89%);
    display: flex;
    flex-direction: column;
    align-items: center;
    justify-content: center;
    min-height: 100vh;
  }
  

  • You don't need to use main and you can remove it. You'd better give min-height, background color, etc. to the body instead of to main

  /* 
  main {
    min-height: 100vh;
    background-color: hsl(0, 0%, 90%);
  } 
  */
  

  Hope I am helpful. :)

  Marked as helpful

  1

  ---

  

  Metodi Gitsev•10

  @mgitsevPosted over 1 year ago

  @ecemgo Using <main> is just a habit I picked up along the way. Thanks for the review.

  1
• 

  EmmanuelUrias•130

  @EmmanuelUriasPosted over 1 year ago

  Great job, some UI advice would be to make the large text a bit smaller to fit better and have better proportions within the card. After looking at your codebase some things I'd point out is that the h2 tag you use for the large text should an h1 then you would make it smaller with css. When you start building larger projects your class name/styling system would get verbose, I would recommend learning how to structure the elements and giving them specific classes like <main class='card-container'> then giving it all it's style properties in the css with the selector - .card-container{
  flex-direction: column; justify-content: center; align-items: center; } Other than that great job!!

  Marked as helpful

  1