-
Notifications
You must be signed in to change notification settings - Fork 35
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 #115
The head ref may contain hidden characters: "Basic-\uC774\uD615\uC900-sprint4"
[이형준]sprint4 #115
Conversation
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.
JavaScript를 사용하여 코드를 처음 작성하였습니다. 모듈 분할을 어떤 기준으로 해야할지 잘 모르겠습니다.
-> 지금 만드신 구조에서는 pwInputCheck
함수처럼 valid 체크를 위한 유틸들을 별도 validation.js 와 같은 파일로 빼는 방법을 하면 좋을 것 같아요.
if(loginButton !== null) { | ||
if(emailIsValid && pwIsValid) { | ||
loginButton.disabled = false; | ||
} else { | ||
loginButton.disabled = 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.
위 코드를 아래처럼 바꿀 수 있어요!
- 동일하게 동작하는데, 중복코드를 줄일 수 있어요.
- 유효성 검사 하는 함수를 별도로 분리해서 재사용성도 높일 수 있습니다.
/**
* 로그인 버튼 유효성 검사
* @returns {boolean}
*/
function isLoginButtonValid() {
return emailIsValid && pwIsValid;
}
/**
* 회원가입 버튼 유효성 검사
* @returns {boolean}
*/
function isSignupButtonValid() {
return emailIsValid && pwIsValid && nicknameIsValid && pwRetypeIsValid;
}
export function buttonDisableChecker(event) {
if (loginButton) {
loginButton.disabled = !isLoginButtonValid();
}
if (signupButton) {
signupButton.disabled = !isSignupButtonValid();
}
}
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.
추가적으로 함수를 작성했을 때, 타입스크립트를 아직 사용하지 않다보니 아래처럼 JSDoc을 통해 주석을 추가해주시면 좋아요.
/**
* 로그인 버튼 유효성 검사
* @returns {boolean}
*/
function isLoginButtonValid() {
return emailIsValid && pwIsValid;
}
관련 링크는 남겨놓을게요!
https://jsdoc.app/about-getting-started
} | ||
} | ||
|
||
if(signupButton !== null) { |
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.
이렇게 if 문 안에서 !== null 대신 truthy 값을 사용하여 조건을 단순화할수 있어요
if (signupButton) {
signupButton.disabled = !isSignupButtonValid();
}
export function emailInputCheck(event) { | ||
const target = event.target; | ||
const errorMessage = document.querySelector('#email-error-message'); | ||
|
||
if (target.value === "") { | ||
target.classList.add('error-input'); | ||
errorMessage.textContent = '이메일을 입력해주세요.'; | ||
emailIsValid = false; | ||
} | ||
// checkValidity() : true ("" or type match), false (different type) | ||
if (!target.checkValidity()) { | ||
target.classList.add('error-input'); | ||
errorMessage.textContent = '잘못된 이메일 형식입니다.'; | ||
emailIsValid = false; | ||
} | ||
if (target.checkValidity() && target.value !== "") { | ||
target.classList.remove('error-input'); | ||
errorMessage.textContent = ''; | ||
emailIsValid = 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.
emailInputCheck
함수 외에도 아래에 있는 validation 역할을 해주는 함수들 중 중복 코드들이 보여요.
이를 해결하기 위해, classList.toggle
사용해서 가독성 있게 해결 해볼 수 있어요.
아래 방법을 nicknameInputCheck, pwInputCheck, pwRetypeInputCheck 함수에 똑같이 적용해주시면 됩니다.
/**
* 에러 메시지 설정 및 클래스 토글 함수
* @param {HTMLElement} target
* @param {HTMLElement} errorMessage
* @param {string} message
* @param {boolean} isValid
*/
function setErrorMessage(target, errorMessage, message, isValid) {
target.classList.toggle('error-input', !isValid);
errorMessage.textContent = message;
}
export function emailInputCheck(event) {
const target = event.target;
const errorMessage = document.querySelector('#email-error-message');
if (!target.value) {
setErrorMessage(target, errorMessage, '이메일을 입력해주세요.', false);
emailIsValid = false;
} else if (!target.checkValidity()) {
setErrorMessage(target, errorMessage, '잘못된 이메일 형식입니다.', false);
emailIsValid = false;
} else {
setErrorMessage(target, errorMessage, '', true);
emailIsValid = true;
}
}
https://developer.mozilla.org/ko/docs/Web/API/Element/classList
</div> | ||
</div> | ||
<button>로그인</button> | ||
<button id="login-button" onclick="location.href='/pages/items'" disabled="true">로그인</button> |
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.
disabled
만 적어주셔도 됩니다! 기본 값이 true
이기 때문이에요~
pw_visibility.js 에서 input태그를 타겟팅 하기 위해서 const input = img.parentElement.parentElement.firstElementChild; -> parentElement와 firstElementChild 사용보다는 각 요소들에 클래스를 통해 접근하는 방법이 좋습니다. 이렇게 진행하면 HTML 구조를 변경해도 유지보수에 이슈가 없을거에요. 예를들어 아래처럼 각 클래스를 추가를 하는게 좋아요.
이후
|
main.js 에서 login page와 signup page의 요소가 달라서 없는 요소에 addEventListener를 이용하여 이벤트를 등록하는 경우 null값이 되어 에러가 발생했습니다. 그래서 if문을 이용해서 처리했는데 다른 좋은 방법이 있을까요? -> 아래처럼 요소 값 체크 하는 함수를 하나 만들어서 해당 함수로 랩핑하는 방법은 어떨까요?
|
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.
과제 제출하시느라 수고 많으셨습니다 🙏
요구사항
기본 요구사항
체크리스트 (기본)
로그인
회원가입
체크리스트 (심화)
피드백 반영
주요 변경사항
사이트 배포 링크
스크린샷
멘토에게
const input = img.parentElement.parentElement.firstElementChild;
이렇게 작성 하였습니다. 다른 좋은 방법이 있을까요? 제가 생각할때는 html에 변경사항이 생기거나 하면 위와 같이 작성할때 문제가 될 것 같고, 그렇다고 id를 이용해서 요소를 찾아내기에는 비밀번호를 보이게 하는 것과 비밀번호 확인을 보이게 하는 것에 두개의 함수가 필요한 것 같습니다..