Final solution upload #16
Conversation
R0EYK
left a comment
R0EYK
left a comment
There was a problem hiding this comment.
Hi, I have left you some comments on your code, good job overall.
My most important note to you here would be that you start relying more on containers where you position all the elements in it correctly once by using gap,padding and margings on that container then you will not need to position every single element with margings and paddings
| align-items: center; | ||
| height: 100vh; | ||
| background-color: rgb(48, 64, 89); | ||
| font-family: system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif; |
There was a problem hiding this comment.
Do we actually need all these fonts? I can see we are importing the Roboto font so I am asking myself if we actually need all the others
| .title{ | ||
| display: flex; | ||
| justify-content:center; | ||
| color: hsla(234, 67%, 17%, 0.863); |
There was a problem hiding this comment.
Consider using css variables to use DO NOT REPEAT YOURSELF principle and have a single point of truth for your colors pallete
| <br> | ||
| <form action="sendEmail"> | ||
| <input type="email" input class="emailInput" placeholder="email@company.com"> | ||
| <br> |
There was a problem hiding this comment.
Its considered a bad practice to use a <br> in order to get spacing, consider having these inputs in a flex container where you could set a gap in order to control the spacing between elements
|
|
||
| <b class ="emailText">Email address</b> | ||
| <br> | ||
| <form action="sendEmail"> |
There was a problem hiding this comment.
Good job on using the <form> element here, this gives us a lot of things out of the box for accessability and functionality like submit button which we will not have in a normal <div> element
| <div class="box"> | ||
|
|
||
| <div class="text"> | ||
| <h1 class="title">Stay updated! </h1> |
There was a problem hiding this comment.
I usually do not like to use <h1> to control the size of my text, you can have much finer control if you use css
|
|
||
| } | ||
|
|
||
| .emailInput{ |
There was a problem hiding this comment.
See how you position the below elements with margings and paddings, instead, you could use a single container and use gap margin and padding attributes on that container so all of the elements in it would be positioned accordingly.
This would make your life much easier
2 participants
No description provided.