Skip to content

שינויים ב-index.html והוספת index.css #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DinaHK2806 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

שינויים ב-index.html והוספת index.css #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions newsletter-sign-up-with-success-message-main/html-css-practice
Open in desktop
Submodule html-css-practice added at 667baf
182 changes: 182 additions & 0 deletions newsletter-sign-up-with-success-message-main/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
body{
background-color: hsl(235, 18%, 26%);
min-height: 100vh;
display: flex;
justify-content: center;
align-items: center;
}

.attribution { font-size: 11px; text-align: center; }
.attribution a { color: hsl(228, 45%, 44%); }

.box{
position: absolute;
width: 900px;
height: 640px;
background-color: white;
border-radius: 20px;
box-shadow:0 5px 20px rgba(0, 0, 0, 0.5);
}

.title{
font-family: "Roboto", sans-serif;
Copy link

@GilHeller GilHeller Nov 26, 2025

Choose a reason for hiding this comment

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

Avoid repeating font-family.

You're defining font-family: Roboto many times across the stylesheet. Instead, consider setting a global default using:

:root {
  font-family: "Roboto", sans-serif;
  ....
}

color: hsl(234, 29%, 20%);
padding-left: 60px;
font-size: 50px;
font-weight: 700;
padding-top: 60px;
}

.p1{
Copy link

@GilHeller GilHeller Nov 26, 2025

Choose a reason for hiding this comment

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

Consider renaming the class name to something more meaningful.

Copy link

@GilHeller GilHeller Nov 26, 2025

Choose a reason for hiding this comment

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

You used the class only once. Consider convert it into id instead.

font-family: "Roboto", sans-serif;
font-size: 16px;
color: rgb(75, 75, 75);
padding-left: 60px;
font-weight: 400;
line-height: 1.5rem;
}

.icon{
width: 20px;
height: 20px;
padding-right: 15px;
}

.list{
display: flex;
align-items: center;
font-weight: 400;
font-family: "Roboto", sans-serif;
font-size: 16px;
color: rgb(75, 75, 75);
margin-bottom: 10px;
padding-left: 60px;
}

h3{
font-family: "Roboto", sans-serif;
color: hsl(234, 29%, 20%);
padding-left: 60px;
font-size: 14px;
font-weight: 700;
padding-top: 40px;
}

.input-email{
margin-left: 60px;
width: 335px;
height: 50px;
border-radius: 10px;
border-color: hsl(0, 0%, 70%);
color: hsl(0, 0%,70%);
Comment on lines +70 to +71
Copy link

@GilHeller GilHeller Nov 26, 2025

Choose a reason for hiding this comment

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

Use CSS variables instead of repeating colors.

:root {
  --neutral-gray: hsl(0, 0%, 70%);
}

....

border-color: var(--neutral-gray);
color: var(--neutral-gray);

border-style: solid;
font-family: "Roboto", sans-serif;
font-weight: 400;
padding-left: 20px;
font-size: 14px;
}

.input-email:hover{
border-color: black;
}


.submit-btn{
width: 360px;
height: 55px;
background-color: hsl(234, 29%, 20%);
border-radius: 10px;
border-color: hsl(234, 29%, 20%);
border-style: solid;
color: white;
font-family: "Roboto", sans-serif;
font-weight: 700;
font-size: 15px;
}

.submit-btn:hover{
background: linear-gradient(to left, rgb(255, 115, 0), rgb(255, 70, 101));
border-style: hidden;
box-shadow: 0 10px 40px rgba(255, 70, 117, 0.5),
0 10px 40px rgb(255, 193, 157);
}

.box-btn{
padding-left: 60px;
padding-top: 20px;
}

.img-box{
display: flex;
justify-content: flex-end;
align-items: center;
align-self: center;
padding-top: 70px;
padding-right: 15px;
}

.ill-sgn-dsk{
position: absolute;
align-self: flex-end;
}

#success-box{
display: none;
position: absolute;
width: 480px;
height: 500px;
background-color: white;
border-radius: 20px;
box-shadow:0 5px 20px rgba(0, 0, 0, 0.5);
}

.success-img{
padding-left: 60px;
padding-top: 40px;
}

.success-title{
font-family: "Roboto", sans-serif;
color: hsl(234, 29%, 20%);
padding-left: 60px;
font-size: 50px;
font-weight: 700;
}

.success-p{
font-family: "Roboto", sans-serif;
font-size: 16px;
color: hsl(234, 29%, 20%);
padding-left: 60px;
font-weight: 400;
line-height: 1.5rem;
}

.success-btn{
display: flex;
padding-top: 20px;
justify-content: center;
}

