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

[김진희] Sprint 11 #326

Merged

Conversation

lemon1335
Copy link
Collaborator

@lemon1335 lemon1335 commented Dec 7, 2024

요구사항

기본

  • 회원가입

  • 유효한 정보를 입력하고 스웨거 명세된 “/auth/signUp”으로 POST 요청해서 성공 응답을 받으면 회원가입이 완료됩니다.

  • 회원가입이 완료되면 “/login”로 이동합니다.

  • 회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 ‘/’ 페이지로 이동합니다.

  • 로그인

  • 회원가입을 성공한 정보를 입력하고 스웨거 명세된 “/auth/signIp”으로 POST 요청을 하면 로그인이 완료됩니다.

  • 로그인이 완료되면 로컬 스토리지에 accessToken을 저장하고 “/” 로 이동합니다.

  • 로그인/회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 ‘/’ 페이지로 이동합니다.

  • 메인

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

심화

  • [x]
  • []

주요 변경사항

스크린샷

image

멘토에게

https://11-sprint-mission-git-next-lemon1335s-projects.vercel.app/

  • 말씀해주셨던 button도 size Prop으로 받게끔 수정했는데, 사이즈들이 완전히 일치하진 않아서 button 태그를 그냥 사용한 것도 있습니다..! 조금 더 실용성있게 어떻게 바꿀 수 있는지 알려주시면 감사하겠습니다!
  • 토큰 값이 undefined일 때도 로그인이 되는것처럼 main페이지로 돌아가지만, main페이지로 가더라도 header부분은 그대로 로그인 버튼이 보입니다. 어떻게 수정하면 좋을까요?
  • 회원가입 후 로그인이 된 채로 로그인 페이지가 아닌, 메인페이지로 이동하는데 어떻게 수정해야 할까요?
  • 로그인할 때 존재하지 않는 이메일이거나 비밀번호가 틀렸을 때 등 콘솔창에 뜨는 에러메시지를 ui에서도 그대로 뜨게 하고 싶은데 어떻게 수정하면 좋을지 말씀해주세요!
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@lemon1335 lemon1335 requested a review from kiJu2 December 7, 2024 11:07
@lemon1335 lemon1335 self-assigned this Dec 7, 2024
@lemon1335 lemon1335 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Dec 7, 2024
@kiJu2
Copy link
Collaborator

kiJu2 commented Dec 9, 2024

스프리트 미션 하시느라 수고 많으셨어요.
학습에 도움 되실 수 있게 꼼꼼히 리뷰 하도록 해보겠습니다. 😊

@kiJu2
Copy link
Collaborator

kiJu2 commented Dec 9, 2024

image 접속에 제한이 있습니다 ! ㅠㅠ

@kiJu2
Copy link
Collaborator

kiJu2 commented Dec 9, 2024

말씀해주셨던 button도 size Prop으로 받게끔 수정했는데, 사이즈들이 완전히 일치하진 않아서 button 태그를 그냥 사용한 것도 있습니다..! 조금 더 실용성있게 어떻게 바꿀 수 있는지 알려주시면 감사하겠습니다!

코드 리뷰로 한 번 확인해보겠습니다 !

@kiJu2
Copy link
Collaborator

kiJu2 commented Dec 9, 2024

토큰 값이 undefined일 때도 로그인이 되는것처럼 main페이지로 돌아가지만, main페이지로 가더라도 header부분은 그대로 로그인 버튼이 보입니다. 어떻게 수정하면 좋을까요?

혹시 로그인 후 새로고침을 하면 어떻게 되실까요? 만약 새로고침을 했을 때 잘 된다면 로그인은 정상적으로 처리된게 맞고, 유저 상태를 갱신하여 헤더가 리렌더링 될 수 있도록 하시면 됩니다!

@kiJu2
Copy link
Collaborator

kiJu2 commented Dec 9, 2024

회원가입 후 로그인이 된 채로 로그인 페이지가 아닌, 메인페이지로 이동하는데 어떻게 수정해야 할까요?

pages/login.jsx에 63번째 라인 router.push('/');이 있으므로 메인페이지로 이동되는게 아닐까 싶어요 !

@kiJu2
Copy link
Collaborator

kiJu2 commented Dec 9, 2024

로그인할 때 존재하지 않는 이메일이거나 비밀번호가 틀렸을 때 등 콘솔창에 뜨는 에러메시지를 ui에서도 그대로 뜨게 하고 싶은데 어떻게 수정하면 좋을지 말씀해주세요!

error.message를 출력하시면 됩니다 ! 객체를 그대로 노출하려 하면 정상적으로 출력이 잘 되지 않을거예요 !

Comment on lines +5 to +24
interface ButtonProp extends ButtonHTMLAttributes<HTMLButtonElement> {
children: ReactNode;
size: 'small' | 'medium' | 'large';
}

