-
Notifications
You must be signed in to change notification settings - Fork 31
[고서영] Sprint4 #131
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 #131
The head ref may contain hidden characters: "Basic-\uACE0\uC11C\uC601-sprint4"
Conversation
dongqui
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.
서영님! 요구 사항에 맞게 잘 구현해 주셨습니다! 👍 변수명, 함수명도 명확해서 보기 좋았습니다 :) 일부 로직을 함수로 추상화 시켜주시면 더욱 좋을 거 같습니다~!
질문 주신 거는 리뷰 참고해 주세요~
| const form = document.querySelector(".login-form"); | ||
| const submitBtn = document.querySelector(".btn-large"); | ||
|
|
||
| submitBtn.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.
html 속성을 이용하실 수 있습니다~!
<button disabled>로그인</button>| if (!emailTouched) return true; | ||
| const emailValue = emailInput.value.trim(); | ||
| if (!emailValue) { | ||
| emailError.textContent = "이메일을 입력해 주세요."; |
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 관련 부분이 반복되고 있네요! 이런 경우 함수로 묶어 주시면 유지보수, 가독성에 도움이 됩니다 :)
| emailInput.classList.add("error"); | ||
| return false; | ||
| } | ||
| emailError.textContent = ""; // 에러 메시지 초기화 |
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.
에러 제거 또한 함수로 정의할 수 있겠죠!
| <link rel="stylesheet" href="./styles/components/footer.css" /> | ||
|
|
||
| <!-- script --> | ||
| <script src="./login.js"></script> |
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.
signup.html 과 login.html에 같은 컴포넌트가 있어 동시 적용하려고 sign.js 하나의 파일에 작성하려고 했으나 잘 동작이 안되서 따로 만들었습니다. 혹시 같은 컴포넌트를 작성하고 임포트 하는 방법이 있나요?
-> 바닐라 자바스크립트에서 import를 쓰실려면 <script type="module" src="./login.js"></script> 로 써주시거나 리엑트 쓰실 때 처럼 웹팩같은 번들러를 활용하셔야 합니다 :)
https://developer.mozilla.org/ko/docs/Web/JavaScript/Guide/Modules
| const isPasswordValid = validatePassword(); | ||
|
|
||
| if (!isEmailValid || !isPasswordValid) { | ||
| event.preventDefault(); // 폼 제출 방지 |
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.
form 의 method=post 때문에 잘 동작이 안되서 event.preventDefault() 를 하고 /item으로 가도록 작성했는데 이에 문제점이 많을 것 같습니다. 여기에 대한 피드백 부탁드립니다.
-> 문제 될 거 없습니다! 요즘은 오히려 preventDefault를 더 많이 사용합니다. submit 기본 이벤트의 경우 새로고침을 하기 때문에 최근 React, vue 등이 추구하는 SPA에 잘 맞지 않습니다 :) 다만 앞으로 form에서만 뿐만 아니라 preventDefault를 쓰실 때 기본 동작이 어떤 것이고 어떤 것을 막는지 명확하게 찾아보시면 좋겠죠~!
| validateConfirmPassword(); | ||
| }); | ||
| // 입력 필드에서 값이 변경될 때 버튼 상태 업데이트 | ||
| emailInput.addEventListener("input", updateSubmitSignupState); |
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.
이벤트 버블링을 이용해 form에 달아도 좋습니다~!
| validatePassword(); | ||
| }); | ||
| passwordConfirmInput.addEventListener("blur", () => { | ||
| passwordConfirmTouched = 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.
토글 아이콘 동작까지 만들고 다시 확인해보니 버튼 활성화가 원하는 대로 잘 안되었습니다. 어떤 코드에서 문제가 있는 지 궁금합니다.
-> 버튼 활성화 상태는 input에 달려고, passwordConfirmTouched = true; 이게 blur에 있어서 잘 안됐던 것이 아닐까요~? 🤔 문제를 찾으실 때 차근차근 콘솔로 찍어보시면 좋습니다 :)
버튼 상태가 안바뀌니
submitBtn.disabled = !(
emailTouched &&
usernameTouched &&
passwordTouched &&
passwordConfirmTouched &&
isEmailVaild &&
isUsernameValid &&
isPasswordValid &&
isPasswordConfirmValid
);
이 요소들을 콘솔로 찍고 확인해 보시는거죠~!
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.
+추가로, vscode쓰신다면 https://velog.io/@lgsgst5613/Vscode-Extension-%EC%B6%94%EC%B2%9CTurbo-console-log
이런거 쓰시면 편합니다 🤣
| @@ -0,0 +1,88 @@ | |||
| document.addEventListener("DOMContentLoaded", () => { | |||
| const emailInput = document.getElementById("email"); | |||
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.
변수명이 명확합니다! 👍
요구사항
기본
로그인
회원가입
심화
주요 변경사항
스크린샷
멘토에게