Skip to content
OverviewSolutions
27
Comments
11
símios enfermo
@FernandoChurros

All comments

  • símios enfermo•440
    @FernandoChurrosPosted almost 2 years ago

    Hi @Omrividal 👋, good job completing this challenge! 🎉

    I have some suggestions you might consider to improve your code:

    • The margin in your .card to center is unnecessary as it is flexbox aligning.

    • For lower resolutions, a margin: 2rem on the sides just so elements don't stick to the edges would be more visually pleasing.

    • HTML itself provides option to choose which image will appear depending on user resolution, have a look at documentation picture .

    Above all, the project is done well👏. I hope those tips will help you! 👍

    God job, and happy coding! 😁

    0
  • Aman•630
    @AmanGupta1703Submitted almost 2 years ago

    I would like to hear your feedback! 😊

    News Homepage

    1
    símios enfermo•440
    @FernandoChurrosPosted almost 2 years ago

    Hi @AmanGupta1703 👋, good job completing this challenge! 🎉

    I have some suggestions you might consider to improve your code:

    • When scrolling with the navigation bar open it just disappears, a position: fixed would be a better choice.

    • In tablet resolution (768px) the page does not behave the way you want, in this situation a more accessible approach would be to use flexbox.

    1
  • símios enfermo•440
    @FernandoChurrosPosted almost 2 years ago

    I have some suggestions you might consider to improve your code:

    • To center content, you can use flexbox on the parent of the element you want to align, using the display: flex and align-items: center (aligns the element on the transverse axis), sets a height: 100vh to always center in relation to the user's screen.
    Marked as helpful
    0
  • símios enfermo•440
    @FernandoChurrosPosted almost 2 years ago

    I have some suggestions you might consider to improve your code:

    • In the body could set a width: 100vw, so the card is always centered regardless of the user's screen resolution.

    • The use of the strong tag is to give 'great importance', including urgent things (like alerts), to apply a style it is recommended to use 'font-weight`, in CSS.

    • For styling, use classes for better code maintenance, if you need to change something in the HTML, your CSS will break, as it is tied to tags.

    0
  • símios enfermo•440
    @FernandoChurrosPosted almost 2 years ago

    I have some suggestions you might consider to improve your code:

    • center tag has been discontinued, for centralization I recommend using flexbox in the parent element of those who want to centralize, setting :
    height: 100vh;
width: 100vw;
display: flex;
justify-content: center;
align-items: center;
    • The value you specified in the font-weight property is invalid, have a look at documentation to know what values this property accepts.
    • The font-size must be specified in a unit of measurement (px, rem, em).
    0
  • símios enfermo•440
    @FernandoChurrosPosted almost 2 years ago

    I have some suggestions you might consider to improve your code:

    • A better way to center the content is to use the display: flex;, align-items: center; and justify-content: center; properties of the Flexbox in the body by setting a height(100vh) and width( 100vw).

    • Putting the background as img in the body is not semantically recommended, it's just a background and not a page content.

    1
  • símios enfermo•440
    @FernandoChurrosPosted about 2 years ago

    I have some suggestions you might consider to improve your code:

    • In some resolutions like the tablet, the page "breaks", the flex-box is not 100% predictable when it comes to responsiveness, it would be more interesting to use the grid template, even to avoid having to write 'divs' inside 'divs ', which makes the code less readable.

    • and to use different images for different resolutions you can use the html itself: MDN - picture

    Marked as helpful
    0
  • símios enfermo•440
    @FernandoChurrosPosted almost 3 years ago

    put the "max-width" property on the element

    Marked as helpful
    1
  • símios enfermo•440
    @FernandoChurrosPosted almost 3 years ago

    o "hover" não está funcionando.

    • Poderia colocar a "div" e a "img" dentro de outra "div", e colocando na "div" pai um position: relative, e na "div" filho um absolute, assim, a "div" filho iria respeitar o pai, e não precisaria setar top ou bottom.
    • A cor do bg em cima da img, você coloca ela no :hover.
    0
  • símios enfermo•440
    @FernandoChurrosPosted over 3 years ago

    I'm not very good with English so I'll be a google translator

    • it is advisable to use relative units of measure, such as 'rem' or 'em', to make the site more responsive, instead of failing the pixel
    • Your html is not semantic at all, it only has a div, the use of structural tags is missing
    0
  • símios enfermo•440
    @FernandoChurrosPosted over 3 years ago
    • Poderia diminuir um pouco os nomes das classes kkk;
    • na seção do "Why Us" aconselharia usar a tag ul, em vez daquele "br's";
    • Não precisa setar um margin "0 auto" na principal_secaoOne e na principal_divisão, poderia apenas usar um align-items na tag main pra centralizar horizontalmente, setar uma height de 100vh, pra ocupar toda a tela do navegador, e um justify-content pra alinhar verticalmente;
    • O h1 tá muito pequeno na minha opinião, o padrão do h1 é 2rem, poreria usar 1.5rem, que é o tamanho de um h2 se achar muito grande.
    • Uma alternativa de ficar dando margin toda hora pra espaçar algo, você pode usar o grid-gap com item com o display flex ou grid, ele dá um espaçamento entre todos os elementos filhos.
    0