[이서정] Sprint22 스프린트 미션 4 #45
Conversation
ByungyeonKim
left a comment
ByungyeonKim
left a comment
There was a problem hiding this comment.
안녕하세요, 서정님!
Basic에서의 마지막 미션을 완료하셨네요. 🥳
이번 미션은 유효성 검사를 통한 JS 로직이 중요하죠!
그에 맞는 고민을 많이 하신 것 같아요.
피드백 시작하겠습니다!
기능에 맞게 분리하려고 노력했는데 잘한건지 잘 모르겠습니다.
중복 코드를 피하려고 했지만 잘 작성한건지 모르겠습니다.
: 좋은 고민이에요. 하나의 목적을 갖는 함수로 분리하려고 노력하신 것 같아요. 하지만, 중복 코드를 지나치게 경계하고, 너무 많이 분리를 해도 오히려 찢어진 종이 조각처럼 더 보기 어려운 경우도 생겨요.
현재 코드에서 이벤트 위임도 활용하시고, 객체를 통해 재사용성을 늘리신 것은 너무나 잘하셨습니다! 자세한 개선 사항들은 아래 코멘트를 참고해주세요.
const isLoginPage = authForm.classList.contains('auth-login-form'); 이렇게 로그인/회원가입을 분기해서 한 파일에서 처리했는데 이 방식이 괜찮은지 궁금합니다.
: 저는 이정도 범위에서는 나쁘지 않다고 봅니다! isLoginPage 변수도 함수 내에서 계속 재정의 되는 게 아니라, 외부 스코프에서 한 번만 정의하도록 둔 것도 좋았어요.
현재는 에러메세지를 에러 발생 시 DOM을 생성/삭제하는 방식으로 구현했는데, 에러 태그를 HTML 파일에 미리 구현하고 메시지만 바꾸는 방식이 더 나을 거 같다는 생각이 들었습니다. 어떤 방식이 더 좋을지 궁금합니다.
: 에러가 발생했을 때만 DOM을 생성/제거하는 방식이 훨씬 명확하고 좋다고 생각해요. 앞으로 진행할 리액트의 철학과도 맞구요. 미리 DOM을 생성해놓는 것은 접근성 측면에서 좋지 않을 수 있고, 에러가 발생하지 않았을 때도 메모리를 차지해요.
현재 프로젝트에서는 성능 차이가 미미하겠지만, 입력 필드가 많아질 수록 차이는 커집니다!
지난 피드백도 잘 반영해주셨네요. 💯
Tab width를 2로 설정해보니 어떤가요? 확실히 가로 스크롤이 줄어들지 않나요? 😁
이번 미션도 고생하셨습니다, 서정님!
다음 리액트 미션에서 뵐게요! 👏
There was a problem hiding this comment.
오..이렇게 상수값을 따로 분리해서 관리하시는 분은 이번 기수에서 처음 봐요. 경험이 있으셔서 그런지, 미션에서도 잘 활용하고 계시네요. 👍
정확한 뜻이 없는 숫자를 매직 넘버라고 하죠. PASSWORD_MIN_LEN 변수처럼 이름을 지어주고, 매직 넘버를 없애주는 것도 좋은 코드 습관입니다!
| @@ -1,159 +1,185 @@ | |||
| body { | |||
| font-family: 'Pretendard', sans-serif; | |||
| font-family: 'Pretendard Variable', sans-serif; | |||
There was a problem hiding this comment.
지난 피드백을 잘 반영해주셨네요 좋습니다.
| const isRequired = (value, fieldName) => | ||
| value.trim() === '' ? `${fieldName}을 입력해주세요.` : ''; | ||
|
|
||
| const emailField = (value) => { | ||
| const requiredError = isRequired(value, '이메일'); | ||
| if (requiredError) return requiredError; | ||
| return EMAIL_REGEX.test(value) ? '' : '잘못된 이메일 형식입니다.'; | ||
| }; | ||
|
|
||
| const nameField = (value) => { | ||
| const requiredError = isRequired(value, '닉네임'); | ||
| if (requiredError) return requiredError; | ||
| return NICKNAME_REGEX.test(value) ? '' : '잘못된 닉네임 형식입니다.'; | ||
| }; | ||
|
|
||
| const passwordField = (value) => { | ||
| const requiredError = isRequired(value, '비밀번호'); | ||
| if (requiredError) return requiredError; | ||
|
|
||
| if (value.length < PASSWORD_MIN_LEN) { | ||
| return `비밀번호를 ${PASSWORD_MIN_LEN}자 이상 입력해주세요.`; | ||
| } | ||
|
|
||
| if (!isLoginPage && !PASSWORD_REGEX.test(value)) { | ||
| return '잘못된 비밀번호 형식입니다.'; | ||
| } | ||
|
|
||
| return ''; | ||
| }; |
There was a problem hiding this comment.
함수와 변수의 네이밍 규칙을 일관성 있게 만들면 더 좋을 것 같아요! updateSubmitButton 함수처럼 네이밍을 동사처럼 만들어보는 건 어떨까요?
| const fieldConfig = { | ||
| email: { input: emailInput, validator: emailField }, | ||
| ...(nicknameInput && { | ||
| nickname: { input: nicknameInput, validator: nameField }, | ||
| }), | ||
| password: { input: passwordInput, validator: passwordField }, | ||
| ...(passwordCheckInput && { | ||
| password_check: { | ||
| input: passwordCheckInput, | ||
| validator: (value) => | ||
| passwordCheckField(value, passwordInput.value), | ||
| }, | ||
| }), | ||
| }; |
There was a problem hiding this comment.
로직을 좀 더 단순화 해볼 수 없을까요? 지금처럼 nicknameInput 프로퍼티의 유무를 바로 결정하지 말고 초기 상태로써 존재하게 둔 다음, 함수 내에서 상태를 바꿔보는 것도 나쁘지 않을 것 같아요.
| crossorigin | ||
| href="https://cdn.jsdelivr.net/gh/orioncactus/[email protected]/dist/web/variable/pretendardvariable-dynamic-subset.min.css" | ||
| /> | ||
| <script defer type="module" src="../js/auth.js"></script> |
There was a problem hiding this comment.
type="module" 적용 시 defer처럼 동작합니다! 🤓
2 participants
배포 링크
https://snazzy-baklava-3e5b7c.netlify.app/
진행한 미션 버전
미션 요구사항
기본
로그인
회원가입
심화
주요 변경사항 및 학습 포인트
주강사에게
집중적으로 봐줬으면 하는 부분
해결하지 못한 문제 / 궁금한 점
const isLoginPage = authForm.classList.contains('auth-login-form');이렇게 로그인/회원가입을 분기해서 한 파일에서 처리했는데 이 방식이 괜찮은지 궁금합니다.