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

Merged

Conversation

dlwnsl
Copy link
Collaborator

@dlwnsl dlwnsl commented Dec 6, 2024

요구사항

기본

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

심화

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

주요 변경사항

  • 회원가입, 로그인, 로그아웃, 상품 등록 구현했습니다

스크린샷

image
image

멘토에게

  • react-hook-form은 활용하지 못 했습니다
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@dlwnsl dlwnsl requested a review from jyh0521 December 6, 2024 02:17
@dlwnsl dlwnsl added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Dec 6, 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.

고생하셨습니다.

values.tags.length > 0;

const handleChange = (name: string, value: any) => {
setValues((initialValues) => ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

value를 업데이트 할때는 initialValues 보다는 prev나 prevState 같은 단어를 사용해주시는 것이 조금 더 직관적일 것 같습니다.

Comment on lines +80 to +91
e.preventDefault();
const formData = new FormData();
formData.append("name", values.name);
formData.append("favorite", values.favoriteCount.toString());
formData.append("description", values.description);
formData.append("price", values.price.toString());
if (values.images) {
values.images.forEach((image, index) => {
formData.append(`images[${index}]`, image);
});
}
formData.append("tags", JSON.stringify(values.tags));
Copy link
Collaborator

Choose a reason for hiding this comment

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

로직 간에 개행으로 구분해주시면 가독성에 도움이 될 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

파일명에는 띄어쓰기를 공백으로 넣기보다는 _를 활용해서 표기하는 것을 추천드립니다.

@@ -8,6 +8,7 @@ import heart from "@/public/svgs/ic_heart (1).svg";
import defaultImage from "@/public/pngs/noImage.png";
import searchIcon from "@/public/svgs/ic_search.svg";
import { getArticles } from "@/lib/api";
import debounce from "@/lib/utils/debounce";
Copy link
Collaborator

Choose a reason for hiding this comment

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

debounce 유틸 함수로 만들어주신거 좋네요.

return 10;
}
};
import getPageSize from "@/lib/utils/getPageSize";

function AllItems() {
const [orderBy, setOrderBy] = useState<string>("recent");
Copy link
Collaborator

Choose a reason for hiding this comment

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

string 타입 대신에 들어가는 문자열을 타입으로 사용해보는 것은 어떨까요?

],
formats: ["image/avif", "image/webp"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

차세대 이미지 형식 적용해주신 점 좋네요.

};

export default function App({ Component, pageProps }: AppPropsWithLayout) {
const getLayout =
Copy link
Collaborator

Choose a reason for hiding this comment

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

getLayout 활용하신거 좋네요.

Comment on lines +37 to +39
const isLoginEnabled = (): boolean => {
return validateEmail(email) && password.length >= 8;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 함수 말고 변수로 만들어도 될 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

폼의 형식이 비슷해서 login에서 사용되는 함수들을 공통 훅으로 분리해서 같이 사용할 수 있게 만들어보셔도 좋을 것 같습니다.

display: flex;
align-items: center;
justify-content: space-between;
background-color: #e6f2ff;
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 47bb5eb into codeit-bootcamp-frontend:Next-이준희 Dec 9, 2024
Comment on lines +5 to +10
remotePatterns: [
{
protocol: "https",
hostname: "**",
},
],
Copy link
Collaborator

Choose a reason for hiding this comment

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

이것만 적용하면 밑에 domains는 없어도 되지 않나요?

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