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

[장혜민] sprint11 #701

Merged

Conversation

hnitam
Copy link
Collaborator

@hnitam hnitam commented Jun 14, 2024

요구사항

Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
피그마 디자인에 맞게 페이지를 만들어 주세요.
기존의 스프린트 미션 8에 이어서 React, Typescript를 사용합니다.

기본

회원가입

  • 유효한 정보를 입력하고 스웨거 명세된 “/auth/signUp”으로 POST 요청해서 성공 응답을 받으면 회원가입이 완료됩니다.
  • 회원가입이 완료되면 “/login”로 이동합니다.
  • 회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 ‘/’ 페이지로 이동합니다.

로그인

  • 회원가입을 성공한 정보를 입력하고 스웨거 명세된 “/auth/signIp”으로 POST 요청을 하면 로그인이 완료됩니다.
  • 로그인이 완료되면 로컬 스토리지에 accessToken을 저장하고 “/” 로 이동합니다.
  • 로그인/회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 ‘/’ 페이지로 이동합니다.

메인

  • 로컬 스토리지에 accessToken이 있는 경우 상단바 ‘로그인’ 버튼이 판다 이미지로 바뀝니다.

멘토에게

  • 추후에 커밋해서 업데이트하겠습니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@hnitam hnitam requested a review from jyh0521 June 14, 2024 14:39
Copy link
Collaborator

@jyh0521 jyh0521 left a comment

Choose a reason for hiding this comment

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

과제하느라 고생하셨습니다. 👍 공통 로직을 분리해보셔도 좋을 것 같습니다.

@@ -45,7 +45,7 @@
font-weight: 400;
line-height: 16.71px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

px 값으로 소수는 사용하시지 않는 것이 좋습니다. 브라우저 마다 소수점을 변환하는 방식이 다르기 때문에 예측못한 버그가 발생할 수 있어요!

@@ -20,7 +20,7 @@
font-weight: 600;
line-height: 23px;
text-align: left;
color: #1f2937;
color: var(--gray-800);
Copy link
Collaborator

Choose a reason for hiding this comment

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

컬러 변수로 바꿔주신거 좋네요!

@@ -0,0 +1,17 @@
.Input {
Copy link
Collaborator

Choose a reason for hiding this comment

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

class 명은 첫글자를 대문자로 사용하시지 않아도 괜찮습니다!

width: 100%;
}

.Input:focus {
Copy link
Collaborator

Choose a reason for hiding this comment

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

focus와 placeholder 가상선택자 활용 좋네요!

@@ -1,3 +1,4 @@
import '../styles/global.css';
Copy link
Collaborator

Choose a reason for hiding this comment

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

alias path라는 것을 찾아보고 적용해보시는 것도 좋을 것 같습니다. 그럼 경로가 ../.. 이렇게 길어지는 것을 방지할 수 있어서 더 깔끔해보입니다.


.login_signup a {
text-decoration-line: underline;
color: #3182f6;
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기는 컬러 변수가 누락되었네요!

Comment on lines +89 to +94
@media (max-width: 767px) {
.login-container {
width: 344px;
margin: 0 auto;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

개인적으로는 해당 클래스 내부에 미디어 쿼리를 넣어주시는 방법을 추천드립니다. 그렇게 하면 해당 클래스가 사이즈 별로 어떻게 변화하는지 한눈에 알아볼 수 있기 때문에 좀 더 편리한 것 같아요!

Comment on lines +30 to +50
const handleEmailBlur = () => {
if (values.email === "") {
setErrorEmail("이메일을 입력해주세요.");
return;
} else if (!validateEmail(values.email)) {
setErrorEmail("잘못된 이메일 형식입니다.");
} else {
setErrorEmail("");
}
};

const handlePasswordBlur = () => {
if (values.password === "") {
setErrorPassword("비밀번호를 입력해주세요.");
return;
} else if (values.password.length < 8) {
setErrorPassword("비밀번호를 8자 이상 입력해주세요");
} else {
setErrorPassword("");
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 함수는 공통으로 사용되는 것 같아서 커스텀 훅으로 만드신 다음에 두 곳에서 꺼내서 사용하게 만드셔도 좋을 것 같습니다.

@jyh0521 jyh0521 merged commit 95b98af into codeit-bootcamp-frontend:Next.js-장혜민 Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants