-
Notifications
You must be signed in to change notification settings - Fork 46
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
[정지성] week14 #437
The head ref may contain hidden characters: "part3-\uC815\uC9C0\uC131-week14"
[정지성] week14 #437
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상태나 컴포넌트 네이밍을 좀 더 직관적으로 수정해주신 것 같아요!
훅폼 라이브러리를 사용하진 않았지만 커스텀 훅으로 분리해서 관리한 점도 좋았습니다.
고생하셨습니다 👏
const router = useRouter(); | ||
|
||
const handleLogout = () => { | ||
localStorage.removeItem('accessToken'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
스토리지 키값도 상수로 관리하면 어떨까요?
try { | ||
const response = await fetch(SIGN_IN_BASE_URL, { | ||
method: 'POST', | ||
headers: { | ||
'Content-Type': 'application/json', | ||
}, | ||
body: JSON.stringify({ email, password }), | ||
}); | ||
|
||
if (!response.ok) { | ||
throw new Error('로그인에 실패했습니다.'); | ||
} | ||
const data = await response.json(); | ||
const { accessToken, refreshToken } = data; | ||
localStorage.setItem('accessToken', accessToken); | ||
localStorage.setItem('refreshToken', refreshToken); | ||
|
||
router.push('/folder'); | ||
} catch (error) { | ||
console.error('Error:', error); | ||
handleIdErrorOnSubmit(); | ||
handlePWdErrorOnSubmit(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetch / success / fail 을 나눠서 관리하면 어떨까요?
// api 관련 파일
const signin = async (body: {email: string, password: string}, {successCb, fallCb}?: {successCb: () => void, failCb: () => void}) => {
// ...
}
function handleSubmit() {
signin({email, 토큰 저장, 에러표시});
// 아니면 fetch 함수만 분리하고 try catch 문과 성공 실패는 이 함수에서 처리해도 괜찮을 것 같아요.
}
const { | ||
email, | ||
isIdError, | ||
idErrorMsg, | ||
handleEmailChange, | ||
handleIdBlur, | ||
handleIdErrorOnSubmit, | ||
} = useSignInId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
커스텀 훅으로 관심사를 분리한 부분 좋습니다 👍
export interface InputProps { | ||
value: string; | ||
onChange: (value: string) => void; | ||
placeholder: string; | ||
isError: boolean; | ||
ErrorMsg: string; | ||
onBlur: () => void; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id, password input 에서 공통으로 사용된다면 외부 파일에 추가해서 관리하는게 좋을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이번에 styled component 에서 module css 로 스타일링 방식을 변경하신 것 같네요!
이유가 있으신가요?
요구사항
기본
심화
주요 변경사항
스크린샷
https://5-weekly-mission-jjs-byukchongs-projects.vercel.app/
멘토에게