Pro1 #31
Pro1 #31
Conversation
Tamir198
left a comment
Tamir198
left a comment
There was a problem hiding this comment.
Hey left you some comments, please make sure to change every place in the code thats relevant to the comment, I didnt want to repeat myself
| height: 100vh; | ||
| margin: 0; | ||
| padding: 0; | ||
| background-color: #2c2e47; |
There was a problem hiding this comment.
colors should come from css vairables
| } | ||
|
|
||
| .maincontainer { | ||
| margin-top: 32%; |
| background: white; | ||
| border-radius: 20px; | ||
| display: flex; | ||
| box-shadow: 0 8px 30px rgba(0, 0, 0, 0.25); |
There was a problem hiding this comment.
Same comment about the color in here and in every other place
| padding: 40px; | ||
| } | ||
|
|
||
| .text h1 { |
There was a problem hiding this comment.
You could give the h1 class making it more specific
| } | ||
|
|
||
| #subscribeBtn { | ||
| width: 110%; |
There was a problem hiding this comment.
Can you use class instead? id will override all of the other css rules made by class.
Also, why do you need this element to be larger than his parent?
|
|
||
| const [, domain] = email.split("@"); | ||
|
|
||
| if (domain === "gmail.com" || domain === "colman.com") { |
There was a problem hiding this comment.
Hardcoded strings should be saved inside constants
|
|
||
| if (domain === "gmail.com" || domain === "colman.com") { | ||
| btn.classList.add("valid"); | ||
| } else { |
There was a problem hiding this comment.
You could add return here to avoid the else
| } | ||
| }); | ||
|
|
||
| btn.onclick = function () { |
There was a problem hiding this comment.
Why do you need to do it like this? you are already using even listeners, dont override the default on click function
| const email = input.value; | ||
|
|
||
| if (!email.includes("@")) { | ||
| error.style.display = "block"; |
There was a problem hiding this comment.
Never style stuff from your js, use it via classes and css
| successCard.style.display = "flex"; | ||
| }; | ||
|
|
||
| const dismissBtn = document.getElementById("dismissBtn"); |
There was a problem hiding this comment.
You should place all of your elements on the top of the file
2 participants
No description provided.