-
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 #68
The head ref may contain hidden characters: "Basic-\uD55C\uC218\uC9C4-sprint4"
[한수진]sprint4 #68
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.
수진님 이번주차 스프린트 미션하시느라 고생하셨습니다.
배우신 JS로 로직을 작성하는 첫 미션인데 요구사항에 따라 잘 작성하셨습니다.
다음주 리액트 미션도 화이팅입니다~
아래는 전반적인 리뷰사항입니다.
- 동일한 사항에 대해서는 한번만 코멘트를 답니다. 비슷한 경우를 찾아서 변경해보세요~
- 로그인, 회원가입 로직에서 공통되는 함수들은 따로 분리하고, inscript, upscript에서 해당 로직들을 가져다 쓰는 방식으로 작업하셔도 좋을 것 같습니다.
- 일관되게 변수명, 함수명을 작성하신것이 좋았습니다. 간결한 이름보다 길더라도 어떤 동작을 하는 친구인지 알 수 있는 이름이 좋으니 편하게 작성해보세요~
아이디 창의 조건이 만족되면 비밀번호의 오류 창이 활성화 됩니다. => 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.
P2:
gitignore 파일은 root dir에 위치해야 합니다~
해당 파일을 root로 옮기시고 .DS_store를 삭제해보시면 더 좋을 것 같아요~
https://docs.github.com/en/get-started/getting-started-with-git/ignoring-files
@@ -60,5 +66,6 @@ | |||
<a class="login-signup-link" href="signup.html">회원가입</a> | |||
</div> | |||
</footer> | |||
<script src="JavaScript/inscript.js"></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.
P2:
HTML 문서 다운로드 후 JS가 다운로드 되게 하단에 배치하신 것 같은데
하단에 배치된 스크립트는 구문분석 순서상 추후에 다운로드 되게 만들어주지만 일반적으로 스크립트 태그가 위치되는 head 요소가 아니므로 가독성 측면에서 단점이 있고 이러한 동작은 스크립트 자체 속성을 통해 이끌어 낼 수 있으므로 script async, defer 속성 사용을 추천드립니다.
https://developer.mozilla.org/ko/docs/Web/HTML/Element/script#defer
https://ko.javascript.info/script-async-defer
<button type="submit" onclick="location.href='/sign'" class="loginclick"> | ||
<span class="error-message"></span> | ||
</div> | ||
<button type="submit" onclick="location.href='items.html'" id="loginclick"> |
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.
P2:
HTML 내부에서 아이디와 클래스 이름의 형식을 통일하는 것이 좋습니다.
지금은 카멜케이스와 케밥케이스가 혼용되어 사용되는 것 같습니다.
클래스명을 케밥케이스로 작성하시니 아이디는 카멜케이스로 작성하시는 것이 더 좋을 것 같네요.
<button type="submit" onclick="location.href='items.html'" id="loginclick"> | |
<button type="submit" onclick="location.href='items.html'" id="loginClick"> |
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.
P1:
함수가 이벤트 바인딩 코드 중간에 있어서 코드 흐름 파악이 어렵습니다.
가능하면 변수 선언 => 함수 선언 => 이벤트 바인딩문
과 같은 순서처럼 관련있는 코드끼리 기준을 정해서 묶어 작성하시는 것을 추천드립니다.
|
||
const loginButton = document.getElementById('loginclick'); | ||
|
||
loginButton.disabled = true; |
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.
P1:
해당 스크립트는 DOMContentLoaded 이벤트가 발생한 후 실행이 되도록 작성하셨습니다.
이런 환경에서 스크립트로 버튼의 disabled 를 조작하면 html 이 파싱되고, script가 다운로드된 후에 해당 코드가 실행되므로
유저가 버튼을 클릭할 수 있게 됩니다.
가능하면 button에 disabled 속성을 추가하시는 걸 추천드립니다.
emailInput.addEventListener('focusout', () => { | ||
const emailValue = emailInput.value.trim(); | ||
if (!emailValue) { | ||
emailError.textContent = '이메일을 입력해 주세요'; | ||
emailInput.classList.add('error'); | ||
|
||
} else if (!/\S+@\S+\.\S+/.test(emailValue)) { | ||
emailError.textContent = '잘못된 이메일입니다'; | ||
emailInput.classList.add('error'); | ||
} else { | ||
emailError.textContent = ''; | ||
emailInput.classList.remove('error'); | ||
} | ||
}); |
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.
P3:
아래 코드도 동일동작합니다.
emailInput.addEventListener('focusout', () => { | |
const emailValue = emailInput.value.trim(); | |
if (!emailValue) { | |
emailError.textContent = '이메일을 입력해 주세요'; | |
emailInput.classList.add('error'); | |
} else if (!/\S+@\S+\.\S+/.test(emailValue)) { | |
emailError.textContent = '잘못된 이메일입니다'; | |
emailInput.classList.add('error'); | |
} else { | |
emailError.textContent = ''; | |
emailInput.classList.remove('error'); | |
} | |
}); | |
emailInput.addEventListener('focusout', () => { | |
const emailValue = emailInput.value.trim(); | |
if (!emailValue) { | |
emailError.textContent = '이메일을 입력해 주세요'; | |
emailInput.classList.add('error'); | |
return; | |
} | |
if (!/\S+@\S+\.\S+/.test(emailValue)) { | |
emailError.textContent = '잘못된 이메일입니다'; | |
emailInput.classList.add('error'); | |
return; | |
} | |
emailError.textContent = ''; | |
emailInput.classList.remove('error'); | |
}); |
@@ -0,0 +1,95 @@ | |||
document.addEventListener('DOMContentLoaded', function() { | |||
const emailInput = document.getElementById('email'); | |||
const emailError = emailInput.nextElementSibling; |
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.
P2:
해당 변수명만 봤을때는 요소의 타입을 알기가 어렵습니다. 에러 메시지처럼 생각되는 변수명이니
조금더 친절한 이름이면 좋을 것 같습니다.
const emailError = emailInput.nextElementSibling; | |
const emailErrorElement = emailInput.nextElementSibling; | |
const emailErrorContainer = emailInput.nextElementSibling; |
요구사항
기본
이메일 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 “이메일을 입력해주세요.” 빨강색 에러 메세지를 보입니다.
이메일 input에서 focus out 할 때, 이메일 형식에 맞지 않는 경우 input에 빨강색 테두리와 아래에 “잘못된 이메일 형식입니다” 빨강색 에러 메세지를 보입니다.
비밀번호 input에서 focus out 할 때, 값이 없을 경우 아래에 “비밀번호를 입력해주세요.” 에러 메세지를 보입니다
비밀번호 input에서 focus out 할 때, 값이 8자 미만일 경우 아래에 “비밀번호를 8자 이상 입력해주세요.” 에러 메세지를 보입니다.
input 에 빈 값이 있거나 에러 메세지가 있으면 ‘로그인’ 버튼은 비활성화 됩니다.
Input 에 유효한 값을 입력하면 ‘로그인' 버튼이 활성화 됩니다.
활성화된 ‘로그인’ 버튼을 누르면 “/items” 로 이동합니다
이메일 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 “이메일을 입력해주세요.” 빨강색 에러 메세지를 보입니다.
이메일 input에서 focus out 할 때, 이메일 형식에 맞지 않는 경우 input에 빨강색 테두리와 아래에 “잘못된 이메일 형식입니다” 빨강색 에러 메세지를 보입니다.
닉네임 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 “닉네임을 입력해주세요.” 빨강색 에러 메세지를 보입니다.
비밀번호 input에서 focus out 할 때, 값이 없을 경우 아래에 “비밀번호를 입력해주세요.” 에러 메세지를 보입니다
비밀번호 input에서 focus out 할 때, 값이 8자 미만일 경우 아래에 “비밀번호를 8자 이상 입력해주세요.” 에러 메세지를 보입니다.
비밀번호 input과 비밀번호 확인 input의 값이 다른 경우, 비밀번호 확인 input 아래에 “비밀번호가 일치하지 않습니다..” 에러 메세지를 보입니다.
input 에 빈 값이 있거나 에러 메세지가 있으면 ‘회원가입’ 버튼은 비활성화 됩니다.
Input 에 유효한 값을 입력하면 ‘회원가입' 버튼이 활성화 됩니다.
활성화된 ‘회원가입’ 버튼을 누르면 “/signin” 로 이동합니다
심화
주요 변경사항
스크린샷
로그인
회원가입
멘토에게
-아이디 창의 조건이 만족되면 비밀번호의 오류 창이 활성화 됩니다.
ㄴfunction으로 연결하지 않고 따로 조건을 넣어서 해결했습니다:)!