Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Basic 김지연 sprint3 #1

Closed
wants to merge 7 commits into from
Closed

Basic 김지연 sprint3 #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
68bc3aa
feat: 스프린트 미션 3 기본 요구사항 - 로그인 페이지 공통 요구사항(에러메세지, 로그인 버튼 동작)
dani784601 Sep 30, 2024
2647ffa
feat: abort controller를 사용하여 window unload 이벤트 발생시 등록했던 event handler 제거
dani784601 Sep 30, 2024
f8e0dcc
feat: 스프린트 미션 3 심화 요구사항 - 오류 메시지 모달 구현
dani784601 Oct 1, 2024
2a2208e
feat: 스프린트미션3 기본요구 사항 - 회원가입 페이지(진행중)
dani784601 Oct 2, 2024
d98fca9
feat: 스프린트미션3 기본요구 사항 - 회원가입 페이지 유효성 검사, 스프린트 미션2 심회 요구사항 - 비밀번호 확인 아…
dani784601 Oct 4, 2024
7a01e75
fix: 잘못된 에러 메세지 선택자 변경
Oct 6, 2024
47bec06
Merge branch 'basic-김지연' into basic-김지연-sprint3
dani784601 Oct 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pages/sign-in.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@
}
}

userEmail.addEventListener('blur', () => {
userEmail.addEventListener('change', () => {
const errMsg = userEmail.nextElementSibling;
if (userEmail.validity.valid === false) {
errMsg.classList.add('fade-in');
Expand All @@ -154,8 +154,8 @@
}
}, { signal: signal });

userPassword.addEventListener('blur', () => {
const errMsg = userPassword.nextElementSibling;
userPassword.addEventListener('change', () => {
const errMsg = userPassword.parentElement.nextElementSibling;
if (userPassword.validity.valid === false) {
submitBtn.classList.replace('bg-primary', 'bg-gray400');
if (userPassword.validity.tooShort) {
Expand Down
32 changes: 15 additions & 17 deletions pages/sign-up.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,12 @@
customError: '비밀번호가 일치하지 않습니다.'
}
}

window.addEventListener('load', () => {
const form = document.querySelector('form');
const visibleBtn = form.querySelectorAll('input[type="checkbox"]');
const inputs = form.querySelectorAll('input[required]');
const button = form.querySelector('button');
const userInputs = { email: '', nickname: '', password: '', '[check-password]': '' };

for (const input of inputs) {
userInputs[input.id] = input;
input.addEventListener('change', () => {
Expand All @@ -141,19 +139,20 @@
}
input.checkValidity();
const formData = new FormData(form);
const errMsg = input.parentElement.nextElementSibling
const errMsg = input.nextElementSibling.tagName === 'P' ? input.nextElementSibling : input.parentElement.nextElementSibling;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

input.nextElementSibling가 null일 수 있으므로 안전하게 접근해주세요

input.nextElementSibling가 존재하지 않을 경우 .tagName에 접근하면 오류가 발생합니다. 이를 방지하기 위해 input.nextElementSibling의 존재 여부를 먼저 확인하는 조건을 추가하는 것이 좋습니다.

다음과 같이 수정할 수 있습니다:

- const errMsg = input.nextElementSibling.tagName === 'P' ? input.nextElementSibling : input.parentElement.nextElementSibling;
+ const errMsg = input.nextElementSibling && input.nextElementSibling.tagName === 'P' ? input.nextElementSibling : input.parentElement.nextElementSibling;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const errMsg = input.nextElementSibling.tagName === 'P' ? input.nextElementSibling : input.parentElement.nextElementSibling;
const errMsg = input.nextElementSibling && input.nextElementSibling.tagName === 'P' ? input.nextElementSibling : input.parentElement.nextElementSibling;

errMsg.classList.add('fade-in');
if (input.validity.valid === true) {
errMsg.textContent = '';
} else {
for (const errType in input.validity) {
if (input.validity[errType] === true) {
if (errType !== 'valid' && input.validity[errType] === true) {
errMsg.textContent = errMsgType[input.id][errType] ?? input.placeholder;

}
}
}
}, { signal: signal });
}

for (const btn of visibleBtn) {
btn.addEventListener('click', (e) => {
e.target.style.backgroundImage = e.target.checked ? 'url(../images/icon/show-off.svg)' : 'url(../images/icon/show-on.svg)';
Expand All @@ -164,17 +163,13 @@
}
}, { signal: signal });
}

form.addEventListener('formdata', (e) => {
Array.from(inputs).every(input => {
if (input.validity.valid === true) {
button.classList.replace('bg-gray400', 'bg-primary');
} else {
button.classList.replace('bg-primary', 'bg-gray400');
}
});
if (Array.from(inputs).every(input => input.validity.valid)) {
button.classList.replace('bg-gray400', 'bg-primary');
} else {
button.classList.replace('bg-primary', 'bg-gray400');
}
}, { signal: signal });

button.onclick = () => {
inputs.forEach(input => input.reportValidity());
if ([...inputs].every(input => input.validity.valid)) {
Expand All @@ -183,14 +178,17 @@
document.querySelector('dialog button').onclick = () => {
document.querySelector('dialog').close();
}
document.querySelector('dialog').showModal();
} else {
window.location.pathname = '/pages/sign-in.html';
document.querySelector('dialog p').textContent = '회원가입이 완료되었습니다.';
document.querySelector('dialog button').onclick = () => {
document.querySelector('dialog').close();
window.location.pathname = '/pages/sign-in.html';
}
Comment on lines +182 to +186
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

중복된 코드 리팩토링을 고려해주세요

document.querySelector('dialog button').onclick 설정 부분이 if와 else 블록에서 반복되고 있습니다. 중복 코드를 제거하고 가독성을 향상시키기 위해 해당 코드를 조건문 밖으로 이동시키는 것을 추천합니다.

다음과 같이 수정할 수 있습니다:

if (USER_DATA_MAP.has(userInputs.email.value)) {
  document.querySelector('dialog p').textContent = '사용 중인 이메일입니다.';
} else {
  document.querySelector('dialog p').textContent = '회원가입이 완료되었습니다.';
+  document.querySelector('dialog button').onclick = () => {
+    document.querySelector('dialog').close();
+    window.location.pathname = '/pages/sign-in.html';
+  }
}
- document.querySelector('dialog button').onclick = () => {
-   document.querySelector('dialog').close();
- }
document.querySelector('dialog').showModal();

또는 onclick 핸들러 내부에서 조건에 따라 동작을 분기할 수도 있습니다.

Committable suggestion was skipped due to low confidence.

⚠️ Potential issue

회원가입 완료 시 사용자 데이터 저장이 필요합니다

회원가입이 완료되었을 때, 새로운 사용자의 데이터를 USER_DATA_MAP 등에 저장하여 이후 로그인이 가능하도록 구현해야 합니다. 현재 코드에서는 사용자 데이터 저장에 대한 처리가 없습니다.

회원가입 완료 시 다음과 같이 사용자 데이터를 저장하는 로직을 추가하는 것을 제안합니다:

if (USER_DATA_MAP.has(userInputs.email.value)) {
  // 기존 코드 유지
} else {
  // 기존 코드 유지
+  USER_DATA_MAP.set(userInputs.email.value, {
+    nickname: userInputs.nickname.value,
+    password: userInputs.password.value
+  });
}

이렇게 하면 가입한 사용자가 로그인 시도 시 올바르게 인증될 수 있습니다.

Committable suggestion was skipped due to low confidence.

}
document.querySelector('dialog').showModal();
}
}
});

window.addEventListener('unload', () => {
controller.abort();
});
Expand Down
Loading