Conversation
There was a problem hiding this comment.
주동님 이번주 스프린트 미션 고생하셨습니다~
전반적으로 읽기 편한 코드였습니다 😄
- 동일한 사항에 대해서는 한번만 코멘트를 답니다. 비슷한 경우를 찾아서 변경해보세요~
- 기존에 추가되어 있던 이미지들이 있는 것 같은데, 사용하지 않는 이미지들은 삭제해주시면 더 좋을 것 같습니다.
눈 모양 아이콘 클릭시 비밀번호의 문자열이 보이기도 하고, 가려지기도 합니다. 비밀번호의 문자열이 가려질 때는 눈 모양 아이콘에는 사선이 그어져있고, 비밀번호의 문자열이 보일 때는 사선이 없는 눈 모양 아이콘이 보이도록 합니다.: 둘 요구사항 구현하신 것 같습니다~ 관련 리뷰 코멘트 보시고 다시 한번 확인해보세요.- auth.js파일 하나로 login, signup에서의 validation 로직을 같이 처리하셨는데, 공통되는 함수들은 따로 분리하고, login, signUp 로직을 분리하시면 코드파악하기 더 좋을 것 같습니다.
There was a problem hiding this comment.
P3:
빈 페이지가 아니라 나름의 내용이 있는게 재미있네요 😁
| </div> | ||
| </main> | ||
|
|
||
| <script src="scripts/auth.js"></script> |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
P1:
이벤트를 연결하는 코드들이 함수 선언문 사이사이에 들어가 있어서 코드 파악이 어렵습니다.
가능하면 변수 선언 => 함수 선언 => 이벤트 바인딩문 과 같은 순서처럼 관련있는 코드끼리 기준을 정해서 묶어 작성하시는 것을 추천드립니다. 또한 변수 선언은 해당 범위 최상단으로 호이스팅되므로 작업하실때도 최상단에 선언하시는게 코드가독성에도 더 좋습니다~
| const showError = (input, errorId) => { | ||
| const errorElement = document.getElementById(errorId); | ||
| errorElement.style.display = "block"; | ||
| input.style.border = "1px solid #f74747"; | ||
| }; | ||
|
|
||
| const hideError = (input, errorId) => { | ||
| const errorElement = document.getElementById(errorId); | ||
| errorElement.style.display = "none"; | ||
| input.style.border = "none"; | ||
| }; | ||
|
|
||
| const validateEmailString = email => { | ||
| const emailRegex = /^[A-Za-z0-9._%-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4}$/; | ||
| return emailRegex.test(email); | ||
| }; |
There was a problem hiding this comment.
P3:
이런 친구들은 해당 함수 외부로 빼셔도 좋을 것 같습니다.
|
|
||
| const isPasswordVisible = inputField.type === "text"; | ||
|
|
||
| inputField.type = isPasswordVisible ? "text" : "password"; |
There was a problem hiding this comment.
P1:
| inputField.type = isPasswordVisible ? "text" : "password"; | |
| inputField.type = isPasswordVisible ? "password" : "text" ; |
There was a problem hiding this comment.
P2:
어떻게 동작하는지 이해하시고 코드를 짜신 느낌이 듭니다.
다만 코드가 각각의 목적과 상관없이 뭉쳐져 있어서 코드파악 측면에서 좀 아쉬움이 있습니다.
DOMContentLoaded이벤트의 callback 함수에서도 크게 목적별로 분리를 해보자면, 유효성 점검, 에러메시지 토글, 가입하기 혹은 로그인 버튼 disabled props 변경, 이벤트 바인딩 부분으로 구분할 수 있습니다.
이를 좀더 코드에서 알 수 있게 쪼개주시면 좋을 것 같습니다.
요구사항
기본
[x]이메일 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 “이메일을 입력해주세요.” 빨강색 에러 메세지를 보입니다.
[x]이메일 input에서 focus out 할 때, 이메일 형식에 맞지 않는 경우 input에 빨강색 테두리와 아래에 “잘못된 이메일 형식입니다” 빨강색 에러 메세지를 보입니다.
[x]닉네임 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 “닉네임을 입력해주세요.” 빨강색 에러 메세지를 보입니다.
[x]비밀번호 input에서 focus out 할 때, 값이 없을 경우 아래에 “비밀번호를 입력해주세요.” 에러 메세지를 보입니다
[x]비밀번호 input에서 focus out 할 때, 값이 8자 미만일 경우 아래에 “비밀번호를 8자 이상 입력해주세요.” 에러 메세지를 보입니다.
[x]비밀번호 input과 비밀번호 확인 input의 값이 다른 경우, 비밀번호 확인 input 아래에 “비밀번호가 일치하지 않습니다..” 에러 메세지를 보입니다.
[x]input 에 빈 값이 있거나 에러 메세지가 있으면 ‘회원가입’ 버튼은 비활성화 됩니다.
[x]Input 에 유효한 값을 입력하면 ‘회원가입' 버튼이 활성화 됩니다.
활성화된 ‘회원가입’ 버튼을 누르면 “/signin” 로 이동합니다.
심화
주요 변경사항
스크린샷
멘토에게