.error-email{
Copy link

@GilHeller GilHeller Nov 26, 2025

Choose a reason for hiding this comment

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

Consider to use id instead.

display: none;
color: hsl(4, 100%, 67%);
font-family: "Roboto", sans-serif;
font-weight: 400;
font-size: 14px;
padding-top: 55px;
}

.email-titles{
display: flex;
flex-direction: column;
width: 420px;
}

.titles-style{
display: flex;
justify-content: space-between;

}


80 changes: 62 additions & 18 deletions newsletter-sign-up-with-success-message-main/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,50 +3,94 @@
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0"> <!-- displays site properly based on user's device -->
<link rel="preconnect" href="https://fonts.googleapis.com">
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Overpass:ital,wght@0,100..900;1,100..900&family=Roboto:ital,wght@0,100..900;1,100..900&display=swap" rel="stylesheet">

<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" href="index.css">
</head>
<body>

<!-- Sign-up form start -->
<div class="box">

Stay updated!

Join 60,000+ product managers receiving monthly updates on:
<h1 class="title">Stay updated!</h1>

Product discovery and building what matters
Measuring to ensure updates are a success
And much more!
<p class="p1">Join 60,000+ product managers receiving monthly<br>
updates on:</p>

Email address
email@company.com
<div class="list">
<img src="assets/images/icon-list.svg" class="icon">
<span>Product discovery and building what matters</span>
</div>
<div class="list">
<img src="assets/images/icon-list.svg" class="icon">
<span>Measuring to ensure updates are a success</span>
</div>
<div class="list">
<img src="assets/images/icon-list.svg" class="icon">
<span>And much more!</span>
</div>

Subscribe to monthly newsletter
<div class="email-titles">
<div class="titles-style">
<h3>Email address</h3>
<span id="email-error" class="error-email">Valid email required</span>
</div>
</div>
<input type="email" id="email" class="input-email" placeholder="email@company.com">

<!-- Sign-up form end -->
<div class=box-btn>
<button type="submit" class="submit-btn" onclick="login()">Subscribe to monthly newsletter</button>
</div>

<!-- Success message start -->
<div class="img-box">
<img src="assets/images/illustration-sign-up-desktop.svg" class="ill-sgn-dsk">
</div>
</div>

Thanks for subscribing!
<!-- Sign-up form end -->

A confirmation email has been sent to ash@loremcompany.com.
Please open it and click the button inside to confirm your subscription.
<!-- Success message start -->
<div id="success-box" class="success-box">
<img src="assets/images/icon-success.svg" class="success-img">
<h1 class="success-title">Thanks for subscribing!</h1>

Dismiss message
<div class="success-p">A confirmation email has been sent to <strong>ash@loremcompany.com</strong>.
Please open it and click<br> the button inside to confirm your subscription.</div>
<div class="success-btn">
<button type="submit" class="submit-btn">Dismiss message</button>
</div>
</div>

<!-- 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>.
</div>
<script>
function login(){
const email=document.getElementById('email').value;
const emailInput = document.getElementById('email');
const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
Comment on lines +80 to +82
Copy link

@GilHeller GilHeller Nov 26, 2025

Choose a reason for hiding this comment

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

Declare constants and cache DOM elements outside the function

if(emailRegex.test(email)){
document.querySelector('.box').style.display = 'none';
document.getElementById('success-box').style.display = 'block';
}
else{
document.getElementById('email-error').style.display = 'block';
emailInput.style.borderColor = 'hsl(4, 100%, 67%)';
emailInput.style.backgroundColor='hsl(4, 100%, 95%)';
Comment on lines +89 to +90
Copy link

@GilHeller GilHeller Nov 26, 2025

Choose a reason for hiding this comment

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

Avoid setting styles directly in JavaScript.
you can add a class and add/remove it when needed:

emailInput.classList.add("input-error");
emailInput.classList.remove("input-error");

}
Comment on lines +83 to +91
Copy link

@GilHeller GilHeller Nov 26, 2025

Choose a reason for hiding this comment

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

Apply Single Responsibility to your functions

Right now login() handles multiple concerns:
(1) email validation, and (2) resetting input styles/UI state.

Consider extracting those concerns into separate helper functions: isValidEmail() and resetInputState(). Then call them inside login()

function login() {
  resetInputState();

  const email = emailInput.value.trim();

  if (isValidEmail(email)) {
    formBox.style.display = "none";
    successBox.style.display = "block";
  } else {
    emailInput.classList.add("input-error");
    errorText.style.display = "block";
  }
}


}
</script>
Comment on lines +78 to +94
Copy link

@GilHeller GilHeller Nov 26, 2025

Choose a reason for hiding this comment

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

Move inline JavaScript to a separate file.

Attach it in your HTML using:

<script src="index.js"></script>

</body>
</html>