function Button({ children, size = 'small', ...rest }: ButtonProp) {
const buttonClass = classNames({
[styles.smallButton]: size === 'small',
[styles.mediumButton]: size === 'medium',
[styles.largeButton]: size === 'large',
});

return (
<>
<button className={buttonClass} {...rest}>
<div>{children}</div>
</button>
</>
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

와우 ㄷㄷㄷ 👍👍👍

질문에 답변하려고 왔는데 .. 공용 컴포넌트로 손색이 없는데요 ..?

import '@/styles/Header.module.css';
import '@/styles/Boards.module.css';
import '@/styles/BestBoard.module.css';
import '@/styles/EntireBoard.module.css';

export default function App({ Component, pageProps }) {
const router = useRouter();
const noHeaderPages = ['/signup', '/login', '/'];
Copy link
Collaborator

Choose a reason for hiding this comment

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

크으 레이아웃도 조건부로 잘 작성하셨네요 👍

훌륭합니다. ㄷㄷㄷ👍👍 화이트리스트를 만들어서 특정 페이지는 노출되지 않게끔 잘 작성하셨네요 👍👍

@@ -123,9 +123,9 @@ export default function Board({ articleData, initialCommentData }) {
/>
</div>
<div className={styles.commentButton}>
<SmallButton type="submit" disabled={!isFormValid}>
<Button type="submit" disabled={!isFormValid}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

isFormValid 상태는 없어도 될 것 같아요 !

Suggested change
<Button type="submit" disabled={!isFormValid}>
<Button type="submit" disabled={!(comment.trim() !== '')}>

isFormValid가 있기에 리렌더링을 한번 더 하게 됩니다. 이런 경우 그냥 조건부에 추가만 해주시면 불필요한 리렌더링 없이 잘 노출될거예요 !

Comment on lines +44 to +68
<div className={styles.logo}>
<Image src={logo} alt="로고" />
<Link href="/" className={styles.title}>
<h1 className={styles.title}>판다마켓</h1>
</Link>
</div>
{isLoggedIn ? (
<>
<button onClick={handlePandaLogoClick}>
<Image src={login} alt="로그인" />
</button>
{showLogout && (
<button onClick={handleLogout} className={styles.logoutButton}>
로그아웃
</button>
)}
</>
) : (
<Link href="/login">
<Button type="button" size="medium">
로그인
</Button>
</Link>
)}
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 컴포넌트는 Header 컴포넌트를 새로 파일을 작성하셔도 될 듯 합니다 !

해당 컴포넌트는 여러 페이지에서 사용될 수 있어요.
혹은 Header를 이미 만드신 것으로 보이는데 여기만 새로 작성하신 이유가 있으실까요 ?

Comment on lines +10 to +27

export default function Login() {
const router = useRouter();
const [formData, setFormData] = useState({
email: '',
password: '',
});

const [formValidity, setFormValidity] = useState({
isEmailValid: false,
isPasswordValid: false,
});
const [isButtonEnabled, setIsButtonEnabled] = useState(false);

const validateEmail = value => {
const regex = new RegExp('^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+.[a-zA-Z]{2,}$');
return regex.test(value);
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

validateEmail, validatePassword 함수는 컴포넌트 외부에 작성하시는걸 제안드립니다 !

Suggested change
export default function Login() {
const router = useRouter();
const [formData, setFormData] = useState({
email: '',
password: '',
});
const [formValidity, setFormValidity] = useState({
isEmailValid: false,
isPasswordValid: false,
});
const [isButtonEnabled, setIsButtonEnabled] = useState(false);
const validateEmail = value => {
const regex = new RegExp('^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+.[a-zA-Z]{2,}$');
return regex.test(value);
};
const validateEmail = value => {
const regex = new RegExp('^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+.[a-zA-Z]{2,}$');
return regex.test(value);
};
export default function Login() {
const router = useRouter();
const [formData, setFormData] = useState({
email: '',
password: '',
});
const [formValidity, setFormValidity] = useState({
isEmailValid: false,
isPasswordValid: false,
});
const [isButtonEnabled, setIsButtonEnabled] = useState(false);

두 함수는 컴포넌트 상태와 무관한 함수예요. 함수에 필요한 것은 파라메터로 잘 받아서 처리하고 있네요 👍
리렌더링 시 불필요한 재선언이 될 수 있으므로 컴포넌트 외부에 작성하시는게 더 바람직해보입니다 👍👍

Comment on lines +53 to +67
try {
const response = await instance.post('/auth/signIn', {
email: formData.email,
password: formData.password,
});
if (!response) return;
// 로그인 성공 시 처리
console.log(response);
const token = response.data.access_token;
localStorage.setItem('token', token);
router.push('/');
} catch (error) {
// 에러 처리
console.log(error);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

크으 try ... catch까지 굳굳 ! 👍👍

router.push('/');
} catch (error) {
// 에러 처리
console.log(error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

에러 관련해서 질문주셨는데 !

Suggested change
console.log(error);
alert(error.message);

위와 같이 작성해보시겠어요 ?

Comment on lines +30 to +37
const validateEmail = value => {
const regex = new RegExp('^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$');
return regex.test(value);
};

const validatePassword = value => value.length >= 8;

const validateNickname = value => value.trim() !== '';
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 함수도 마찬가지로 컴포넌트 외부에 선언하시는걸 권장드립니다 !

@kiJu2
Copy link
Collaborator

kiJu2 commented Dec 9, 2024

훌륭합니다 ! 수고하셨어요 진희님.
특히 버튼 공용 컴포넌트 만드신게 인상깊네요 👍👍
disabled를 위한 isFormValid는 지워보시는걸 추천드릴게요 !

@kiJu2 kiJu2 merged commit 9242d77 into codeit-bootcamp-frontend:Next-김진희 Dec 9, 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