-
Notifications
You must be signed in to change notification settings - Fork 37
[이민서] Sprint4 #152
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 #152
The head ref may contain hidden characters: "Basic-\uC774\uBBFC\uC11C-sprint4"
Conversation
kich555
left a comment
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.
고생하셨습니다~!
| removeEventListeners(); // 폼 제출 시 이벤트 제거 | ||
| window.location.href = "/index.html"; |
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.
window.location.href = "/index.html" 를 사용하면 어차피 새로운 페이지로 넘어가면서 기존 페이지의 스크립트와 이벤트 리스너는 자동으로 해제됩니다. 때문에 removeEventListeners()를 굳이 할 필요가 있을까? 싶긴 하네요 ㅎㅎ
하지만 연습이라는 의미에서는 좋습니다~
| function removeEventListeners() { | ||
| if (emailInput) { | ||
| emailInput.removeEventListener("focusout", checkEmailValidity); | ||
| } | ||
| if (nicknameInput) { | ||
| nicknameInput.removeEventListener("focusout", checkNicknameValidity); | ||
| } | ||
| if (passwordInput) { | ||
| passwordInput.removeEventListener("focusout", checkPasswordValidity); | ||
| passwordInput.removeEventListener("input", checkPasswordValidity); | ||
| } | ||
| if (passwordConfirmationInput) { | ||
| passwordConfirmationInput.removeEventListener("focusout", checkPasswordConfirmationValidity); | ||
| passwordConfirmationInput.removeEventListener("input", checkPasswordConfirmationValidity); | ||
| } | ||
| } | ||
|
|
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 removeEventListeners() {
const inputs = [
{ element: emailInput, events: ["focusout"], handler: checkEmailValidity },
{ element: nicknameInput, events: ["focusout"], handler: checkNicknameValidity },
{ element: passwordInput, events: ["focusout", "input"], handler: checkPasswordValidity },
{ element: passwordConfirmationInput, events: ["focusout", "input"], handler: checkPasswordConfirmationValidity },
];
inputs.forEach(({ element, events, handler }) => {
if (!element) return;
events.forEach((eventType) => {
element.removeEventListener(eventType, handler);
});
});
}| const loginForm = document.querySelector('.login-form'); | ||
| const signupForm = document.querySelector('.signup-form'); | ||
| const emailInput = document.querySelector('#email'); | ||
| const nicknameInput = document.querySelector('#nickname'); | ||
| const passwordInput = document.querySelector('#password'); | ||
| const passwordConfirmationInput = document.querySelector('#passwordConfirmation'); | ||
| const submitButton = document.querySelector('button[type="submit"]'); | ||
| const loginForm = document.querySelector(".login-form"); | ||
| const signupForm = document.querySelector(".signup-form"); | ||
| const emailInput = document.querySelector("#email"); | ||
| const nicknameInput = document.querySelector("#nickname"); | ||
| const passwordInput = document.querySelector("#password"); | ||
| const passwordConfirmationInput = document.querySelector("#passwordConfirmation"); | ||
| const submitButton = document.querySelector("button[type='submit']"); |
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.
이런 부분은 커밋 이름인 feat(auth): 비밀번호 표시/숨기기 아이콘 토글 기능 추가 과 연관성이 없고 사전에 self-review를 진행했다면 나오지 않을 불필요한 변경점 인 것 같아요 ㅎ
뭐 크리티컬하게 문제가 있는 부분은 아니긴한데 커밋을 잘 통제할 수 있도록 git 다루는 연습을 미리 해두시면 좋을것 같아요!
| toggleIcon.src = isPasswordVisible ? "/images/icons/eye-invisible.svg" : "/images/icons/eye-visible.svg" | ||
| toggleIcon.alt = isPasswordVisible ? "비밀번호 숨김 상태 아이콘" : "비밀번호 표시 상태 아이콘"; |
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.
HTML에서 이렇게 data- 속성을 지정하면 js에서 이렇게 간편하게 쓸 수 있어요~
<button class="password-toggle-button"
data-visible-icon="/images/icons/eye-visible.svg"
data-invisible-icon="/images/icons/eye-invisible.svg"
data-visible-alt="비밀번호 표시 상태 아이콘"
data-invisible-alt="비밀번호 숨김 상태 아이콘">
<img class="password-toggle-icon" src="/images/icons/eye-invisible.svg" alt="비밀번호 숨김 상태 아이콘">
</button>JS
// 데이터 속성에서 아이콘 및 대체 텍스트 가져오기
const visibleIcon = button.dataset.visibleIcon;
const invisibleIcon = button.dataset.invisibleIcon;
const visibleAlt = button.dataset.visibleAlt;
const invisibleAlt = button.dataset.invisibleAlt;
요구사항
기본
로그인
회원가입
심화
주요 변경사항
스크린샷
멘토에게