-
Notifications
You must be signed in to change notification settings - Fork 56
부산대FE_ 안형찬_1단계 - 로그인 기능 구현하기 #103
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: hyeongchanan
Are you sure you want to change the base?
부산대FE_ 안형찬_1단계 - 로그인 기능 구현하기 #103
Conversation
1단계 내용 가져오기
커스텀 훅 Login.tsx 추가 이를 사용해서 로그인 에러 로직 구현
SeonJun-Hwang
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.
반갑습니다 형찬님! 1차 과제 진행간 고생 많으셨습니다 👍
짧은 코드지만 완성도가 높아 보기 좋았습니다 🥇
개인적인 생각은 코멘트에 담았습니다! 형찬님의 생각을 코멘트로 남겨주셔도 좋습니다!
| function useInput(initialValue: string, validator: ValidatorFn) { | ||
| const [value, setValue] = useState(initialValue); | ||
| const [error, setError] = useState<string | null>(null); | ||
| const [touched, setTouched] = useState(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.
touched는 어떤 경우에 사용이 되는건가요?
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 랑 pw를 입력하다가 다른곳을 클릭해 포커스가 나가면, 그때부터 touched 가 true가 되고 오류가 있으면 오류 메세지가 출력됩니다.
src/page/Login.tsx
Outdated
| const validateEmail = (value: string): string | null => { | ||
| if (!value) return 'ID를 입력해주세요.'; | ||
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||
| if (!emailRegex.test(value)) return 'ID는 이메일 형식으로 입력해주세요.'; | ||
| return null; | ||
| }; | ||
|
|
||
| const validatePassword = (value: string): string | null => { | ||
| if (!value) return 'PW를 입력해주세요.'; | ||
| if (value.length < 8) return 'PW는 최소 8글자 이상이어야 합니다.'; | ||
| 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.
이부분들은 컴포넌트에서 사용하지만 컴포넌트 파일에 있을 필요는 없어보여요
추가적으로 유효성 조건이 길어질 수록 컴포넌트를 읽기 더 불편할거 같아요
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.
넵 utils 폴더를 만들어 함수를 옮겼습니다! 감사합니다
src/page/Login.tsx
Outdated
| `; | ||
|
|
||
| const InputSection = styled.input` | ||
| const InputSection = styled.input<{ hasError?: boolean }>` |
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도 컴포넌트에 들어가지만 컴포넌트 파일과는 분리하는게 좋아보여요
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/hook/useInput.ts
Outdated
|
|
||
| type ValidatorFn = (value: string) => string | null; | ||
|
|
||
| function useInput(initialValue: string, validator: ValidatorFn) { |
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.
initialValue에 의해 useInput이 완성도를 올려주네요!
다만 useInput을 사용할 때 첫 값 대부분을 ''로 채우게 될거 같은데 🤔
특별하게 지정해줘야 하는 경우가 아니라면 기본으로 채워주면 편할거 같아요
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/page/Login.tsx
Outdated
| <LoginButton onClick={() =>navigate('/')}>로그인</LoginButton> | ||
| <LoginButton | ||
| disabled={!canSubmit} | ||
| onClick={() => canSubmit && navigate('/')} |
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.
onClick으로 분리하면 더 깔끔할거같은데 🤔 조금 만 더 다듬어 볼까요?
분리하면 어떤점이 더 좋아질까요 ❓
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.
넵 알겠습니다! 리뷰해주셔서 감사합니다
컴포넌트 분리 -Login.styled.ts 추가 -utils 폴더와 파일 추가 -초기값 설정 함수 분리 등 리펙토링
…n/react-gift-order into upstream/hyeongchan0
1단계 미션인 로그인 페이지를 구현했습니다