@FluffyKas
Posted
Heyo,
Looks pretty good! JS wise the only thing I'd double-check is the regex. test@test passed as a valid email address even though it's not exactly valid.
Semantic markup:
- You should wrap everything in a <main>. This could be instead of the div called main-wrapper. Alternatively, you can separate the content of the page into header (ping logo), main (form and image) and footer (socials).
- The social links should be actual links with aria-labels describing where they lead. Best to wrap these in a <ul>.
- The input and button should be wrapped in a <form>. A label is also missing. Each input should have a label describing what they do, a placeholder is not a reliable equivalent of this. Since the design doesn't contain the label itself, it can be easily hidden with an sr-only (for screen readers) class, like so:
.sr-only {
position: absolute;
width: 1px;
height: 1px;
padding: 0;
margin: -1px;
overflow: hidden;
clip: rect(0, 0, 0, 0);
white-space: nowrap;
border-width: 0;
}
CSS:
- The mobile and desktop sizes are good, but the tablet size is a bit lacking. You could give a max-width to the image and to the form as well so they don't stretch the whole width of the screen.
- The input has a huge left padding in desktop view, you could perhaps reduce this a bit to look nicer.
- Socials: I saw this code there:
@media (min-width: 1440px)
.social-wrapper {
padding: 0rem 40.9375rem;
}
.social-wrapper {
display: grid;
grid-template-columns: repeat(3, 1fr);
grid-template-rows: 1fr;
padding: 0rem 7.8125rem;
}
I see what you're trying to achieve here but instead of the huge padding, you could do something easier:
display: flex;
justify-content: center;
gap: 1.5rem; (note that this is just a guess)
This would be a bit harder to achieve with a grid, at least how you have done it. With flexbox you probably won't even need a media query.
I hope I was able to help! (:
Marked as helpful
@ClaudioAmareno
Posted
@FluffyKas Thank you a lot!