Conversation
| @@ -0,0 +1,41 @@ | |||
| //모든 input의 유효성 검사를 통과했을 때에만 누를 수 있어야하므로 validate와 작동방식이 달라야함 | |||
| export function switchBtnStatus() { | |||
There was a problem hiding this comment.
switch보다는 update나 toggle이 더 적합한 네이밍이 아닐까싶네요!
switchBtnStatus => toggleBtnStatus
| btn.classList.remove("btn--disabled"); | ||
| } | ||
|
|
||
| export function switchVisibility(event) { |
There was a problem hiding this comment.
이 함수 네이밍도 다수의 정해진 상태에서 왔다갔다하는게 아닌 Y/N 두가지에 해당하는 상태를 가지고있으니 toggle이 더 나은 네이밍일것같아요 :)
There was a problem hiding this comment.
그리고 내부 로직이 UI와 결합되어있어, 다소 재사용성이 떨어질것같은데 지금과 같이 모듈화하는것보다는 페이지별로 실행되는 js파일에 함수를 만드는게 좋겠네요 :)
There was a problem hiding this comment.
그리고 내부 로직이 UI와 결합되어있어, 다소 재사용성이 떨어질것같은데 지금과 같이 모듈화하는것보다는 페이지별로 실행되는 js파일에 함수를 만드는게 좋겠네요 :)
저는 switchVisibility 함수가 비밀번호를 요구하는 모든 폼에서 사용할 수 있다고 생각해서 재사용 가능성이 높다고 생각했는데, UI와 결합되어 있어 재사용성이 떨어진다는 부분이 잘 와닿지 않습니다ㅠㅠ 해당 함수 코드 중 어느 부분이 그런 성격을 가지고 있는지 풀어서 설명해주시면 좋겠습니다. 그리고 해당 피드백이 switchBtnStatus에도 적용되는 것인지 궁금합니다.
| } from "./validate.js"; | ||
| import { switchBtnStatus, switchVisibility } from "./btn-functions.js"; | ||
|
|
||
| function isSignupPage() { |
There was a problem hiding this comment.
함수이름은 명사형보다는 동사형으로 지어주는게(동사로 시작하는게) 일반적입니다.
네이밍 컨벤션 참고해볼까요?
참고
There was a problem hiding this comment.
단순히 signup 페이지인지 여부만 판단하는 일이라면, 함수를 따로 만들지않고 변수로 만들어서 삼항연산자를 사용해주면 깔끔할것같네요! :)
| form.addEventListener("focusout", (event) => { | ||
| const { id } = event.target; | ||
|
|
||
| if (validateMap[id]) { | ||
| validateMap[id](event); | ||
| //비밀번호확인란부터 입력후 비밀번호를 변경할 시 비밀번호 확인 유효성 검사 | ||
| if (id === "password" && isSignupPage()) { | ||
| const passwordConfirmInput = document.getElementById("password-confirm"); | ||
| const passwordConfirm = passwordConfirmInput.value; | ||
| if (!isEmpty(passwordConfirm)) { | ||
| validatePasswordConfirmReverse(); | ||
| } | ||
| } | ||
| switchBtnStatus(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
우선 validateMap을 만들어 각 필드마다 유효성검사를 수행하는 함수를 사용해 코드를 더 간결하게 만든건 아주 잘하셨습니다! 👍 다만 비밀번호 확인의 경우, 비밀번호 확인 함수 내부에서 비밀번호 확인란에 값이 있는지 체크하고 유효성 검사를 실행하는 로직을 써주지않고, 이 함수에서 꼭 조건문을 써줘야하는 필연적인 이유가 보이진 않네요 :)
There was a problem hiding this comment.
만약 함수가 하는 일을 좀 더 세부적으로 쪼개서 사용해보고싶었던거라면, 이런식으로 나눠보시는게 더 나을것같아요.
- 유효성 검사하기: validate()
- 에러 상태 토글하기: toggleError()
- 에러 상태 없애기: clearError()
각 요소를 핸들링하는 함수에서
- clearError() 실행
- validate() 실행
- toggleError() 실행
이런식으로 해야하는 일들을 순서대로 보여주게하면 좀 더 흐름이 좋아지고 개별적으로 테스트하기도 용이해져서 유지보수에 좋겠죠? :)
또한 중간에 추가적으로 핸들링해줘야하는 작업이 또 생긴다면 얼마든지 확장도 가능해질겁니다! :)
| } | ||
| } | ||
|
|
||
| export function validateEmail(event) { |
There was a problem hiding this comment.
email, password, password confirm, nickname 지금은 이렇게 네개지만 지금과 같은 구조라면 폼 요소가 더 늘어날때 작성되는 코드양도 같이 늘어나겠죠?
재사용 가능한 단위까지 로직을 쪼개서 단일 책임을 가지고있는 함수를 만들면 어떨까요?
코드 예시를 드려볼게요 :)
- 유효성 검사 규칙
export const validators = {
email: (value) => {
if (!value) return { isValid: false, message: "이메일을 입력해주세요" };
if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(value)) {
return { isValid: false, message: "올바른 이메일 형식이 아닙니다" };
}
return { isValid: true, message: "" };
},
password: (value) => {
if (!value) return { isValid: false, message: "비밀번호를 입력해주세요" };
if (value.length < 8) {
return { isValid: false, message: "비밀번호는 8자 이상이어야 합니다" };
}
return { isValid: true, message: "" };
},
};- 유효성 검사 수행 및 UI 상태 업데이트
const validateInput = (input, errorMsgElement, validatorType) => {
const { isValid, message } = validators[validatorType](input.value);
errorMsgElement.innerText = message;
// 스타일 업데이트
input.classList.remove("success", "error");
errorMsgElement.classList.remove("error");
if (isValid) {
input.classList.add("success");
} else {
input.classList.add("error");
errorMsgElement.classList.add("error");
}
return isValid;
};
질문에 대한 답변
파일을 분리하기보다는, 비즈니스 로직과 UI를 먼저 분리해보고 재사용 가능한 단위로 함수의 작업을 쪼개보시는 연습을 해보시면 좋을것같아요 :) |
요구사항
배포
https://pandamarket-sprint-yjh.netlify.app/
기본
로그인
회원가입
심화
주요 변경사항
-폼 태그 input요소 유효성 검사 기능
-비밀번호 표시 기능
스크린샷
멘토에게