Skip to content

[김수영] sprint11 #130

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

Conversation

swim-kim
Copy link
Collaborator

@swim-kim swim-kim commented Nov 9, 2024

요구사항

기본

회원가입

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

로그인

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

메인

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

멘토에게

  • 처음에 로컬스토리지에 접근이 안돼서 isMount를 설정해서 마운트 되면 그때 로컬스토리지에서 값을 받아오도록 했는데 또 다른 방법이 있을까요..??

@GANGYIKIM GANGYIKIM self-requested a review November 11, 2024 00:12
Copy link
Collaborator

@GANGYIKIM GANGYIKIM left a comment

Choose a reason for hiding this comment

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

수영님 이번 스프린트 미션 고생하셨습니다~
질문주신건 코멘트에 있으니 참고해보세요~

<MS.BannerText>일상의 모든 물건을<br />거래해 보세요</MS.BannerText>
<MS.ItemsButton href="/items">구경하러 가기</MS.ItemsButton>
</MS.BannerTextBox>
<MS.BannerImg src={BannerImg} alt="베너이미지" ></MS.BannerImg>
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
저는 children이 없으면 self closing 방식을 더 선호합니다~
특히 img 태그는 html에서도 self closing 방식이니 아래처럼 하시는 것을 추천드립니다.

Suggested change
<MS.BannerImg src={BannerImg} alt="베너이미지" ></MS.BannerImg>
<MS.BannerImg src={BannerImg} alt="베너이미지" />

Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
figma에 auth/signin 이라고 각 페이지별 주소를 명시해주었으니 가능하면 따르시는 것을 추천드립니다~

Comment on lines +14 to +21
useEffect(() => {
if (isMounted) {
const accessToken = localStorage.getItem('accessToken');
if (accessToken) {
router.push('/');
}
}
}, [isMounted]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
해당 state 대신 isReady 속성을 쓰시면 좋을 것 같습니다.

Suggested change
useEffect(() => {
if (isMounted) {
const accessToken = localStorage.getItem('accessToken');
if (accessToken) {
router.push('/');
}
}
}, [isMounted]);
useEffect(() => {
if (router.isReady) {
const accessToken = localStorage.getItem("accessToken");
if (accessToken) {
router.push("/");
}
}
}, [router]);

if (response && response.accessToken) {
localStorage.setItem('accessToken', response.accessToken);
}
router.push('/sign/in');
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
회원가입 완료 후 로그인 페이지로 이동시키는 경우이기에 push보다 replace가 더 적절할 것 같습니다~

https://nextjs.org/docs/pages/api-reference/functions/use-router#routerreplace

{...register("email", {
required: "이메일을 입력해주세요",
pattern: {
value: /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4}$/,
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2:
해당 regex가 회원가입에서도 로그인에서도 같은 정규표현식이 사용되므로 따로 선언해두시고 공용으로 쓰시면 더 좋겠습니다~

Copy link
Collaborator

Choose a reason for hiding this comment

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

에러메시지도 반복적으로 사용된다면 따로 관리하시는것을 추천드립니다~

useEffect(()=>{
const accessToken = localStorage.getItem('accessToken')
setIsLogin(!accessToken);
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1:

Suggested change
})
},[])

Comment on lines +53 to +60
export async function postSignUp(signUpData : SignUpFormInputs) {
try {
const response = await axios.post(`${BASE_URL}/auth/signUp`, {
email:signUpData.email,
nickname:signUpData.nickname,
password:signUpData.password,
passwordConfirmation:signUpData.passwordConfirm,
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3:
아래 코드도 동일동작합니다~
signUpData 매개변수 객체 자체를 쓰는 일이 없으니 아래처럼 받아서 사용하시는게 가독성 측면에서 더 좋을 것 같습니다~

Suggested change
export async function postSignUp(signUpData : SignUpFormInputs) {
try {
const response = await axios.post(`${BASE_URL}/auth/signUp`, {
email:signUpData.email,
nickname:signUpData.nickname,
password:signUpData.password,
passwordConfirmation:signUpData.passwordConfirm,
});
export async function postSignUp({
email,
nickname,
password,
passwordConfirm: passwordConfirmation,
}: SignUpFormInputs) {
try {
const response = await axios.post(`${BASE_URL}/auth/signUp`, {
email,
nickname,
password,
passwordConfirmation,
});

@GANGYIKIM GANGYIKIM merged commit 665df24 into codeit-bootcamp-frontend:Next-김수영 Nov 11, 2024
@swim-kim swim-kim self-assigned this Nov 12, 2024
@swim-kim swim-kim added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Nov 12, 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