-
Notifications
You must be signed in to change notification settings - Fork 21
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
[이수지] sprint4 #62
The head ref may contain hidden characters: "Basic-\uC774\uC218\uC9C0-sprint4"
[이수지] sprint4 #62
Conversation
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.
수지님 고생하셨습니다! 다음번엔 완성을 목표로!
@@ -0,0 +1,126 @@ | |||
document.addEventListener('DOMContentLoaded', function () { |
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.
너무 잘해주셨네요 :)
function handleEnterKey(event) { | ||
if (event.key === 'Enter' || event.code === 'NumpadEnter') { | ||
if (event.target === emailInput) { | ||
handleEmailBlur(); // 이메일 유효성 검사 수행 |
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.
handleEmailBlur 라는 이름 보다는 validateEmail 등의 함수 이름이 더 적절해보입니다!
password도 마찬가지구요.
errorElement.style.display = 'none'; | ||
} | ||
|
||
function toggleLoginButton() { |
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.
toggleLoginButton 안에서 단순 toggle 을 해주는 것 뿐만아니라 validation까지 관리하고 있습니다.
지금은 작은 함수여서 괜찮으나 규모가 커진다면 validate 함수가 파편화되어 유지보수하기 힘들어질 수 있습니다.
updateLoginButtonStatus 정도의 이름으로 함수를 변경하고 안에서는 작은 함수로 쪼개어진 validate 함수를 호출해서 사용하거나, 혹은 toggleLoginButton 의 이름에 맞추어서 toggle만 하게 하는 좀 더 가벼운 함수로 변경하고, 함수를 호출하는 곳에서 적절한 조건에 맞게 toggle 을 해주는형태로 변경해보는것도 좋아보여요!
@@ -56,6 +56,15 @@ main { | |||
display: none; | |||
} | |||
|
|||
.login__password-toggle-icon--inactive.visible, |
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.
bem 규칙으로 네이밍 해주셨군요! 아주 보기 좋습니다
@@ -3,9 +3,21 @@ | |||
<head> | |||
<meta charset="UTF-8" /> | |||
<meta name="viewport" content="width=device-width, initial-scale=1.0" /> | |||
<title>판다마켓</title> | |||
<meta property="og:title" content="판다 마켓" /> |
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.
seo 정보들도 잘 넣어주셨군요 :) 좋습니다
} | ||
|
||
function handlePasswordBlur() { | ||
const passwordValue = passwordInput.value.trim(); |
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.
여기 부분은
const {value} = passwordInput;
if(value.trim()==="") { ... }
이러헥 썼으면 조금 더 깔끔했을 것 같아요!
} | ||
|
||
function toggleLoginButton() { | ||
const isEmailValid = |
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.
boolean 을 따로 이름지어서 선언해주시는 습관 매우 좋습니다.
요구사항
기본
로그인
회원가입
심화
주요 변경사항
스크린샷
멘토에게