Conversation
There was a problem hiding this comment.
중복된 코드가 있더라도 로그인 페이지와 회원가입 페이지 자바스크립트 파일을 각각 나눠주시는게 좋습니다.
실무에서는 로그인 및 회원가입 각각에 더 복잡한 로직이 존재 할 수 있습니다.
| } | ||
|
|
||
| // login eventListener | ||
| email.addEventListener('blur', validateEmail); |
There was a problem hiding this comment.
같은 맥락의 코드는 서로 가깝게 위치시켜주면 가독성을 높여줍니다.
| email.addEventListener('blur', validateEmail); | |
| function validateName() { | |
| removeErrorMessage('nameErrorEmpty', nickname); | |
| if (nickname.value.trim() === "") { | |
| showErrorMessage(nickname, "닉네임을 입력해주세요.", 'nameErrorEmpty'); | |
| } | |
| activateButton(); | |
| } | |
| const email = document.querySelector('#email'); | |
| email.addEventListener('blur', validateEmail); |
There was a problem hiding this comment.
비슷한 역할을 하는 focusout 이벤트가 있습니다. 어떤 차이가 있을까요?
( https://mygumi.tistory.com/321 )
캡쳐링/버블링을 이용해 좀 더 좋은 코드를 작성 할 수 있으므로 focusout을 사용하는게 더 모던한 것 같습니다.
| function showErrorMessage(element, message, errorClass) { | ||
| removeErrorMessage(errorClass, element); | ||
| let errorMessage = document.querySelector(`.${errorClass}`); | ||
| if (!errorMessage) { | ||
| errorMessage = document.createElement('p'); | ||
| errorMessage.textContent = message; | ||
| errorMessage.classList.add('errorColor', errorClass); | ||
| element.classList.add('errorOutline'); | ||
| element.after(errorMessage); | ||
| } | ||
| } | ||
|
|
||
| function removeErrorMessage(errorClass, element) { | ||
| let errorMessage = document.querySelector(`.${errorClass}`); | ||
| if (errorMessage) { | ||
| errorMessage.remove(); | ||
| element.classList.remove('errorOutline'); | ||
| } | ||
| } |
There was a problem hiding this comment.
에러를 보여주는 부분과 제거하는 부분의 인터페이스를 맞추어 함수를 활용하는 점은 좋습니다 !
| let errorMessage = document.querySelector(`.${errorClass}`); | ||
| if (!errorMessage) { | ||
| errorMessage = document.createElement('p'); | ||
| errorMessage.textContent = message; | ||
| errorMessage.classList.add('errorColor', errorClass); | ||
| element.classList.add('errorOutline'); | ||
| element.after(errorMessage); | ||
| } |
There was a problem hiding this comment.
DOM에서 요소를 생성/제거를 반복하는 것보다. DOM 객체를 만들어 놓고 디폴트로 숨긴 다음 에러가 있을때 DOM 객체 보여주는 CSS 클래스를 부여하는 방식이 어떨까요?
DOM에 요소를 추가/제거하는 작업은 CSS조작보다는 리소스가 많이 들어갑니다.
| } else if (!regex.test(email.value)) { | ||
| showErrorMessage(email, "잘못된 이메일 형식입니다.", 'emailErrorForm'); | ||
| } | ||
| activateButton(); |
There was a problem hiding this comment.
activateButton이 각 함수에 반복되고 있는데
input 이벤트와 form 태그에서 버블링 페이즈를 이용하면 1번만 작성 할 수 있습니다.
혹시 해볼 수 있을까요?
네이밍은 잘 하고 계십니다 ! 이대로 유지해주시면 좋습니다 ! |
function validateEmail() {
const regex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
removeErrorMessage('emailErrorEmpty', email); <--- 이부분
removeErrorMessage('emailErrorForm', email); < --- 이부분
if (email.value.trim() === "") {
showErrorMessage(email, "이메일을 입력해주세요.", 'emailErrorEmpty'); < -- 이부분 등
} else if (!regex.test(email.value)) {
showErrorMessage(email, "잘못된 이메일 형식입니다.", 'emailErrorForm');
}
activateButton();
}제가 이해한게 맞다면,
|
굳이 배열을 활용하고자 한다면 각 input 엘레먼트들을 참조한 뒤 간단한 예로 const isAllFieldsWritten = [email.value, nickname.value, password.value, passwordConfirm.value].every((text) => text !== ""); |
|
스프린트 수고 하셨습니다 ! 피드백을 무조건 다음 미션에 적용해야지 ! 라고 부담을 가지기 보다, 한번 쓱 읽어보고 바로 코드로 옮길 수 있는것들부터 차근차근 의식적으로 실행에 옮겨 보세요 ~ |
요구사항
기본
로그인
회원가입
심화
주요 변경 사항
변경 예정 사항
➡️ 스프린트 미션 3 코드 리뷰를 받은 후 아직 수정하지 못한 부분입니다.
➡️ 버튼 태그에 링크 속성을 추가하는 코드를 입력했으나 '로그인' 버튼 클릭 후 해당 페이지로 이동하지 않았습니다.
스크린샷
사이트
https://sprint-mission1-kyj.netlify.app/
멘토에게
스프린트 미션3 후 피드백 받았던 부분은 아직 수정하지 못했습니다. 🙏
함수 이름은 명령어로 짓고 변수 이름은 명사로 짓도록 통일해봤는데 이름을 적절하게 지은 건지 모르겠습니다.
errorMessage를 생성하는 함수와 errorMessage를 지우는 함수를 따로 만든 후 각 input 태그의 유효성을 검사하는 함수에서 앞에 만들어 두었던 함수들을 호출하여 errorMessage를 수정하도록 구현했습니다. 작동은 잘 되는 것 같은데 가독성이 떨어지는 방식인지 접근이 잘못된 건지 궁금합니다.
배열과 배열 메서드를 활용해보고 싶었는데 어디서 사용하면 좋을지 짐작이 안 가 잘 사용하지 못한 것 같습니다. 지금 코드에서 메서드를 활용하면 효율적인 부분이 있는지 궁금합니다.