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

♻️ 회원가입 페이지 sms 개선 #64

Merged
merged 1 commit into from
Dec 23, 2024
Merged

Conversation

Ethen1264
Copy link
Contributor

@Ethen1264 Ethen1264 commented Dec 23, 2024

💡 배경 및 개요

회원가입 페이지 sms 개선

📃 작업내용

코드 전송 버튼 팬딩 설정
코드 확인 버튼 팬딩 설정
상황에 따른 disabled 설정
tanstack query로 변경

Summary by CodeRabbit

  • 새로운 기능

    • SMS 코드 검증 기능 강화: 성공적인 검증 시 성공 메시지 표시 및 상태 업데이트.
    • SMS 전송 및 검증 상태를 관리하는 새로운 상태 변수 isSmsVerified 추가.
  • 버그 수정

    • SMS 전송 실패 시 오류 메시지 수정.
  • UI 개선

    • 로딩 컴포넌트의 레이아웃 조정으로 전체 화면 너비 사용.
    • 버튼 텍스트 업데이트: SMS 검증 완료 시 "인증 완료"로 변경.

@Ethen1264 Ethen1264 added ♻️ Refactor 코드 리팩토링 🐞 Bug 버그 발생 labels Dec 23, 2024
@Ethen1264 Ethen1264 self-assigned this Dec 23, 2024
Copy link

coderabbitai bot commented Dec 23, 2024

Walkthrough

이 풀 리퀘스트는 SMS 인증 프로세스와 관련된 여러 파일의 기능을 개선합니다. 주요 변경 사항은 SMS 코드 확인, 인증 상태 추적, 타이머 관리 등을 포함하며, 사용자 경험을 향상시키기 위해 상태 관리와 UI 로직을 세밀하게 조정했습니다.

Changes

파일 변경 요약
src/entities/signUp/api/getCheckSmsCode.ts SMS 코드 확인 API 응답 처리 개선, 성공/실패 시 토스트 메시지 추가
src/entities/signUp/model/useCheckSmsCode.ts SMS 인증 상태 설정을 위한 setIsSmsVerified 매개변수 추가
src/entities/signUp/model/useSendSms.ts setIsSmsSent 매개변수 복원, 오류 메시지 텍스트 수정
src/entities/signUp/model/useTimer.ts isSmsVerified 매개변수 추가, 타이머 로직 업데이트
src/entities/signUp/ui/SignUpForm/index.tsx SMS 인증 상태 추적을 위한 isSmsVerified 상태 도입, 버튼 및 입력 필드 동작 개선
src/shared/ui/Loading/index.tsx 로딩 컴포넌트의 레이아웃 클래스에 w-screen 추가

Poem

🐰 SMS 토끼의 인증 노래 🔐

코드를 보내고 기다리네
검증의 춤을 추는 중
상태는 흐르고 타이머 돈다
인증의 길을 밝히는 빛
토끼의 마법, 완벽한 Sign Up! 🎉

Tip

CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command @coderabbitai generate docstrings to have CodeRabbit automatically generate docstrings for your pull request. We would love to hear your feedback on Discord.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 170b288 and 9a440af.

📒 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:

  1. The complete implementation of useTimer hook
  2. Any test cases that verify the behavior
  3. 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}
Copy link

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:

  1. isCheckingCode is isPending from useCheckSmsCode hook
  2. The button's disabled condition is !watch('code') || isSmsVerified || !isCheckingCode
  3. 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

@Ethen1264 Ethen1264 merged commit 9bca178 into develop Dec 23, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug 버그 발생 ♻️ Refactor 코드 리팩토링
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants