-
Notifications
You must be signed in to change notification settings - Fork 0
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
♻️ 회원가입 페이지 sms 개선 #64
Conversation
Walkthrough이 풀 리퀘스트는 SMS 인증 프로세스와 관련된 여러 파일의 기능을 개선합니다. 주요 변경 사항은 SMS 코드 확인, 인증 상태 추적, 타이머 관리 등을 포함하며, 사용자 경험을 향상시키기 위해 상태 관리와 UI 로직을 세밀하게 조정했습니다. Changes
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/entities/signUp/api/getCheckSmsCode.ts (1)
13-19
: Http 상태 코드별 추가 처리 로직 권장
현재 200이 아닌 경우를 에러로 처리하고 있으나, 400/500 등 개별 상태 코드에 따른 상세 처리(예: 토스트 메시지 다변화 등)가 필요할 수 있습니다. 추후 확장성을 고려해 상태 코드별 분기 처리를 검토해 주세요.src/entities/signUp/model/useCheckSmsCode.ts (1)
4-8
: 파라미터 증가에 따른 책임 분산 여부 점검
새로 추가된 setIsSmsVerified로 인해 Hook의 역할이 커졌습니다. 이 hook이 지나치게 많은 책임을 지지 않도록, 필요하다면 로직을 적절히 분리할지 고려해 주세요.src/entities/signUp/ui/SignUpForm/index.tsx (3)
31-31
: isSmsVerified 상태 변수 추가 확인
이 변수는 SMS 검증 결과를 관리하기 위해 추가된 것으로 보입니다. SMS 검증 완료 시 다른 입력 요소와 상호 작용이 제한될 수 있으므로, 다른 컴포넌트나 훅에서도 isSmsVerified 상태를 적절히 활용하고 있는지 확인해 주세요.
38-41
: useCheckSmsCode 훅의 isPending 상태 도입
isPending을 통해 인증 코드 확인 중이라는 로딩 상태를 나타낼 수 있어, 사용자 경험을 향상시키는 데 유용합니다. 인증 과정에서 에러 처리가 필요한 경우, 적절한 에러 메시지도 함께 처리해 주시면 좋겠습니다.
44-44
: useTimer 호출 시 isSmsVerified 인자 활용
검증이 완료되면 타이머 중단 또는 비활성화 처리가 가능해 보입니다. 만약 타이머가 필요 없어지는 시점이 명확하다면, useEffect 내부에서 isSmsVerified로 타이머를 중단하거나 초기화하도록 처리하는 것도 고려해 보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/entities/signUp/api/getCheckSmsCode.ts
(1 hunks)src/entities/signUp/model/useCheckSmsCode.ts
(1 hunks)src/entities/signUp/model/useSendSms.ts
(2 hunks)src/entities/signUp/model/useTimer.ts
(1 hunks)src/entities/signUp/ui/SignUpForm/index.tsx
(3 hunks)src/shared/ui/Loading/index.tsx
(1 hunks)
🔇 Additional comments (11)
src/entities/signUp/model/useSendSms.ts (2)
7-7
: 매개변수 복원 확인
setIsSmsSent 매개변수 복원이 정상적으로 이루어졌습니다. 이로 인해 SMS 전송 상태 플래그를 간편하게 관리할 수 있게 되었습니다. 변경 취지에 부합하므로 유지하셔도 좋겠습니다.
17-17
: 오타 수정 확인
에러 메시지 내 "문제" → "문자" 오타가 수정되어 사용자 혼선을 줄일 수 있습니다. 유효한 수정입니다.
src/entities/signUp/model/useCheckSmsCode.ts (1)
11-17
: 결과 반환 값과 상태 동기화 확인
getCheckSmsCode의 반환값(불리언)에 따라 setIsSmsVerified를 호출하는 로직이 명확합니다. 단, 불리언 외 다른 예외 상황(예: 네트워크 에러, 응답 구조 변경 등) 처리 방안을 추가 검토하면 안정성이 향상됩니다.
✅ Verification successful
Let me gather more information about the error handling and response structure by examining the codebase.
Now I can generate the final response based on the implementation details found in the codebase.
예외 처리가 이미 적절히 구현되어 있습니다
getCheckSmsCode 함수에서 다음과 같은 예외 상황들이 이미 처리되고 있습니다:
- 입력값 검증: phoneNumber와 code가 없는 경우
- 네트워크 응답: HTTP 상태 코드 200 여부에 따른 처리
- 사용자 피드백: toast 메시지를 통한 성공/실패 알림
또한 useCheckSmsCode에서 result 타입 체크를 통해 상태 업데이트를 안전하게 수행하고 있어 추가적인 예외 처리가 필요하지 않습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Check the implementation of getCheckSmsCode
ast-grep --pattern 'const getCheckSmsCode = $_'
# Search for any error handling patterns or type definitions related to SMS verification
rg "getCheckSmsCode" -A 5 -B 5
# Look for any type definitions or interfaces related to SMS verification
rg "interface.*Sms" -A 5
rg "type.*Sms" -A 5
Length of output: 3255
src/entities/signUp/model/useTimer.ts (2)
6-7
: 의미 있는 파라미터 추가 확인
타이머 로직과 SMS 인증 상태(isSmsVerified)가 결합되어, 필요 시 타이머를 중단하거나 초기화할 수 있게 된 점이 유효해 보입니다.
10-20
: SMS 인증 완료 시점 처리 로직 검토
타이머가 0이거나 isSmsVerified가 true일 경우 setIsSmsSent를 false로 바꾸고 timer를 0으로 초기화하는 방식이 명확합니다. 다만, 콜백이 동시에 여러 번 호출될 가능성 등을 고려하여 상태 업데이트 흐름을 확인해 주세요.
✅ Verification successful
Let me gather more information about the state management and callback handling in this component.
This will help us understand:
- The complete implementation of useTimer hook
- Any test cases that verify the behavior
- Related state management patterns in the codebase
Let me check the complete implementation of the useTimer hook to better understand the state management flow.
상태 업데이트 로직이 안전하게 구현되어 있음을 확인했습니다
useTimer 훅의 구현을 검토한 결과, 다음과 같은 이유로 상태 업데이트가 안전하게 처리되고 있습니다:
- useEffect의 의존성 배열에 timer, setTimer, isSmsVerified가 올바르게 포함되어 있어 관련 상태 변경 시 적절히 반응합니다
- setTimer는 함수형 업데이트를 사용하여 이전 상태를 안전하게 참조합니다
- 타이머 종료(timer === 0)와 SMS 인증 완료(isSmsVerified === true) 시의 상태 초기화가 단일 조건문에서 처리되어 경쟁 상태를 방지합니다
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for the component implementation and related hooks
ast-grep --pattern 'const useTimer = ($_) => {
$$$
}'
# Search for any tests related to this timer functionality
rg "useTimer" -A 5
# Look for related state management
ast-grep --pattern 'const [$_, $_] = useState($_)'
Length of output: 2516
Script:
#!/bin/bash
# Get the complete implementation of useTimer
cat src/entities/signUp/model/useTimer.ts
# Look for any related SMS verification logic
rg "isSmsVerified" -A 3
Length of output: 3555
src/entities/signUp/ui/SignUpForm/index.tsx (5)
33-36
: useSendSms 훅의 isPending 상태 도입
isPending 상태가 추가되어 sendSms가 진행 중임을 명확히 파악할 수 있게 되었습니다. 이 상태를 활용하여 로딩 UI 표시나 버튼 비활성화 등을 진행하면 사용자 경험 개선에 도움이 될 것 같습니다.
125-125
: 연락처 입력 필드 비활성화
isSmsVerified가 true일 경우 연락처 수정이 불가능하도록 처리하여, 이미 인증된 번호를 바꾸는 실수를 방지하고 있습니다. 적절한 로직으로 보입니다.
139-139
: 인증번호 입력 필드 비활성화
이미 인증 요청이 완료되지 않았거나, 인증이 완료되었을 때 필드를 비활성화하여 사용자의 오입력을 막는 로직이 유효해 보입니다.
160-164
: 버튼 텍스트 동적 변경
인증 완료, 재발송, 발송 등 다양한 상태를 직관적으로 표시합니다. 사용자 입장에서 상태를 명확히 인지할 수 있어, 매우 유익한 UI 개선입니다.
153-158
: 인증번호 발송 버튼 disabled 조건
isSendingSms, isSmsSent, isSmsVerified 등 다양한 조건에 의해 버튼이 비활성화됩니다. 로직 상 타이밍에 문제가 없는지, 조건 간 충돌이 없는지 다시 한번 체크해 주세요.
✅ Verification successful
Let me gather more information about the SMS verification states and their usage in the codebase.
Let me check the SMS verification hooks to understand the complete flow.
SMS 인증 상태 관리 로직이 적절하게 구현되어 있습니다
SMS 인증 관련 상태들의 흐름을 분석한 결과, 다음과 같이 적절하게 구현되어 있습니다:
isSendingSms
: SMS 발송 중일 때만 활성화되며useSendSms
훅에서 관리됩니다isSmsSent
: SMS 발송 성공 시 활성화되고 타이머가 만료되면 비활성화됩니다isSmsVerified
: 인증번호 확인이 성공적일 때만 활성화되며, 이후 전화번호 입력필드와 인증 관련 버튼들이 모두 비활성화됩니다- 타이머는 SMS 발송 성공 시 180초로 설정되며, 시간 만료나 인증 완료 시 자동으로 리셋됩니다
각 상태 간의 전환이 명확하고 서로 충돌하지 않으며, 사용자 경험 측면에서도 적절한 순서로 처리됩니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for the state variables and their usage
ast-grep --pattern 'const $_ = $state($$$)'
# Search for any related SMS verification logic
rg "isSendingSms|isSmsSent|isSmsVerified" -A 5 -B 5
Length of output: 6610
Script:
#!/bin/bash
# Search for SMS related hooks implementation
rg "useSendSms|useCheckSmsCode" -A 10
# Search for any error handling or state transitions
ast-grep --pattern 'setIsSmsSent($$$)'
ast-grep --pattern 'setIsSmsVerified($$$)'
Length of output: 3993
src/shared/ui/Loading/index.tsx (1)
22-22
: 전체 화면 대비 Loading 컴포넌트 확장
w-screen 클래스를 추가하여 로딩 컴포넌트가 화면 너비 전체를 차지하게 하였습니다. 다양한 해상도에서 로딩 화면이 깔끔하게 맞춰지도록 도와줄 것으로 보입니다.
/> | ||
<Button | ||
onClick={() => checkSmsCode()} | ||
text="확인" | ||
width="20%" | ||
disabled={!isSmsSent} | ||
disabled={!watch('code') || isSmsVerified || !isCheckingCode} |
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.
💡 Codebase verification
Let me analyze the code context we have. I can see that:
isCheckingCode
isisPending
fromuseCheckSmsCode
hook- The button's disabled condition is
!watch('code') || isSmsVerified || !isCheckingCode
useCheckSmsCode
is a query hook that verifies the SMS code
I can now provide the final response:
SMS 인증 버튼의 disabled 로직 수정 필요
현재 !isCheckingCode
조건으로 인해 로딩 상태가 아닐 때만 버튼이 활성화되는데, 이는 잘못된 로직입니다. 일반적인 UX 패턴에서는 오히려 인증 확인 중일 때(isCheckingCode === true
) 버튼을 비활성화하는 것이 바람직합니다. disabled 조건을 다음과 같이 수정하는 것을 추천드립니다:
disabled={!watch('code') || isSmsVerified || isCheckingCode}
🔗 Analysis chain
확인 버튼 disabled 조건 재검토
현재 확인 버튼은 “코드 값 없음 || 이미 인증된 상태 || !isCheckingCode” 조건에서 비활성화됩니다. isCheckingCode는 ‘인증 확인 과정이 진행 중임’을 나타내므로, 로딩 상태일 때 버튼을 활성화해도 되는지 혹은 비활성화해야 하는지 사용성 측면을 한번 더 고민해 보시면 좋겠습니다.
Let me gather more information about the code context to better understand the verification flow and usage of isCheckingCode
.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for the SignUpForm component implementation and related state management
ast-grep --pattern 'const SignUpForm = $_' -l typescript
# Find usages of isCheckingCode
rg "isCheckingCode" -A 3 -B 3
# Look for any verification or SMS-related state management
rg "isSmsVerified|setIsSmsVerified" -A 3 -B 3
Length of output: 6969
💡 배경 및 개요
회원가입 페이지 sms 개선
📃 작업내용
코드 전송 버튼 팬딩 설정
코드 확인 버튼 팬딩 설정
상황에 따른 disabled 설정
tanstack query로 변경
Summary by CodeRabbit
새로운 기능
isSmsVerified
추가.버그 수정
UI 개선