-
Notifications
You must be signed in to change notification settings - Fork 5
Feature: 회원가입, 로그인 화면 구현 및 axios 인터셉터 설정 #52
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
… feature/signup
… feature/signup
… feature/signup
|
@summerDev96 is attempting to deploy a commit to the 626-ju's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| } from '@/lib/form/schemas'; | ||
| import { LoginRequest, LoginResponse, SignupRequest, SignupResponse } from '@/types/AuthTypes'; | ||
|
|
||
| const SignupSchema = z |
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.
zod를 사용해 스키마 정의 잘하셨네요!
| const { userData, isLoading } = useAuthRedirect(); | ||
| const router = useRouter(); | ||
|
|
||
| const methods = useForm<SignupData>({ |
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.
추후 여유가 있다면 criteriaMode: 'all' 를 추가해 여러 조건의 에러를 표시해도 좋을 것 같다는 생각을 했습니다!
에러 메시지가 늘어나며 레이아웃 밀림이 있다면 안정성 우선이 좋겠습니다.
const methods = useForm<SignupData>({
resolver: zodResolver(SignupSchema),
mode: 'all',
criteriaMode: 'all', // 추가
});에러 메시지 예시
- 비밀번호는 8자 이상이어야 합니다
- 특수문자를 포함해야 합니다
- 숫자를 포함해야 합니다
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.
확인 감사합니다! 한번 테스트해보겠습니다
|
|
||
| /* useAuthRedirect 훅에서 유저 데이터 요청 후 리디렉트 처리 */ | ||
| // 로딩중이거나 데이터 없으면 화면 안보이게 처리 | ||
| if (isLoading || userData) return null; |
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 default function App({ Component, pageProps }: AppProps) { | ||
| const { pathname } = useRouter(); | ||
| const pagesWithoutGnb = ['/login', '/signup', '/signin', '/_error']; | ||
| const pagesWithoutGnb = [ |
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.
테일윈드 색상 추가한 부분도 꼼꼼히 확인해주셨군요 감사합니다!
src/components/common/FormInput.tsx
Outdated
| onClick={togglePasswordIcon} | ||
| > | ||
| {passwordType === 'password' ? <PasswordIconOff /> : <PasswordIconOn />} | ||
| </span> |
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.
접근성 측면에서
span이 아닌 button을 사용하는 건 어떠실까요?
aria-label로 비밀번호 보이기 숨기기도 명시해주면 좋을 것 같습니다.
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.
button으로 변경 및 aria-label, aria-toggle 속성 추가하였습니다!
youdaeng2
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.
회원가입 폼, 로그인 로직이 Zod, React-Hook-Form, React-Query, Axios 인터셉터로 잘 구성되어 있네요!
제게 어려운 부분이었는데 코드 읽으면서 공부가 된 것 같습니다.
바로 승인하겠습니다.
후에 여유가 된다면 회원가입 요청 시 응답이 오기 전까지 로딩 스피너를 넣어도 좋을 것 같네요!!
(사용자에게 시각적 피드백을 주기 위해)
|
로딩 스피너 추가가 필요하다고 생각했는데 모달 끝난 후 한번 살펴보겠습니다! |
📦 Pull Request
📝 요약(Summary)
기능 및 컴포넌트 추가
커스텀 훅 추가
💬 공유사항 to 리뷰어
ErrorModal,useErrorModal훅도 모달 합성 컴포넌트로 변경 후 수정예정입니다.env.local파일에 환경변수 세팅 후 API 요청 시 사용하시면 됩니다.프로젝트 파일 환경변수 세팅
🗂️ 관련 이슈
📸 스크린샷
[회원가입 화면]


[로그인 화면]


✅ 체크리스트