Mobile-first Social Link Profile

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted 9 months ago

  Here is some feedback

  • don't capitalise landmark or element names in html
  • IDs are not for styling! Use classes as style selectors. If you want to understand what the ID attribute is for I've written a detailed post about IDs in HTML
  • Please Indent your code consistently so it's easier to read and spot bugs. Your code editor can even do this formatting automatically for you with prettier.
  • you can remove all divs inside the card component. They are all unnecessary and bloating the html for no benefit.
  • Jessica's picture is meaningful content in my opinion so should have alt text.
  • you have invalid html in the list. Only list items can be direct children of lists. Move the links inside the list items.
  • the attribution should be in a footer outside of the main landmark.
  • always use a modern css reset at the start of the styles in every project. Andy Bell has a good one you can look up and use. Get into the habit of doing this straight away in every project you do.
  • Font size must never ever be in px
  • make sure you understand the difference between padding and margin. And never use large padding or margin to try and build a layout.
  • the background and border radius should be on the links not list items.
  • this challenge should not have a media query. And even if it did need one, you shouldn't set it at such a large size, and must define it in rem or em not px. Here is a post about using media queries well
  • the card must not have a width or height. It should only have a max width in rem.

  Marked as helpful

  1
• 

  Daniel 🛸•44,310

  @danielmrz-devPosted 9 months ago

  Fala @Depaulaeduardo!

  Seu projeto ficou ótimo!

  Notei que você utilizou padding pra centralizar o card. Usar padding não é a melhor opção pra centralizar elementos.

  📌 Existe uma forma melhor e mais eficiente de posicionar elementos no meio da página (tanto na horizontal quanto na vertical):

  • Aplique isso ao body (Pra funcionar corretamente, não use position ou margins):

  body {
      min-height: 100vh;
      display: flex; /*  também funciona com grid  */
      justify-content: center;
      align-items: center;
  }
  

  Espero que ajude!

  Fora isso, você fez um excelente trabalho!

  Marked as helpful

  0

  ---

  

  P

  Depaulaeduardo•90

  @DepaulaeduardoPosted 9 months ago

  @danielmrz-dev gostei muito da sua dica, para o desktop vi que resolveu meu problema, mas quando vou inpencionar modo mobile no navegar, vejo que o card não está centralizado no body. Alguma dica?

  1

  

  Daniel 🛸•44,310

  @danielmrz-devPosted 9 months ago

  @Depaulaeduardo

  Se você removeu o padding e adicionou o código acima ao body, o elemento centraliza automaticamente. Dei uma olhada aqui no devTools e fiz o teste removendo o padding e o card fica centralizado.

  O que eu acho que pode estar causando isso é o fato de você ter colocado o footer (a div.attribution) dentro do container principal. Assim, como ele faz parte do container, ele ocupa espaço lá dentro e empurra todo o restante do conteúdo pra cima, especialmente pelo fato de você ter adicionado um padding de 32px a ele no eixo vertical (32px em cima e mais 32px em baixo). Isso faz ele ocupar um espaço bem grande dentro do card.

  Tenta posicionar o footer fora do card e vê se melhora. Outra dica é mudar o background do card pra uma cor mais clara, pois ele está com a mesma cor do body, parecendo que é tudo uma coisa só.

  0

  

  P

  Depaulaeduardo•90

  @DepaulaeduardoPosted 9 months ago

  @danielmrz-dev entendi, agora faz sentindo.

  Última pergunta, no max-width, como substituir o auto pelo rem?

  0