made with HTML, Sass and JS

Community feedback

• 

  Marlon Passos•940

  @MarlonPassos-gitPosted over 3 years ago

  • you could have put the logo in a <header> tag

  <header class="logoContainer">
        <img src="images/logo.svg" alt="company logo" />
  </header>
  

  • In my opinion there are unnecessary <div> tags, for example the image could be:

  <picture class="heroContainer">
          <source media="(max-width:900px)" srcset="images/hero-mobile.jpg">
          <source media="(min-width:900px)" srcset="images/hero-desktop.jpg">
          <img class="imagen" src="images/hero-mobile.jpg" alt="hero-image">
   </picture>
  

  too many elements are wrapped by a div unnecessarily

  -I could change the size of the image so that it occupies the space better in a proportional way, putting it always with 50% of the page runs away even from the original designer

  -the button :hover animation was too outrageous, try something smoother and a little slower

  -on larger screens the background is lost, it would be nice if the backgroud were all over the screen

  -in your CSS code you could use variables for font colors and sizes, so it would be much better to maintain

  Marked as helpful

  0

  ---

  

  Francisco•145

  @FrankiiizePosted over 3 years ago

  @MarlonPassos-git hello thx i'll inprove those tips =), i forgot the header tag =( i will use it!, in the picture tag i was trying to use diferents sources acording to the windows width to get the img jajaja but that was something i whant to try for me, i guess its better change the img with css on the responsive, i will put some transiction to the :hover your rigth, can you show me how on large screem the background lost ?, i use variables to colors, but i made the css on sass i added the variables there

  0

  

  Marlon Passos•940

  @MarlonPassos-gitPosted over 3 years ago

  @Frankiiize I ended up forgetting about SASS :) jajaja.

  This print was taken on a full HD monitor https://prnt.sc/18eofok

  I think if I put a backgroud in this part of the same color as the SVG it would be cool

  0

  

  Francisco•145

  @FrankiiizePosted over 3 years ago

  @ MarlonPassos-git haha yeah sass make you live easier XD, ahmms I get it, I'll try to fix that, that's because I put a maximum width of 1440 in the grid. but it looks kinda ugly ... working on it!

  1