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

refactor: 회원가입 로그인페이지 리팩토링 #185

Merged
merged 10 commits into from
Oct 15, 2024

Conversation

hakyoung12
Copy link
Contributor

@hakyoung12 hakyoung12 commented Oct 14, 2024

✏️ 작업 내용

  • 인증오류 메시지 상수로 변경
  • auth 페이지 이미지 잘림 문제 해결
  • user context 삭제
  • Input, PasswordInput 컴포넌트 통합

📷 스크린샷

모바일 및 태블릿사이즈에서 이미지 잘림

스크린샷 2024-10-14 오후 2 25 39

해결

스크린샷 2024-10-14 오후 2 26 41

close #125

@hakyoung12 hakyoung12 added the 🧩 Refactor 리팩토링 label Oct 14, 2024
@hakyoung12 hakyoung12 self-assigned this Oct 14, 2024
Comment on lines +37 to +56
switch (true) {
case error.message.includes('잘못된 이메일 또는 비밀번호입니다'):
setError('email', {
type: 'manual',
message: '잘못된 이메일 또는 비밀번호입니다',
});
setError('password', {
type: 'manual',
message: '잘못된 이메일 또는 비밀번호입니다',
});
break;
case error.message.includes('유효한 이메일 주소를 입력하세요'):
setError('email', {
type: 'manual',
message: '유효한 이메일 주소를 입력하세요',
});
break;
default:
console.error('Error:', error.message);
break;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if else문으로 되어있던 로직 switch문으로 변경

Copy link
Contributor

Choose a reason for hiding this comment

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

훨씬 보기 좋은 것 같아요!

Copy link
Contributor

Choose a reason for hiding this comment

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

switch에 true는 처음 보는 유형이네요 ~

Copy link
Contributor

Choose a reason for hiding this comment

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

P4) 좋은 방법인 것 같습니다! 혹시 if-else 문으로 안 쓰신 이슈가 있으실까요? 직관성을 위해서일까요?
궁금해서 여쭤봅니다..!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

성능보다는 가독성과 직관성때문에 switch문으로 바꿨습니다!
그리고 만약 다른 에러메시지를 추가할 때에 추가하기 용이하기 때문이기도 합니다

그리고 쿠키는 파일을 옮겨놓겠습니다!

{
hasError = false,
className = '',
isPassword = false,
Copy link
Contributor Author

@hakyoung12 hakyoung12 Oct 14, 2024

Choose a reason for hiding this comment

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

isPassword prop은 입력 필드가 비밀번호 입력용인지 여부를 전달하여 비밀번호 토글버튼을 렌더링할지 결정합니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

고정값으로 되어있던 width와 height를 삭제했습니다.

Copy link
Contributor

@yunchaeney yunchaeney left a comment

Choose a reason for hiding this comment

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

리팩토링 고수시네요 👍 고생하셨습니다!

Comment on lines +6 to +15
const ERROR_MESSAGES = {
REQUIRED_EMAIL: '이메일을 입력해주세요.',
INVALID_EMAIL: '이메일 형식을 입력해주세요.',
REQUIRED_PASSWORD: '비밀번호를 입력해주세요.',
REQUIRED_NAME: '이름을 입력해주세요.',
REQUIRED_COMPANY: '회사명을 입력해주세요.',
INVALID_PASSWORD:
'비밀번호는 영문과 숫자가 포함된 8자 이상이 되도록 해 주세요.',
PASSWORD_MISMATCH: '비밀번호가 일치하지 않습니다.',
};
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines +37 to +56
switch (true) {
case error.message.includes('잘못된 이메일 또는 비밀번호입니다'):
setError('email', {
type: 'manual',
message: '잘못된 이메일 또는 비밀번호입니다',
});
setError('password', {
type: 'manual',
message: '잘못된 이메일 또는 비밀번호입니다',
});
break;
case error.message.includes('유효한 이메일 주소를 입력하세요'):
setError('email', {
type: 'manual',
message: '유효한 이메일 주소를 입력하세요',
});
break;
default:
console.error('Error:', error.message);
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

훨씬 보기 좋은 것 같아요!

Copy link
Contributor

@BeMatthewsong BeMatthewsong left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! LGTM 👏

Comment on lines +37 to +56
switch (true) {
case error.message.includes('잘못된 이메일 또는 비밀번호입니다'):
setError('email', {
type: 'manual',
message: '잘못된 이메일 또는 비밀번호입니다',
});
setError('password', {
type: 'manual',
message: '잘못된 이메일 또는 비밀번호입니다',
});
break;
case error.message.includes('유효한 이메일 주소를 입력하세요'):
setError('email', {
type: 'manual',
message: '유효한 이메일 주소를 입력하세요',
});
break;
default:
console.error('Error:', error.message);
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

switch에 true는 처음 보는 유형이네요 ~

Copy link
Contributor

@HMRyu HMRyu left a comment

Choose a reason for hiding this comment

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

리팩토링 하면서 코드가 훨씬 더 깔끔해진 것 같습니다!

고생 많으셨습니다...! 👍👍

코멘트 확인 한 번 부탁드립니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 훨씬 더 간단하게 사용할 수 있겠네요!

Comment on lines +37 to +56
switch (true) {
case error.message.includes('잘못된 이메일 또는 비밀번호입니다'):
setError('email', {
type: 'manual',
message: '잘못된 이메일 또는 비밀번호입니다',
});
setError('password', {
type: 'manual',
message: '잘못된 이메일 또는 비밀번호입니다',
});
break;
case error.message.includes('유효한 이메일 주소를 입력하세요'):
setError('email', {
type: 'manual',
message: '유효한 이메일 주소를 입력하세요',
});
break;
default:
console.error('Error:', error.message);
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

P4) 좋은 방법인 것 같습니다! 혹시 if-else 문으로 안 쓰신 이슈가 있으실까요? 직관성을 위해서일까요?
궁금해서 여쭤봅니다..!

Comment on lines 44 to 48
<SavedGatheringProvider>
<QueryClientProvider client={queryClient}>
{children}
</QueryClientProvider>
</SavedGatheringProvider>
Copy link
Contributor

Choose a reason for hiding this comment

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

P4) 유저 정보를 최상위 서버 컴포넌트에서 받기 때문에 UserProvider 는 삭제하신 걸까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 이제 사용하시는 분이 없어서 제거했습니다!

@HMRyu
Copy link
Contributor

HMRyu commented Oct 14, 2024

그리고 혹시 cookie.ts 파일 위치 변경을 요청드려도 될까요?

현재

src/actions/auth/cookie

현재는 이곳에 저장되고 있는데,

  • actions 폴더가 중복인 점 (/api/actions)
  • server actions 를 사용하고는 있지만 쿠키는 일반적인 CRUD 와 구분하면 좋을 것 같다는 생각

위 두 이유로 쿠키는 api/auths/ 쪽에 저장되어도 괜찮을 것 같다는 생각이 들었습니다..!

@hakyoung12 hakyoung12 merged commit ec192e5 into develop Oct 15, 2024
4 checks passed
@hakyoung12 hakyoung12 deleted the refactor/go/KAN-103-refactor-auth-pages branch October 16, 2024 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧩 Refactor 리팩토링
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor: 로그인 회원가입 페이지 리팩토링
4 participants