-
Notifications
You must be signed in to change notification settings - Fork 38
שינויים ב-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.
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
base: main
Are you sure you want to change the base?
Conversation
GilHeller
left a comment
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.
Great work. I left you some comments.
| } | ||
|
|
||
| .title{ | ||
| font-family: "Roboto", sans-serif; |
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.
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;
....
}| padding-top: 60px; | ||
| } | ||
|
|
||
| .p1{ |
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.
Consider renaming the class name to something more meaningful.
| padding-top: 60px; | ||
| } | ||
|
|
||
| .p1{ |
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.
You used the class only once. Consider convert it into id instead.
| border-color: hsl(0, 0%, 70%); | ||
| color: hsl(0, 0%,70%); |
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.
Use CSS variables instead of repeating colors.
:root {
--neutral-gray: hsl(0, 0%, 70%);
}
....
border-color: var(--neutral-gray);
color: var(--neutral-gray);| <script> | ||
| function login(){ | ||
| const email=document.getElementById('email').value; | ||
| const emailInput = document.getElementById('email'); | ||
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||
| 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%)'; | ||
| } | ||
|
|
||
| } | ||
| </script> |
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.
Move inline JavaScript to a separate file.
Attach it in your HTML using:
<script src="index.js"></script>| const email=document.getElementById('email').value; | ||
| const emailInput = document.getElementById('email'); | ||
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; |
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.
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%)'; | ||
| } |
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.
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";
}
}| emailInput.style.borderColor = 'hsl(4, 100%, 67%)'; | ||
| emailInput.style.backgroundColor='hsl(4, 100%, 95%)'; |
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.
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");| justify-content: center; | ||
| } | ||
|
|
||
| .error-email{ |
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.
Consider to use id instead.
הוספתי בדף הHTML את התכונות והפקודות הרצויות והוספתי דף עיצוב CSS שבו מעוצב כל דף הHTML. בנוסף הוספתי תגית script שמפעילה את כפתור ההתחברות, בודקת האם המייל שהוכנס תקין, אם כן, היא מעבירה את המשתמש לדף הבא שבו הודעת ההתחברות עברה בהצלחה. אם לא, מעלה תגית שיש צורך במייל תקין.