-
Notifications
You must be signed in to change notification settings - Fork 26
[김다은] Sprint9 #203
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
base: next-김다은
Are you sure you want to change the base?
[김다은] Sprint9 #203
The head ref may contain hidden characters: "next-\uAE40\uB2E4\uC740-sprint9"
Conversation
dokdo2013
left a comment
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.
전반적으로 잘 작성하셨습니다. 고생 많으셨습니다!
- 현재 API 호출하는 부분이 공통 부분 쓰는 경우도 있고, 개별적으로 fetch를 쓰는 경우도 있습니다. 하나로 방식이 통일되고, API Base URL도 공통으로 분리해서 쓰면 재사용성에 도움이 될 것 같습니다.
| if (err.message.includes("존재하지")) { | ||
| setModalMessage("존재하지 않는 이메일입니다."); | ||
| } else if (err.message.includes("비밀번호")) { | ||
| setModalMessage("비밀번호가 일치하지 않습니다."); | ||
| } else { | ||
| setModalMessage("로그인에 실패했습니다."); | ||
| } | ||
| setShowModal(true); | ||
| } finally { | ||
| setLoading(false); | ||
| } |
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.
보통 이런 경우에는 서버에서 내려준 값을 그대로 쓰거나, 아니면 에러 코드같은 걸 미리 만들어두고 매핑해서 보여주는 식으로 많이 사용합니다. 여기서는 이렇게 처리해준 특별한 이유가 있을까요?
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.
이 컴포넌트는 기능적으로 PasswordField 컴포넌트와 검증하는 부분 하나 제외하고는 동일한데 하나로 통합할 수 있을 것 같습니다.
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게