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.

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

[임상훈] week6 #251

Open
wants to merge 2 commits into
base: part1-임상훈
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Binary file modified .DS_Store
Binary file not shown.
10 changes: 8 additions & 2 deletions signin.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,16 @@
<body>
<header>
<a class="logo-link" href="index.html">
<img class="header-logo" src="./images/logo.svg" alt="홈으로 연결된 Linkbrary 로고" />
<img
class="header-logo"
src="./images/logo.svg"
alt="홈으로 연결된 Linkbrary 로고"
/>
</a>
<p class="header-message">
회원이 아니신가요?<a class="header-link" href="signup.html">회원 가입하기</a>
회원이 아니신가요?<a class="header-link" href="signup.html"
>회원 가입하기</a
>
</p>
</header>
<div class="sign-box">
Expand Down
30 changes: 22 additions & 8 deletions signup.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,43 @@
<body>
<header>
<a class="logo-link" href="index.html">
<img class="header-logo" src="./images/logo.svg" alt="홈으로 연결된 Linkbrary 로고" />
<img
class="header-logo"
src="./images/logo.svg"
alt="홈으로 연결된 Linkbrary 로고"
/>
</a>
<p class="header-message">
이미 회원이신가요?<a class="header-link" href="signin.html">로그인 하기</a>
이미 회원이신가요?<a class="header-link" href="signin.html"
>로그인 하기</a
>
</p>
</header>
<div class="sign-box">
<form class="sign-form">
<form id="form" class="sign-form">
<div class="sign-inputs">
<div class="sign-input-box">
<label class="sign-input-label">이메일</label>
<input class="sign-input" />
<input id="email" class="sign-input" />
<p id="email-error-message" class="error-message"></p>
</div>
<div class="sign-input-box sign-password">
<label class="sign-input-label">비밀번호</label>
<input class="sign-input" type="password" />
<button class="eye-button" type="button">
<input id="password" class="sign-input" type="password" />
<p id="password-error-message" class="error-message"></p>
<button id="password-toggle" class="eye-button" type="button">
<img src="./images/eye-off.svg" />
</button>
</div>
<div class="sign-input-box sign-password">
<label class="sign-input-label">비밀번호 확인</label>
<input class="sign-input" type="password" />
<button class="eye-button" type="button">
<input id="confirm-password" class="sign-input" type="password" />
<p id="confirm-password-error-message" class="error-message"></p>
<button
id="confirm-password-toggle"
class="eye-button"
type="button"
>
<img src="./images/eye-off.svg" />
</button>
</div>
Expand All @@ -59,5 +72,6 @@
</div>
</div>
</div>
<script type="module" src="./src/signup.js"></script>
</body>
</html>
70 changes: 54 additions & 16 deletions src/signin.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,15 @@ import {

const emailInput = document.querySelector("#email");
const emailErrorMessage = document.querySelector("#email-error-message");
emailInput.addEventListener("focusout", (event) => validateEmailInput(event.target.value));
emailInput.addEventListener("focusout", (event) =>
validateEmailInput(event.target.value)
);
function validateEmailInput(email) {
if (email === "") {
setInputError({ input: emailInput, errorMessage: emailErrorMessage }, "이메일을 입력해주세요.");
setInputError(
{ input: emailInput, errorMessage: emailErrorMessage },
"이메일을 입력해주세요."
);
return;
}
if (!isEmailValid(email)) {
Expand All @@ -26,7 +31,9 @@ function validateEmailInput(email) {

const passwordInput = document.querySelector("#password");
const passwordErrorMessage = document.querySelector("#password-error-message");
passwordInput.addEventListener("focusout", (event) => validatePasswordInput(event.target.value));
passwordInput.addEventListener("focusout", (event) =>
validatePasswordInput(event.target.value)
);
function validatePasswordInput(password) {
if (password === "") {
setInputError(
Expand All @@ -35,7 +42,10 @@ function validatePasswordInput(password) {
);
return;
}
removeInputError({ input: passwordInput, errorMessage: passwordErrorMessage });
removeInputError({
input: passwordInput,
errorMessage: passwordErrorMessage,
});
}

const passwordToggleButton = document.querySelector("#password-toggle");
Expand All @@ -45,19 +55,47 @@ passwordToggleButton.addEventListener("click", () =>

const signForm = document.querySelector("#form");
signForm.addEventListener("submit", submitForm);

function submitForm(event) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jose0229

submitForm 은 사실 submit 이벤트가 발생되면 어떻게 처리할지를 정의해주는 함수이죠? 따라서 submitForm 이라는 네임보다는 onFormSubmit 또는 handleFormSubmit 이라는 함수 이름이 더 좋아보입니다

event.preventDefault();
let inputEmailPw = {
email: emailInput.value,
password: passwordInput.value,
};
Comment on lines +61 to +64
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jose0229

변경되지 않을 변수인데 굳이 let을 사용하는건 좋지 않아보여요.
let이 포함하고 있는 의미는 이후에 재할당이 된다는 뜻을 지니고 있어서요
const로 선언해 두는건 어떠신가요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

또한 inputEmailPw 라는 이름보단, 로그인 관련 정보입을 표현하는 변수명이 더 좋아보입니다


const isTestUser =
emailInput.value === TEST_USER.email && passwordInput.value === TEST_USER.password;

if (isTestUser) {
location.href = "/folder";
return;
}
setInputError({ input: emailInput, errorMessage: emailErrorMessage }, "이메일을 확인해주세요.");
setInputError(
{ input: passwordInput, errorMessage: passwordErrorMessage },
"비밀번호를 확인해주세요."
);
fetch("https://bootcamp-api.codeit.kr/api/sign-in", {
method: "POST",
headers: {
"Content-Type": "application/json", // 이 부분 추가
},
body: JSON.stringify(inputEmailPw),
})
.then((response) => {
if (response.ok) {
window.location.href = "/folder";
} else {
throw new Error("Server responded with an error!");
}
})
.catch((error) => {
console.error(error);
setInputError(
{ input: emailInput, errorMessage: emailErrorMessage },
"오류가 발생했습니다. 다시 시도해주세요."
);
});
}
Comment on lines +66 to 87
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jose0229

실제 로그인 액션을 처리하는 녀석을 분리시켜보면 어떨까요? 오늘 우리 멘토링떄 이야기한 내용 참고해보면 좋을 것 같아요!


// const isTestUser =
// emailInput.value === TEST_USER.email && passwordInput.value === TEST_USER.password;

// if (isTestUser) {
// location.href = "/folder";
// return;
// }
// setInputError({ input: emailInput, errorMessage: emailErrorMessage }, "이메일을 확인해주세요.");
// setInputError(
// { input: passwordInput, errorMessage: passwordErrorMessage },
// "비밀번호를 확인해주세요."
// );
// }
165 changes: 165 additions & 0 deletions src/signup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
import {
setInputError,
removeInputError,
isEmailValid,
isPasswordValid,
togglePassword,
TEST_USER,
} from "./utils.js";

const emailInput = document.querySelector("#email");
const emailErrorMessage = document.querySelector("#email-error-message");
emailInput.addEventListener("focusout", (event) =>
validateEmailInput(event.target.value)
);
function validateEmailInput(email) {
if (email === "") {
setInputError(
{ input: emailInput, errorMessage: emailErrorMessage },
"이메일을 입력해주세요."
);
return;
}

fetch("https://bootcamp-api.codeit.kr/api/check-email", {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({ email: email }), // 수정됨
})
.then((response) => response.json()) // 응답을 JSON으로 파싱
.then((data) => {
if (data.isAvailable) {
// 가정: 응답에 isAvailable 필드가 있다고 가정
removeInputError({
input: emailInput,
errorMessage: emailErrorMessage,
});
} else {
setInputError(
{ input: emailInput, errorMessage: emailErrorMessage },
"이미 사용 중인 이메일입니다."
);
}
})
.catch((error) => {
console.error("Error checking email:", error);
setInputError(
{ input: emailInput, errorMessage: emailErrorMessage },
"이메일 확인 중 오류가 발생했습니다."
);
});
Comment on lines +24 to +52
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jose0229

validateEmailInput 에서 처리하기에 너무 무게감이 큰 녀석이에요!
또한 단순 input 체크보단 데이터베이스에 존재하는지 여부를 체크하는 목적인만큼, 외부 함수로 구현해 활용하는게 좋겠어요.
음 만약 코드잇 요구사항에서 이메일 입력창 벗어날때마다 이 네트워크 통신을 통한 밸리데이션이 필요하다 명세했다면 어쩔 수 없지만, 보통 프로덕션 제품에서는 이런 기능은 회원가입 실행 후 체크하는 요소로 평가됩니다.

}

const passwordInput = document.querySelector("#password");
const passwordErrorMessage = document.querySelector("#password-error-message");
passwordInput.addEventListener("focusout", (event) =>
validatePasswordInput(event.target.value)
);

function validatePasswordInput(password) {
if (password === "" || !isPasswordValid(password)) {
setInputError(
{ input: passwordInput, errorMessage: passwordErrorMessage },
"비밀번호는 영문, 숫자 조합 8자 이상 입력해 주세요."
);
return false;
}
removeInputError({
input: passwordInput,
errorMessage: passwordErrorMessage,
});
return true;
}
Comment on lines +61 to +74
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jose0229

validation과 ui controlling을 함꼐하고 있네요. 조금 더 분리시킬 수 있으면 좋겠어요!


const confirmPasswordInput = document.querySelector("#confirm-password");
const confirmPasswordErrorMessage = document.querySelector(
"#confirm-password-error-message"
);
confirmPasswordInput.addEventListener("focusout", (event) =>
validateConfirmPasswordInput(event.target.value)
);
function validateConfirmPasswordInput(confirmPassword) {
if (confirmPassword === "" || !isPasswordValid(confirmPassword)) {
setInputError(
{
input: confirmPasswordInput,
errorMessage: confirmPasswordErrorMessage,
},
"비밀번호는 영문, 숫자 조합 8자 이상 입력해 주세요."
);
return false;
}
if (passwordInput.value !== confirmPassword) {
setInputError(
{
input: confirmPasswordInput,
errorMessage: confirmPasswordErrorMessage,
},
"비밀번호가 일치하지 않아요."
);
return false;
}
removeInputError({
input: confirmPasswordInput,
errorMessage: confirmPasswordErrorMessage,
});
return true;
}

const passwordToggleButton = document.querySelector("#password-toggle");
passwordToggleButton.addEventListener("click", () =>
togglePassword(passwordInput, passwordToggleButton)
);

const confirmPasswordToggleButton = document.querySelector(
"#confirm-password-toggle"
);
confirmPasswordToggleButton.addEventListener("click", () =>
togglePassword(confirmPasswordInput, confirmPasswordToggleButton)
);

const signForm = document.querySelector("#form");
signForm.addEventListener("submit", submitForm);
function submitForm(event) {
event.preventDefault();

const isEmailInputValid = validateEmailInput(emailInput.value);
const isPasswordInputValid = validatePasswordInput(passwordInput.value);
const isConfirmPasswordInputValid = validateConfirmPasswordInput(
confirmPasswordInput.value
);

if (
isEmailInputValid &&
isPasswordInputValid &&
isConfirmPasswordInputValid
Comment on lines +135 to +137
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jose0229

이렇게 뭉뜽그려 validation를 체크하게 되면, 어떤 요소로 인해 회원가입이 실패했는지 확인하기 어렵습니다.
따라서 validate~~~Input 이라는 녀석은 실제 값에 대해 validation만 처리하고, 그 건에 대해 어떻게 ui 업데이트를 해줄건지를 각각의 케이스에 따라 if 문으로 구현하는게 좋아보여요!

) {
fetch("https://bootcamp-api.codeit.kr/api/sign-up", {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({
email: emailInput.value,
password: passwordInput.value,
}),
})
.then((response) => {
if (response.ok) {
window.location.href = "/folder";
} else {
throw new Error("Server responded with an error!");
}
})
.catch((error) => {
console.error(error);
setInputError(
{ input: emailInput, errorMessage: emailErrorMessage },
"오류가 발생했습니다. 다시 시도해주세요."
);
});
}
location.href = "/folder";
Comment on lines +139 to +164
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jose0229

이렇게 구현하면 fetch, then then catch등이 실행되고 난 후에 location.href가 실행되는 순서가 보장되나요?
한번 비동기 코드 실행순서 체크 필요할 것 같습니다.

}
6 changes: 6 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ export function isEmailValid(email) {
return new RegExp(EMAIL_REGEX).test(email);
}

export function isPasswordValid(password) {
const isEightLettersOrMore = password.length >= 8;
const hasNumberAndCharacter = password.match(/[0-9]/g) && password.match(/[a-zA-Z]/gi);
return isEightLettersOrMore && hasNumberAndCharacter;
}

export function togglePassword(input, toggleButton) {
if (input.getAttribute("type") === "password") {
input.setAttribute("type", "text");
Expand Down