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 #322

Conversation

Injaeeee
Copy link
Collaborator

@Injaeeee Injaeeee commented Aug 23, 2024

요구사항

기본

회원가입

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

로그인

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

심화

  • 로그인, 회원가입 기능에 react-hook-form을 활용해봅니다.

주요 변경사항

스크린샷

image image

멘토에게

  • 디자인 부분의 구현은 아직 미완성이라 차차 만들어갈 예정입니다..
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@Injaeeee Injaeeee requested a review from jyh0521 August 23, 2024 17:22
@Injaeeee Injaeeee added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Aug 23, 2024
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.

과제 하느라 고생하셨습니다! 👍

{/* <Image
src={board.image}
<Image
src={board.image || defaultImage}
Copy link
Collaborator

Choose a reason for hiding this comment

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

board에 image가 없는 경우 defaultImage를 잘 넣어주셨네요!
추가로 board에 image가 있더라도 이미지 로딩 시 에러가 발생했을때 defaultImage를 넣어주는 것도 반영해주시면 좋을 것 같습니다.

@@ -31,7 +32,8 @@ function FileInput({ value, onChange }: FileInputType) {
const nextpreview = URL.createObjectURL(value);

setPreview(nextpreview);
console.log(nextpreview);
previewChange(nextpreview);
// console.log(nextpreview);
Copy link
Collaborator

Choose a reason for hiding this comment

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

불필요한 주석은 지워주시면 좋을 것 같습니다.

@@ -12,7 +12,11 @@ export default function PrimaryButton({
disabled: disabled = false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기는 disabled = false 이런식으로 작성해주셔도 똑같이 동작합니다!

border-radius: 12px;
padding: 16px 24px;
background-color: #f3f4f6;
color: #9ca3af;
Copy link
Collaborator

Choose a reason for hiding this comment

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

컬러 변수가 누락된 곳이 종종 있는데, 혹시 사용하지 않고 계시다면 사용해보셔도 좋을 것 같습니다. 컬러를 변수로 관리 하신다면 색상 수정할때 한번에 수정할 수 있거든요.

color: #9ca3af;
font-size: 16px;
font-weight: 400;
border: ${({ hasError }) => (hasError ? "1px solid red" : "none")};
Copy link
Collaborator

Choose a reason for hiding this comment

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

hasError가 undefined가 올 수도 있기 때문에 !!hasError 이런식으로 작성해주시면 확실하게 boolean으로 판단이 가능해집니다.

@@ -55,6 +59,20 @@ export default function Board() {
setCommentContent(e.target.value);
};

const handleCommentClick = async (e: React.MouseEvent<HTMLButtonElement>) => {
e.preventDefault();
if (typeof id === "string") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

id의 타입을 검사하는 방법도 있지만, id.toString() 해서 id의 타입을 강제하는 방법도 있습니다!

Comment on lines 94 to 98
<CommentTextarea
placeholder="댓글을 입력해주세요."
onChange={handleCommentChange}
value={commentContent}
></CommentTextarea>
Copy link
Collaborator

Choose a reason for hiding this comment

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

컴포넌트의 children이 없는 경우는 이렇게 하나로 축약해주셔도 좋습니다.

확인해보세요{" "}
</ElementMainText>
<ElementSubText>
{" "}
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 +32 to +46
useEffect(() => {
const { email, nickname, password } = signupInfo;
if (
email &&
nickname &&
password &&
!error.email &&
!error.password &&
!error.passwordConfirmation
) {
setSignupDisable(false);
} else {
setSignupDisable(true);
}
}, [signupInfo]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 로직을

const signupDisabled = email &&
      nickname &&
      password &&
      !error.email &&
      !error.password &&
      !error.passwordConfirmation

이런식으로 변수에 담아서 바로 사용해도 좋을 것 같습니다!

Comment on lines +40 to +45
{
headers: {
Authorization: `Bearer ${accessToken}`,
"Content-Type": "application/json",
},
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 헤더 같은 부분은 axios instance를 만들때 미리 같이 작성해두셔도 좋습니다.

@jyh0521 jyh0521 merged commit f850267 into codeit-bootcamp-frontend:Next-정인재 Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants