Skip to content
OverviewSolutions
14
Comments
4
Cris Augusto
@Cristhyam-Augusto

All comments

  • Larissa•10
    @larisnSubmitted almost 2 years ago

    Uma dúvida que passou pela minha cabeça foi na hora de centralizar a box, eu utilizei as propriedades top, left e transform para isso, mas talvez eu pudesse ter usado o display flex, mas não tenho certeza de quais propriedades eu usaria para isso.

    Product preview card, mobile first

    1
    Cris Augusto•160
    @Cristhyam-AugustoPosted almost 2 years ago

    Ola Larissa, tudo bem? Parabéns por concluir o desafio, a solução ficou realmente bem parecida. 🎉🎉 Se você me permite, gostaria de pontuar algumas melhorias para essa solução:

    • Ao inves de utilizar display block, absolute, relative e outros para posicionar os elementos da maneira que deseja, você pode utilizar display:flex e alinhar os elementos de forma flexivel dentro de uma box que vai se ajustar dependendo do tamanho da tela.

    More Info for FlexBox

    • Ademais, notei que você utiliza margem para gerar espaço entre os elementos, você pode substituir esse tipo e atividade pela propriedade gap, juntamente com o display flex.

    More info for Gap

    • Para centralizar o seu container principal no meio da tela, você pode utilizar as seguintes propriedades:
    body {
    display: flex;
    align-items: center;
    justify-content: center;
    height: 100vh;
}
    • Por fim, caso você se interesse em aprender mais sobre padrões de projeto no css, recomendo a leitura sobre ITCSS, SMACSS ou/e BEM.

    Parabéns novamente pela solução, qualquer dúvida pode me chamar. Caso meus comentários tenham sido útels, não esqueça de marcar como útil. Happy Coding! 😊

    0
  • rockdoginafog•710
    @rockdoginafogSubmitted almost 2 years ago

    I learned a lot by code review. I'm hungry of knowledge and I want more of you guys. If I become better in coding I will do code reviews on this website too, but now I don't feel very confident to do it. Also I don't want to lead someone astray.

    So guys, keep up the good work, and let me know what do you think of this layout!

    Social Proof Section (slightly different)

    1
    Cris Augusto•160
    @Cristhyam-AugustoPosted almost 2 years ago

    Congratulations for the work and for the interest in modifying the layout to make it more beautiful. 👏👏

    Keep coding, happy coding. 💪

    1
  • Cris Augusto•160
    @Cristhyam-AugustoPosted almost 2 years ago

    Hello @JcCaldas, how are you? Congratulations for the work, it turned out very well. I noticed that you used a javascript function to change the color of items when hovering the mouse (mouseover), however, you can do this with just css

    .item:hover {
  background color: hsl(25, 97%, 53%);
  color: hsl(0, 0%, 100%);
}

    The same goes for the submit button. I hope the feedback was helpful, please don't hesitate to MARKED AS HELPFUL, thanks. Happing coding.

    Marked as helpful
    1
  • Cris Augusto•160
    @Cristhyam-AugustoPosted almost 2 years ago

    Hi Furka how are you?? Congratulations on the design, it was really identical. I did a project like this yesterday, and I got some feedback that might be useful for you too: I noticed that I can send a review even without filling in the value, so you can change the code as below to avoid this.

      rate.addEventListener("click", () => {
    rate.style.backgroundColor = "grey";
    rate.style.color = "white";
    rating.innerHTML = rate.innerHTML;
    submitButton.addEventListener("click", () => {
      mainContainer.style.display = "none";
      thanksContainer.style.display = "flex";
    });
  });
});

    I also noticed that in the html you used ID and class references for the LI, but I don't think that's necessary, because you used only classes in css. In general I think that's it, congratulations on the work, it was really good.

    0