elyan/newsletter #18
elyan/newsletter #18
Conversation
elyanyemini
force-pushed
the
feat/form
branch
5 times, most recently
from
98e3b3d to
f0dd90e
Compare
elyanyemini
force-pushed
the
feat/form
branch
from
f0dd90e to
3d2b1c9
Compare
R0EYK
left a comment
R0EYK
left a comment
There was a problem hiding this comment.
Hi, I have left you some notes on your code.
Good job overall, code is stractured nice and is easy to read.
| <!-- displays site properly based on user's device. :O --> | ||
|
|
||
| <!-- This is for the requested font --> | ||
| <link rel="preconnect" href="https://fonts.googleapis.com"> |
There was a problem hiding this comment.
These days its considered best practice to import the fonts and use them in your css , you can do it really easily
@font-face {
font-family:Roboto ;
src: url(./assets/fonts/Roboto-Regular.ttf);
}
.body {
font-family: 'Roboto';
}
This would apply that font to all elements in your body and it looks cleaner
| Join 60,000+ product managers receiving monthly updates on: | ||
|
|
||
| <!-- Sign-up form end --> | ||
| <div class="bullet-list"> |
There was a problem hiding this comment.
just my OCD but in some codebases or companys, they will be hard with naming conventions,
so if you started naming your classes with camel case convention (someVariable), try to stick with it and do not have some-variable
|
|
||
| <!-- Success message start --> | ||
| <div class="emailPart"> | ||
| <form action="" method="post" target="_self"> |
There was a problem hiding this comment.
the <form> action , method and target are meant to help us send the form data to some API endpoint which we are not trying to do now, so we can remove these to avoid the form sending weird api requests currently
| <div class="emailPart"> | ||
| <form action="" method="post" target="_self"> | ||
| <h5>Email address:</h5> | ||
| <input type="email" placeholder="email@company.com" class="emailBox" name="email" id="email" required><br><br> |
There was a problem hiding this comment.
Good job on using these input attributes, many HTML elements gives us attributes which we normally would have to use JavaScript to get, but being aware of them can save us coding time
| } | ||
|
|
||
| .hidden { | ||
| display: none !important; |
| max-width: 700px; | ||
| background-color: white; | ||
| border-radius: 3%; | ||
| overflow: hidden; |
There was a problem hiding this comment.
Border radius and paddings are usually given with some other measurment value instead of percentages, try using rem,
this would give you much more consistent layout
| } | ||
|
|
||
| .submitButton{ | ||
| background-color: hsl(235, 18%, 26%); |
There was a problem hiding this comment.
Consider using CSS variables for these colors, it will make the code easier to understand and you will have a single source of truth for your collor pallete
|
|
||
| if (!emailAddress) { | ||
| console.error("Error: Could not find the email input field! Check your HTML IDs."); | ||
| return; |
There was a problem hiding this comment.
Why do we need to validate ourselves here? We are the developers so if we have an element with an id called "email"
we know this would exist so no reason to check for it
2 participants
No description provided.