-
Notifications
You must be signed in to change notification settings - Fork 31
[차경훈] sprint4 #141
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 #141
The head ref may contain hidden characters: "Basic-\uCC28\uACBD\uD6C8-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.
경훈님 오랜만에 미션 하시느냐고 수고 많으셨습니다 🤣
역시나 기본적인 js는 잘 활용해 주시네요! 코드 중복을 줄이고 구조화 시켜보시면 더욱 좋을 거 같습니다 :)
| ".input-wrapper .toggle-password" | ||
| ); | ||
|
|
||
| // 이메일 유효성 검사 함수 (signin.js와 동일) |
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.
공통된 부분은 파일을 따로 만들어 import 하시면 좋습니다 :)
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.
현대 브라우저는 모듈 환경을 제공하고 있어요!
https://developer.mozilla.org/ko/docs/Web/JavaScript/Guide/Modules#applying_the_module_to_your_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.
네 감사합니다! 해당 부분 잘 숙지하겠습니다!
| ); | ||
|
|
||
| // 이메일 유효성 검사 함수 (signin.js와 동일) | ||
| const validateEmail = () => { |
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.
깔끔하게 함수를 잘 나눠주셨네요! 👍
| // 이메일 유효성 검사 함수 (signin.js와 동일) | ||
| const validateEmail = () => { | ||
| const email = emailInput.value.trim(); | ||
| const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/; |
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.
정규식을 사용해 주셨군요! 👍
| emailError.textContent = ""; | ||
| emailInput.classList.remove("input-error"); | ||
| } | ||
| updateSignupButtonState(); |
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 함수 이름만 보면 버튼 상태를 변경하는 것을 알 수 없죠 :)
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.
네 기능별로 함수 나눠서 설계하도록 노력하겠습니다.
| isPasswordValid && | ||
| isPasswordConfirmationValid | ||
| ) { | ||
| signupButton.disabled = false; |
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.
signupButton.disabled = !(isEmailValid && isNicknameValid && isPasswordValid && isPasswordConfirmationValid)요렇게 표현할 수도 있겠네요~!
| }); | ||
|
|
||
| // 페이지 로드 시 초기 버튼 상태 설정 | ||
| updateSignupButtonState(); |
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.
<button disabled> 를 쓰셨다면 불필요한 거 같네요 :)
| }; | ||
|
|
||
| // 각 비밀번호 필드에 토글 기능 설정 | ||
| if (togglePasswordButtons.length > 0) { |
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.
data 속성 등을 이용해 구조화 해보시는 것도 좋습니다!
<img
src="images/icons/eye-invisible.svg"
alt="비밀번호 숨김"
class="toggle-password"
data-target="password"
/>
<img
src="images/icons/eye-invisible.svg"
alt="비밀번호 숨김"
class="toggle-password"
data-target="passwordConfirmation"
/>togglePasswordButtons.forEach((button) => {
const inputId = button.dataset.target;
const passwordField = document.getElementById(inputId);
setupPasswordToggle(passwordField, button);
});https://developer.mozilla.org/ko/docs/Web/HTML/How_to/Use_data_attributes
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.
오! 네! 알겠습니다. 감사합니다!
| passwordInput.value.trim() === passwordConfirmationInput.value.trim() | ||
| ) { | ||
| if (passwordConfirmationInput.classList.contains("input-error")) { | ||
| passwordConfirmationError.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를 추가하고 제거하는 로직이 계속 반복되는데, 해당 부분을 함수로 묶어보셔도 좋을 거 같네요 :)
function addErrorUI(id, message) {
const input= document.getElementById(id);
const error= document.getElementById(`${id}-error`);
//...
}
요구사항
기본
로그인
회원가입
심화
주요 변경사항
스크린샷
멘토에게