[이지현] Sprint4 #155
[이지현] Sprint4 #155
Conversation
- 'pages' 폴더를 생성하고 폴더 구조 개선을 위해 관련 파일들 정리 - 'signup.html'과 'login.html'외에도 추가적인 HTML 파일들을 'pages' 폴더로 이동
- 이메일 포커스 아웃 시 유효성 검사 추가 - 유효하지 않은 이메일 형식일 경우 빨간색 테두리 표시 - 유효한 이메일일 경우 에러 메시지 제거
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
지현님 4번째 미션 제출 고생하셨습니다.
코드를 잘 정리해주셔서 가독성이 좋았습니다.
다만 회원가입 페이지에서 비밀번호 입력 후 비밀번호 확인 입력하고 다시 비밀번호를 변경하면 비밀번호 확인 인풋은 재확인 로직을 실행하지 않네요~ 한번 다양한 케이스를 테스트해보세요.
다음 React 미션도 화이팅입니다~
코드가 지저분한 건지 아닌 건지 잘 모르겠는데 이것에 대한 관리하는 기준이 따로 있을까요?: 지금은 혼자서 작업하시지만 추후 다른 분들과 작업하게 되면 프로젝트 자체적으로 일관성을 위한 규칙을 정하고 코드를 작성합니다. 그럴때는 정해진 룰에 맞춰 작업하시면되고, 지금에서 좋은 코드를 작성하고 싶으시다면 eslint 같은 툴을 통해 기존 유명한 코드 규칙중 마음에 드시는 것을 골라 따라보시면 좋습니다.
| @@ -0,0 +1,57 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="kr"> | |||
There was a problem hiding this comment.
❗️ 수정요청
html lang 속성은 스크린리더, 검색 엔진 등에게 해당 사이트가 어떤 언어 기반으로 작성되었는지 설명하는 속성입니다.
해당 속성에는 특정 언어들을 의미하는 예약어가 들어가야 합니다. 예를들어 en은 english 즉 영어를 의미합니다.
저희 사이트는 한국어 기반이므로 ko를 써야합니다~
| <html lang="kr"> | |
| <html lang="ko"> |
| <script src="../scripts/loginValidation.js"></script> | ||
| <script src="../scripts/loginTogglePassword.js"></script> |
There was a problem hiding this comment.
💊 제안
아마 script 태그를 HTML의 하단에 배치하신 이유가 script가 문서의 렌더링을 막지 않도록 하기 위해서이실 것 같아요.
하지만, script 태그에 defer나 async 속성을 사용하면 이런 문제를 해결할 수 있기 때문에 반드시 하단에 배치할 필요는 없습니다!
또한 script 태그는 상단에 있는게 구조 파악에서도 유리하기 때문에 상단 head 태그에 두시는 것을 추천드려요~
지금과 같은 경우 DOM을 조작하는 JS 이니 defer 속성을 추가하시면 되겠습니다~
| function showPassword(input, img) { | ||
| input.type = 'text'; | ||
| img.src = '../assets/icons/show-password.png'; | ||
| toggle = false; | ||
| } | ||
|
|
||
| function hidePassword(input, img) { | ||
| input.type = 'password'; | ||
| img.src = '../assets/icons/hide-password.png'; | ||
| toggle = true; | ||
| } |
There was a problem hiding this comment.
💊 제안
signupTogglePassword 파일의 함수들과 동일한 함수네요.
해당 함수를 한번만 정의해서 두 파일에서 같이 사용할 수 있게 하시면 더 좋을 것 같아요~
이는 다른 로직들도 동일합니다. 같은 로직의 경우 하나를 재사용하는 것이 유지보수 측면에서도 좋으니 auth.css를 만드셨던 것처럼 두 파일의 중복을 줄이는 법도 고민해보세요~
| const passwordInput = document.querySelector('#pwd'); | ||
| const passwordBtn = document.querySelector('.pwd-icon-btn'); | ||
| const toggleImg = document.querySelector('.pwd-icons'); |
There was a problem hiding this comment.
❗️ 수정요청
해당 변수는 passwordBtn 이벤트 바인딩 문에서만 사용되므로 전역변수로 선언하실 필요가 없을 것 같아요.
const passwordBtn = document.querySelector('.pwd-icon-btn');
passwordBtn.addEventListener('click', function (e) {
const passwordInput = document.querySelector('#pwd');
const toggleImg = document.querySelector('.pwd-icons');
e.preventDefault();
if(toggle) {
showPassword(passwordInput, toggleImg);
} else {
hidePassword(passwordInput, toggleImg);
}
});| document.documentElement.style.setProperty('--red', '#F74747'); | ||
| document.documentElement.style.setProperty('--gray400', '#9CA3AF'); |
There was a problem hiding this comment.
💊 제안
JS로 색상 변수를 추가하시기보다 variables.css에 추가해서 사용하시는 것을 추천드려요!
스타일관련 코드들은 JS보다 CSS에 있는 것이 유지보수 측면에서도 좋습니다~
| document.documentElement.style.setProperty('--gray400', '#9CA3AF'); | ||
|
|
||
| // 페이지 로드 시 버튼을 비활성화 | ||
| submitButton.disabled = true; |
There was a problem hiding this comment.
❗️ 수정요청
JS 가 아니라 html 에서 button에 disabled 속성을 true로 주고 시작하는게 더 좋을 것 같아요.
JS에서 이렇게 해야할 이유를 잘 모르겠고 이렇게 되면 HTML 에서 button이 활성화된채로 렌더링됩니다~
| if (validationMessage) { | ||
| messageElement.textContent = validationMessage; | ||
| inputElement.style.border = '1px solid var(--red)'; | ||
| return false; | ||
| } else { | ||
| messageElement.textContent = ''; | ||
| inputElement.style.border = 'none'; | ||
| return true; | ||
| } |
There was a problem hiding this comment.
❗️ 수정요청
if문에서 return문으로 조건 만족시 해당 함수가 조기 종료되므로 else 문은 없어도 됩니다~
| if (validationMessage) { | |
| messageElement.textContent = validationMessage; | |
| inputElement.style.border = '1px solid var(--red)'; | |
| return false; | |
| } else { | |
| messageElement.textContent = ''; | |
| inputElement.style.border = 'none'; | |
| return true; | |
| } | |
| if (validationMessage) { | |
| messageElement.textContent = validationMessage; | |
| inputElement.style.border = '1px solid var(--red)'; | |
| return false; | |
| } | |
| messageElement.textContent = ''; | |
| inputElement.style.border = 'none'; | |
| return true; |
| } | ||
|
|
||
| function validateEmail(check_email) { | ||
| const email_regex = /^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,4}$/i; |
There was a problem hiding this comment.
💊 제안
loginValidation 파일에서도 동일한 정규식을 사용하고 있는데 그때마다 선언해서 쓰면 문제가 있을 것 같아요.
정규식을 반복해서 선언하면 유지보수가 어려워지고, 코드의 일관성이 떨어질 수 있습니다. 만약 정규식을 수정해야 할 때 여러 곳에서 변경해야 하고 실수로 서로 다른 정규식을 사용할 위험도 커집니다.
이렇게 두개의 파일에서 사용하고 있으니 해당 정규식과 test 함수를 따른 파일로 분리해 사용하시는 것을 추천드려요!
2 participants
요구사항
기본
로그인
이메일 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 에 유효한 값을 입력하면 '회원가입' 버튼이 활성화 됩니다.
활성화된 '회원가입' 버튼을 누르면 로그인 페이지로 이동합니다
심화
주요 변경사항
스크린샷
멘토에게