JS AND SASS

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted over 3 years ago

  Hey, great work on this one. Though I had to zoom out more since I cannot see the top part of the layout which is not scrollable, the mobile state on the other hand is great.

  Suggestion/s would be that:

  • Please refrain from using height: 100vh, this will only limit your layout based on the remaining viewport height or the height of your screen , this is why I had to zoom out because of that limit. Instead of that, min-height: 100vh will be much better and not specifying the height at all.

  • Adding some custom :focus state on the interactive elements will be really awesome.

  • For the hover state on the bottom, instead of adding padding when hovered, declare it initially to have that padding and border-radius, then when you hover, you could just transition the background-color. This way, there will be no pushing effect.

  Agata already mentioned above key points which is awesome.

  But still, good job on this^

  1

  ---

  

  Diego.Carhuavilca•115

  @diegocarhuavilcaPosted over 3 years ago

  @pikamart thanks you so much for the feedback that help me a lot !

  0
• 

  Agata Liberska•3,995

  @AgataLiberskaPosted over 3 years ago

  Hi @diegocarhuavilca, what issue do you have with the placeholder? Looks ok to me :)

  The things I'd have a look at are hover states on the social media icons (which could also be links in my opinion) - the entire page moves around because a lot of padding is added on hover, and the position of the error message, I think it should be just below the input on mobile (it's below the button now) and it's not aligned with the input on some larger screens.

  Hope this helps :)

  1