-
Notifications
You must be signed in to change notification settings - Fork 26
[김해빈] sprint4 #74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[김해빈] sprint4 #74
The head ref may contain hidden characters: "Basic-\uAE40\uD574\uBE48-sprint4"
Conversation
|
첫 js 미션 수행하느라 고생 많으셨습니다!👍👍 질문에 대한 답변은 아래 내용 참고 부탁드립니다.
이런 이슈는 보통 placeholder 글자 유무, 입력 폰트 크기/줄높이, 또는 input padding 변화 때문입니다. CSS에서 눈 아이콘을 absolute로 위치시키고, input 요소를 기준으로 항상 정중앙에 위치하도록 고정해주면 해결될거에요. 아래 코드 참고해보세요!
기본적으로
꼭 그런 건 아니지만 대부분의 반응형 처리는 여전히 미디어 쿼리 기반으로 구건별로 작성하게 됩니다.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validateEmail, validatePassword, validateConfirmPassword 등 기능별 함수 분리가 잘 되어있네요! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로그인과 회원가입에서 validateEmail, validatePassword, createErrorMessage, clearErrorMessage 함수가 거의 동일하게 반복되고 있습니다.
공통 유효성 함수 & UI 헬퍼 함수는 formUtils.js 등의 별도 파일로 분리해서 재사용하도록 구조화하면 좋을 것 같습니다.
| } | ||
|
|
||
| //로그인 버튼 비활성화,활성화 | ||
| function checkFormValidity(showMessage = true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
showMessage 인자가 필요 없는 함수로 보입니다. 혼란의 여지가 있으니 제거해 주세요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
또한 해당 함수는 “검증” 과 “UI 처리” 를 동시에 하고있어서 역할이 모호해지는 것 같습니다.
showMessage가 true일 때는 UI까지 다루고, false일 땐 검증만 한다면 검증 함수인지 UI 컨트롤러인지 구분하기가 어렵습니다.
단일 책임 원칙(SRP)을 지키기 위해 함수가 분리되면 좋을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
대략 요런 구조로 수정되면 좋을 것 같네요.
function isEmailValid(value) {
const regex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
return regex.test(value);
}
function validateEmailField(input) {
const value = input.value.trim();
if (!value) {
showError(input, '이메일을 입력해주세요.');
return false;
}
if (!isEmailValid(value)) {
showError(input, '잘못된 이메일 형식입니다.');
return false;
}
clearError(input);
return true;
}
| return false; | ||
| } | ||
| if(passwordValue !== confirmValue) { | ||
| if (showMessage) createErrorMessage(confirmPasswordInput, '비밀번호를 일치하지 않습니다.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단순 오타가 있습니다.
"비밀번호를 일치하지 않습니다" → "비밀번호가 일치하지 않습니다"
요구사항
기본
로그인
회원가입
체크리스트 [심화]
스크린샷
사진 1 :
사진 2 :
멘토에게