Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 44 additions & 40 deletions newsletter-sign-up-with-success-message-main/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,51 +2,55 @@
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0"> <!-- displays site properly based on user's device -->

<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">

<title>Frontend Mentor | Newsletter sign-up form with success message</title>

<!-- Feel free to remove these styles or customise in your own stylesheet 👍 -->
<style>
.attribution { font-size: 11px; text-align: center; }
.attribution a { color: hsl(228, 45%, 44%); }
</style>
<link rel="stylesheet" type="text/css" href="./style.css">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need the type?

</head>
<body>
<div class="backOfWite">
<div class="frontBox">
<div class="allText">

<h1 class="textPage" id="higlight">Stay updated!</h1>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You dont really need both id and class

<p class="textPage">Join 60,000+ product managers receiving monthly updates on:</p>

<div class="checkText">
<img src="./assets/images/icon-list.svg">
<p class="textPage">Product discovery and building what matters</p>
</div>
<div class="checkText">
<img src="./assets/images/icon-list.svg">
<p class="textPage">Measuring to ensure updates are a success</p>
</div>
<div class="checkText">
<img src="./assets/images/icon-list.svg">
<p class="textPage">And much more!</p>
</div>

<div class="mail">
<label class="textPage" for="email" id="e-mail">Email address</label>
<input id="email" type="text" placeholder="email@example.com">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here why do you need id? you could achieve the same with class

<p id="errorMsg"></p>
<div class="subs">
<button id="buttonSub" type="button">Subscribe to monthly newsletter</button>
</div>
</div>

</div>

<div class="frontBox">
<img src="./assets/images/illustration-sign-up-desktop.svg">
</div>

</div>
</div>

<!-- Sign-up form start -->

Stay updated!

Join 60,000+ product managers receiving monthly updates on:

Product discovery and building what matters
Measuring to ensure updates are a success
And much more!

Email address
email@company.com

Subscribe to monthly newsletter

<!-- Sign-up form end -->

<!-- Success message start -->

Thanks for subscribing!

A confirmation email has been sent to ash@loremcompany.com.
Please open it and click the button inside to confirm your subscription.

Dismiss message

<!-- Success message end -->

<div class="attribution">
Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>.
Coded by <a href="#">Your Name Here</a>.
Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>.
Coded by <a href="#">Linoy Rainshtein</a>.
</div>

<script src="./scrips.js"></script>
</body>
</html>
</html>
39 changes: 39 additions & 0 deletions newsletter-sign-up-with-success-message-main/scrips.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
document.addEventListener("DOMContentLoaded", function() {
const emailInput = document.getElementById("email");
const button = document.getElementById("buttonSub");
const errorMsg = document.getElementById("errorMsg");

function isValidEmail(email) {
const regex = /^[a-zA-Z]{1,15}@[a-zA-Z]{1,15}\.com$/;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regex should be saved inside constant variable

return regex.test(email);
}

button.addEventListener("click", function() {
console.log("Button clicked!");
const emailValue = emailInput.value.trim();
console.log("Email" ,emailValue);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove your consoles, we dont want to push consoles.

Also I would add the ? symbol for null safety on the email


// שדה ריק
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments that explain what the code is doing are bad, the code should explain itself.
Only write comment if you have really good reason.

Also, never write in hebrew

if (emailValue === "") {
errorMsg.textContent = "You Need To Enter An EMAIL first";
errorMsg.style.visibility = "visible";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We dont want to change the styles via js, do it with dynamic classes and css

emailInput.style.border = "2px solid red";
return;
}

// פורמט לא תקין
if (!isValidEmail(emailValue)) {
errorMsg.textContent = "Wrong Mail Format";
errorMsg.style.visibility = "visible";
emailInput.style.border = "2px solid red";
return;
}

// הצלחה
errorMsg.style.visibility = "hidden";
emailInput.style.border = "2px solid green";

// מעבר לעמוד אחר
window.location.href = "success.html";
});
});
89 changes: 89 additions & 0 deletions newsletter-sign-up-with-success-message-main/style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
body {
background-color: hsl(235, 18%, 26%);
Copy link
Member

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 variables

}

.backOfWite {
display: flex;
flex-direction: row;
align-items: center;
justify-content: center;
margin-top: 5%;
}

.frontBox {
display: flex;
flex-direction: row;
background-color: white;
justify-content: space-around;
border-radius: 5%;
}

.allText {
display: flex;
flex-direction: column;
justify-content: space-between;
margin-left: 5%;
margin-bottom: 10%;
margin-top: 5%;
}

.checkText {
display: flex;
flex-direction: row;
gap: 5%;
}

@font-face {
font-family: 'myFont1';
src: url('./assets/fonts/Roboto-Regular.ttf') format('truetype');
}

@font-face {
font-family: 'myFont2';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need 2 font face?

src: url('./assets/fonts/Roboto-Bold.ttf') format('truetype');
}

.textPage {
font-family: "myFont1", sans-serif;
font-size: 16px;
font-weight: 400;
}

#higlight {
font-family: "myFont2", sans-serif;
font-size: 50px;
font-weight: 700;
}

.mail {
display: flex;
flex-direction: column;
gap: 10px;
margin-top: 5%;
}

#errorMsg {
color: red;
font-size: 12px;
margin-top: -5px;
visibility: hidden; /* JS ישנה ל-visible */
}

#e-mail {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

be consistent with your naming, like-this or likeThis

font-family: "myFont2", sans-serif;
font-size: 13px;
font-weight: 900;
}

#buttonSub {
background-color: hsl(234, 29%, 20%);
color: white;
cursor: pointer;
}

.mail input, #buttonSub {
width: 300px;
padding: 12px;
border-radius: 5px;
box-sizing: border-box;
}
28 changes: 28 additions & 0 deletions newsletter-sign-up-with-success-message-main/stylsSuccess.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
body {
background-color: hsl(235, 18%, 26%);
}

.backOfWite {
display: flex;
flex-direction: row;
align-items: center;
justify-content: center;
margin-top: 5%;
}

.frontBox {
display: flex;
flex-direction: row;
background-color: rgb(246, 125, 12);
align-items: center;
justify-content: center;
border-radius: 5%;
height: 500px;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this be responsive in mobile?

width: 500px;
}

#WELCOMEH{
color: white;
font-size: 50px;
font-weight: bold;
}
15 changes: 15 additions & 0 deletions newsletter-sign-up-with-success-message-main/success.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

<html>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are you using this html page?

<head>
<link rel="stylesheet" type="text/css" href="./stylsSuccess.css">
</head>

<body>
<div class="backOfWite">
<div class="frontBox">
<p id="WELCOMEH">WELCOME !!!!!!</p>
</div>
</div>
</body>

</html>