Land page criada com GRID e MediaQuery

Community feedback

• 

  P

  Øystein Håberg•11,030

  @Islandstone89Posted 11 months ago

  Hi, well done. Here is some feedback:

  HTML:

  • Change .container from a <div> to a <main>, and remove .grid-container - the <main> must wrap all of the content (except for <header> and <footer>), hence it cannot have a <div> outside of itself. And since the card is the only main content, you don't need to wrap the card in a <div>.

  • .attribution should be a <footer>, and its text must be wrapped in a <p>.

  CSS:

  • It's good practice to include a CSS Reset at the top.

  • You have declared display: grid twice.

  • The card should not have a width but it does need a max-width of around 20rem, so it doesn't get too wide on larger screens.

  • Since all text should be centered, you only need to set text-align: center on the body, and remove it elsewhere. The children will inherit the value.

  • As the design doesn't change, there is no need for any media queries. However, well done for doing mobile styling as the default, and having media queries in rem.

  Marked as helpful

  1

  ---

  

  Elson Martins•150

  @ElsonMartinsPosted 11 months ago

  @Islandstone89

  Thank you very much, I appreciate the feedback, I managed to improve a lot with your help, I will pay more attention to good practice issues, these are my first codes, I am working hard on this aspect but I think I made a mistake. Regarding the guidance, I will pay more attention when reviewing the code so that there is no unnecessary code. Regarding the issue of code inheritance, thank you very much too

  1
• 

  Daniel 🛸•44,430

  @danielmrz-devPosted 11 months ago

  Fala Elson!

  Mais um excelente projeto, parabéns!

  Tenho uma sugestão pra esse também:

  • Ao invés de usar uma div e também a tag main, você poderia ter usado somente a tag main como container. Dei uma olhada no código e, a não ser que você tenha alguma razão específica pra usar os dois, você poderia simplesmente remover a div e utilizar apenas main.

  Isso te ajuda a reduzir linhas desnecessárias de código 😊

  Espero que ajude!

  Fora isso, belo trabalho!

  Marked as helpful

  0

  ---

  

  Elson Martins•150

  @ElsonMartinsPosted 11 months ago

  @danielmrz-dev

  Muito obrigado Daniel eu estou no começo mas tenho algumas dificuldade mas eu estou progredindo com os feedback do pessoal da comunidade muito obrigado.

  1

  

  Daniel 🛸•44,430

  @danielmrz-devPosted 11 months ago

  @ElsonMartins

  É isso aí, Elson!

  To acompanhando teus projetos e vou ajudar sempre que puder 😁

  Marked as helpful

  0
• 

  P

  Dilhan Boca•220

  @dboca93Posted 11 months ago

  Hello @ElsonMartins,

  Congratulations on submitting this task. I sincerely hope you're enjoying your coding experience. Looking at the design file, the central container needs to be centered both horizontally and vertically in the middle of the page. I found some code you wrote which is preventing this from happening, which is:

  @media (min-width: 22rem)
  body {
      min-height: 0; 
      padding-top: 5rem; 
  }
  

  If you remove this code, the other code you placed on the body, which is much better, will be able to shine through:

  body {
      font-family: 'Outfit', sans-serif;
      background-color: var(--light-gray);
      min-height: 100vh;
      display: grid;
      place-content: center;
  }
  

  I wish you all the best for the future. Please mark this comment as helpful if you can ! Please feel free to add me on Twitter if you like : @dboca93

  Marked as helpful

  0