Pro1 #31
There are no files selected for viewing
Binary file not shown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| body { | ||
| height: 100vh; | ||
| margin: 0; | ||
| padding: 0; | ||
| background-color: #2c2e47; | ||
| font-family: Arial, sans-serif; | ||
| } | ||
|
|
||
| .maincontainer { | ||
| margin-top: 32%; | ||
|
|
||
| height: 100vh; | ||
| display: flex; | ||
| flex-direction: column; | ||
| justify-content: center; | ||
| align-items: center; | ||
| } | ||
|
|
||
| .center { | ||
| padding: 20px; | ||
| width: 750px; | ||
| background: white; | ||
| border-radius: 20px; | ||
| display: flex; | ||
| box-shadow: 0 8px 30px rgba(0, 0, 0, 0.25); | ||
|
Member
There was a problem hiding this comment. Same comment about the color in here and in every other place |
||
| overflow: hidden; | ||
| margin-bottom: 20px; | ||
| } | ||
|
|
||
| #successCard { | ||
| display: none; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| justify-content: center; | ||
| width: 381px; | ||
| } | ||
|
|
||
| .text { | ||
| flex: 1; | ||
| padding: 40px; | ||
| } | ||
|
|
||
| .text h1 { | ||
|
Member
There was a problem hiding this comment. You could give the |
||
| margin-top: 0; | ||
| font-size: 32px; | ||
| font-weight: 700; | ||
| } | ||
|
|
||
| .text p { | ||
| color: #555; | ||
| margin-bottom: 15px; | ||
| } | ||
|
|
||
| .text ul { | ||
| list-style: none; | ||
| padding: 0; | ||
| margin-bottom: 30px; | ||
| } | ||
|
|
||
| .text ul li { | ||
| margin-bottom: 10px; | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 10px; | ||
| } | ||
|
|
||
| .text label { | ||
| font-size: 12px; | ||
| font-weight: bold; | ||
| display: block; | ||
| margin-bottom: 6px; | ||
| } | ||
|
|
||
| .text input { | ||
| width: 100%; | ||
| padding: 12px; | ||
| margin-bottom: 20px; | ||
| border-radius: 6px; | ||
| border: 1px solid #ccc; | ||
| font-size: 14px; | ||
| } | ||
|
|
||
| #subscribeBtn { | ||
| width: 110%; | ||
|
Member
There was a problem hiding this comment. Can you use class instead? id will override all of the other css rules made by class. |
||
| padding: 14px; | ||
| border-radius: 6px; | ||
| border: none; | ||
| background-color: #1d223a; | ||
| color: white; | ||
| font-size: 14px; | ||
| cursor: pointer; | ||
| font-weight: bold; | ||
| transition: 0.3s ease; | ||
| } | ||
|
|
||
| #subscribeBtn.valid { | ||
| background: linear-gradient(to right, #ff6f91, #ff9671); | ||
| } | ||
|
|
||
| #dismissBtn:hover { | ||
| opacity: 0.9; | ||
| } | ||
|
|
||
| #errorMsg { | ||
| display: none; | ||
| color: red; | ||
| font-size: 9px; | ||
| margin-bottom: 5px; | ||
| text-align: right; | ||
| margin-right: -23px; | ||
| } | ||
|
|
||
| #dismissBtn { | ||
| width: 110%; | ||
| padding: 14px; | ||
| border-radius: 6px; | ||
| border: none; | ||
| background-color: #1d223a; | ||
| color: white; | ||
| font-size: 14px; | ||
| cursor: pointer; | ||
| font-weight: bold; | ||
| margin-top: 20px; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,52 +1,61 @@ | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
|
|
||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <link rel="icon" type="image/png" sizes="32x32" href="./assets/images/favicon-32x32.png"> | ||
| <link rel="stylesheet" href="index.css"> | ||
| <title>Newsletter Sign-up</title> | ||
| </head> | ||
|
|
||
| <body> | ||
|
|
||
| <div class="maincontainer"> | ||
|
|
||
|
|
||
| <div id="formCard" class="center"> | ||
|
Member
There was a problem hiding this comment. Why do you need both id and class? |
||
|
|
||
| <div class="text"> | ||
| <h1>Stay updated!</h1> | ||
| <p>Join 60,000+ product managers receiving monthly updates on:</p> | ||
|
|
||
| <ul> | ||
| <li><img src="./assets/images/icon-list.svg" class="list-icon"> Product discovery and building what matters</li> | ||
|
Member
There was a problem hiding this comment. You should add alt attribute for accessibility |
||
| <li><img src="./assets/images/icon-list.svg" class="list-icon"> Measuring to ensure updates are a success</li> | ||
| <li><img src="./assets/images/icon-list.svg" class="list-icon"> And much more!</li> | ||
| </ul> | ||
|
|
||
| <label>Email address</label> | ||
| <p id="errorMsg">Valid email required</p> | ||
| <input id="emailInput" type="email" placeholder="email@company.com"> | ||
|
|
||
| <button id="subscribeBtn">Subscribe to monthly newsletter</button> | ||
| </div> | ||
|
|
||
| <div class="imager"> | ||
| <img src="./assets/images/illustration-sign-up-desktop.svg" alt="Sign Up Illustration"> | ||
|
|
||
| </div> | ||
|
|
||
| </div> | ||
|
|
||
| <div id="successCard" class="center"> | ||
| <div class="success-content"> | ||
| <img src="./assets/images/icon-success.svg" class="success-icon" alt=""> | ||
|
|
||
| <h1>Thanks for subscribing!</h1> | ||
|
|
||
| <p> | ||
| A confirmation email has been sent. | ||
| Please open it and click the button inside to confirm your subscription. | ||
| </p> | ||
|
|
||
| <button id="dismissBtn">Dismiss message</button> | ||
| </div> | ||
| </div> | ||
|
|
||
| </div> | ||
|
|
||
| <script src="index.js"></script> | ||
| </body> | ||
|
|
||
| </html> | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| const input = document.getElementById("emailInput"); | ||
| const error = document.getElementById("errorMsg"); | ||
| const btn = document.getElementById("subscribeBtn"); | ||
| const formCard = document.querySelector("#formCard"); | ||
| const successCard = document.querySelector("#successCard"); | ||
|
|
||
| input.addEventListener("input", function () { | ||
| const email = input.value; | ||
|
|
||
| if (!email.includes("@")) { | ||
| btn.classList.remove("valid"); | ||
| return; | ||
| } | ||
|
|
||
| const [, domain] = email.split("@"); | ||
|
Member
There was a problem hiding this comment. What are you doing in here? how is this syntax called? and why are you doing it only to access the second array elemnt? You could access the array that split is returning |
||
|
|
||
| if (domain === "gmail.com" || domain === "colman.com") { | ||
|
Member
There was a problem hiding this comment. Hardcoded strings should be saved inside constants |
||
| btn.classList.add("valid"); | ||
| } else { | ||
|
Member
There was a problem hiding this comment. You could add return here to avoid the else |
||
| btn.classList.remove("valid"); | ||
| } | ||
| }); | ||
|
|
||
| btn.onclick = function () { | ||
|
Member
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"; | ||
|
Member
There was a problem hiding this comment. Never style stuff from your js, use it via classes and css |
||
| return; | ||
| } | ||
|
|
||
| const parts = email.split("@"); | ||
| const domain = parts[1]; | ||
|
|
||
| if (domain !== "gmail.com" && domain !== "colman.com") { | ||
| error.style.display = "block"; | ||
| return; | ||
| } | ||
|
|
||
| error.style.display = "none"; | ||
| formCard.style.display = "none"; | ||
| successCard.style.display = "flex"; | ||
| }; | ||
|
|
||
| const dismissBtn = document.getElementById("dismissBtn"); | ||
|
Member
There was a problem hiding this comment. You should place all of your elements on the top of the file |
||
|
|
||
| dismissBtn.onclick = function () { | ||
| successCard.style.display = "none"; | ||
| formCard.style.display = "flex"; | ||
| }; | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
colors should come from css vairables