CS Dev Club 1st task - David Norman #15
Conversation
…igation based on a valid email regex; 4. error on bad regex email; 5. bonus: star wall animation (made using AI)
| body { | ||
| font-family: 'Roboto-Regular', sans-serif; | ||
| background-color: hsla(235, 18%, 26%, 1.00); | ||
| height: 95vh; |
There was a problem hiding this comment.
why not 100vh (Full viewport height)
| height: 60vh; | ||
| width: 60vw; | ||
| background-color: hsl(0, 0%, 100%); | ||
| border-radius: 1.5rem; |
| border-radius: 0.5rem; | ||
| text-align: center; | ||
| color: black; | ||
| border: 1px solid black; |
There was a problem hiding this comment.
General note: I don't see a @media (max-width: 768px) {} is it responsive for mobile ?
| dismissBtn.style.backgroundImage = "linear-gradient(to right, hsla(313, 97%, 38%, 1.00), hsla(334, 85%, 26%, 1.00), hsla(313, 97%, 38%, 1.00))"; | ||
| }); | ||
|
|
||
| dismissBtn.addEventListener("mouseleave", () => { |
There was a problem hiding this comment.
This is a lambda function, worth reading up on how and when to use them
| const starColumn2 = document.querySelector("#star-column-2"); | ||
| const starsColumn2 = starColumn2.querySelectorAll(".child"); | ||
| const starColumn3 = document.querySelector("#star-column-3"); | ||
| const starsColumn3 = starColumn3.querySelectorAll(".child"); |
There was a problem hiding this comment.
nice use of const and let, when you have the chance read about the difference, and why we never use "var"
There was a problem hiding this comment.
Readable code, nice structure. good work!
Tamir198
left a comment
Tamir198
left a comment
There was a problem hiding this comment.
Hey left my review about it , nice animation on the stars
|
|
||
| <!-- Sign-up form end --> | ||
| <body> | ||
| <div class="main-container"> |
|
|
||
| Thanks for subscribing! | ||
| <div class="star-wall"> | ||
| <div class="star-column" id="star-column-1"> |
There was a problem hiding this comment.
Why do you need both id and class
| Please open it and click the button inside to confirm your subscription. | ||
| <img class="illustration" | ||
| src="./assets/images/illustration-sign-up-desktop.svg" | ||
| alt="Sign up illustration"> |
| const starsColumn3 = starColumn3.querySelectorAll(".child"); | ||
|
|
||
| function validateEmail() { | ||
| let inputMail = emailInpt.value; |
There was a problem hiding this comment.
I would check that the value is actually not null
| let inputMail = emailInpt.value; | ||
| const regex = new RegExp("^\\S+@\\S+\\.\\S+$"); | ||
| if (regex.test(inputMail)) { | ||
| mainContainer.style.display = "none"; |
There was a problem hiding this comment.
Its not recommended to style elements like this, change he css classes and let the css classes do the work for you
|
|
||
|
|
||
| subscribeBtn.addEventListener("mouseover", () => { | ||
| subscribeBtn.style.backgroundImage = "linear-gradient(to right, hsla(313, 97%, 38%, 1.00), hsla(334, 85%, 26%, 1.00), hsla(313, 97%, 38%, 1.00))"; |
There was a problem hiding this comment.
Same comment in here about styling via js, also save those hardcoded strings into variables because you have repeating yourself
|
|
||
| - Blue 800: hsl(234, 29%, 20%) | ||
| - Blue 700: hsl(235, 18%, 26%) | ||
| - Blue 700: hsla(235, 18%, 26%, 1.00) |
| flex-direction: column; | ||
| height: 50vh; | ||
| width: 30vw; | ||
| background-color: hsl(0, 0%, 100%); |
There was a problem hiding this comment.
All of your colors in here should come from the css vraibles
| text-align: center; | ||
| } | ||
|
|
||
| /* Only this animation was done with AI! */ |
There was a problem hiding this comment.
Great but this comment is not needed, its ok to use ai and if the code is written in a good way you dont acutally need to write comment about it
| } | ||
|
|
||
| #star-column-1 .star, | ||
| #star-column-3 .star { |
There was a problem hiding this comment.
Will it not work with only .star ?
Also why do you need the usage of the id? its got high specificity in css
3 participants
No description provided.