Skip to content

Final solution upload #16

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
RazBenLulu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Final solution upload #16

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
92 changes: 92 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,92 @@
@import url('https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,100..900;1,100..900&display=swap');

body {
display: flex;
justify-content: center;
align-items: center;
height: 100vh;
background-color: rgb(48, 64, 89);
font-family: system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif;
Copy link

@R0EYK R0EYK Nov 28, 2025

Choose a reason for hiding this comment

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

Do we actually need all these fonts? I can see we are importing the Roboto font so I am asking myself if we actually need all the others

}

.box {
display: flex;
background-color: white;
border-radius: 40px;
padding: 25px;
gap: 80px;
height: 600px;
align-items: stretch;
overflow: hidden;
}

.text {
display: flex;
flex-direction: column;
justify-content: center;
}

.rightImg {
width: 350px;
height: 100%;
object-fit: cover;
border-radius: 15px;
}

.title{
display: flex;
justify-content:center;
color: hsla(234, 67%, 17%, 0.863);
Copy link

@R0EYK R0EYK Nov 28, 2025

Choose a reason for hiding this comment

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

Consider using css variables to use DO NOT REPEAT YOURSELF principle and have a single point of truth for your colors pallete

font-size: 55px;
padding-left: 30px;
margin: 0;
line-height: 1.5;
}

ul {
list-style-type: none;
}

li {
display: flex;
padding: 6px;
gap: 13px;

}

form{
display: flex;
flex-direction: column;

}

.emailInput{
Copy link

@R0EYK R0EYK Nov 28, 2025

Choose a reason for hiding this comment

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

See how you position the below elements with margings and paddings, instead, you could use a single container and use gap margin and padding attributes on that container so all of the elements in it would be positioned accordingly.

This would make your life much easier

width: 324px;
padding: 13px;
border:2px solid rgb(196, 196, 196);
border-radius: 5px;
margin: -3px;
margin-left: 50px;
padding-left:20px;

}

.subscribeButton{
background-color: hsla(234, 67%, 17%, 0.863);
color: white;
padding: 12px;
border-radius: 5px;
width:362px;
margin-left: 50px;
margin-bottom: 55px;

}

.emailText{
margin-left: 50px;
margin-top: 20px;
}

.sub{
margin-left: 44px;
}
55 changes: 35 additions & 20 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
@@ -1,22 +1,14 @@
<!DOCTYPE html>
<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 -->

<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>
</head>
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link rel="stylesheet" href="index.css">

<body>

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

Stay updated!

Expand All @@ -29,24 +21,47 @@
Email address
email@company.com

Subscribe to monthly newsletter
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
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>.

<div class="box">

<div class="text">
<h1 class="title">Stay updated! </h1>
Copy link

@R0EYK R0EYK Nov 28, 2025

Choose a reason for hiding this comment

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

I usually do not like to use <h1> to control the size of my text, you can have much finer control if you use css

<p class="sub">Join 60,000+ product managers receiving monthly <br>updates on:</p>
<ul class="list">
<li><img src="./assets/images/icon-list.svg">Product discovery and building what matters</li>
<li><img src="./assets/images/icon-list.svg">Measuring to ensure updates are a success</li>
<li><img src="./assets/images/icon-list.svg">And much more!</li>
</ul>

<b class ="emailText">Email address</b>
<br>
<form action="sendEmail">
Copy link

@R0EYK R0EYK Nov 28, 2025

Choose a reason for hiding this comment

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

Good job on using the <form> element here, this gives us a lot of things out of the box for accessability and functionality like submit button which we will not have in a normal <div> element

<input type="email" input class="emailInput" placeholder="email@company.com">
<br>
Copy link

@R0EYK R0EYK Nov 28, 2025

Choose a reason for hiding this comment

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

Its considered a bad practice to use a <br> in order to get spacing, consider having these inputs in a flex container where you could set a gap in order to control the spacing between elements

<input type="submit" class="subscribeButton" value="Subscribe to monthly newsletter">
</form>
</div>

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


</div>


</body>

</html>