-
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
[WEAV-79] 이름 입력 뷰 UI 구현 #25
Conversation
Walkthrough이 변경 사항은 Changes
Possibly related PRs
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: 3
🧹 Outside diff range and nitpick comments (5)
Projects/Core/CommonKit/Sources/Path/PathTypes.swift (3)
37-37
: LGTM! 새로운 케이스가 잘 추가되었습니다.
authName
케이스와 해당하는 이름이 적절하게 추가되었습니다. 코드 구조와 일관성이 잘 유지되고 있습니다.가독성 향상을 위해
switch
문 내의 케이스들을 알파벳 순서로 정렬하는 것을 고려해 보시는 것은 어떨까요? 이렇게 하면 향후 새로운 케이스를 추가할 때 일관된 위치에 배치하기 쉬워집니다.
51-51
: LGTM!SignUpSubViewType
에 새로운 케이스가 잘 추가되었습니다.
authName
케이스가PathType
enum의 변경사항과 일관되게 추가되었습니다. 네이밍 컨벤션도 잘 지켜졌습니다.
PathType
enum에 대한 제안과 마찬가지로, 여기서도 케이스들을 알파벳 순서로 정렬하는 것을 고려해 보시는 것이 어떨까요? 이렇게 하면 두 enum 간의 일관성을 유지하고 코드 가독성을 향상시킬 수 있습니다.
Line range hint
1-51
: 전체적으로 잘 구현되었습니다. 몇 가지 개선 사항을 제안드립니다.
두 enum에서 케이스들을 알파벳 순서로 정렬하는 것을 고려해 보세요. 이는 코드의 일관성과 가독성을 향상시킬 것입니다.
SignUpSubViewType
에 대한 문서화 주석을 추가하는 것이 좋겠습니다. 각 케이스가 어떤 단계를 나타내는지 간단히 설명하면 코드의 이해도를 높일 수 있습니다.
PathType
의name
프로퍼티에 대한 문서화 주석도 추가하면 좋겠습니다. 이 프로퍼티의 용도와 반환값의 의미를 설명하면 도움이 될 것 같습니다.이러한 개선사항들은 코드의 품질을 더욱 향상시키고, 향후 유지보수를 용이하게 할 것입니다.
Projects/Features/SignUp/Sources/AuthPhoneInput/AuthPhoneInputView.swift (1)
Line range hint
1-103
: 전반적으로 잘 구현된 것 같습니다.전화번호 입력 UI의 구현이 깔끔하고 체계적으로 되어 있습니다. 상태 관리, 유효성 검사, 그리고 사용자 인터페이스 구성이 잘 되어 있습니다. "010-" 접두사를 추가한 변경사항은 사용자 경험을 개선하는 좋은 시도로 보입니다.
추가적인 제안:
- 접근성(Accessibility) 지원을 위한 코드를 추가하는 것을 고려해 보세요.
- 현재 구현에 대한 단위 테스트를 작성하여 코드의 안정성을 높이는 것이 좋을 것 같습니다.
Projects/Features/SignUp/Sources/ProfileInput/AuthProfileAge/AuthProfileAgeInputView.swift (1)
80-81
: 코드 변경 요약 및 제안이 변경사항은 사용자 흐름에 중요한 영향을 미칩니다. 다음 단계를 위해 제안드립니다:
- TODO 항목에 대한 구체적인 계획을 수립하세요.
- 현재의 임시 구현이 적절한지 팀과 논의하세요.
- 사용자 경험 관점에서 이 흐름을 검토하고 필요한 경우 조정하세요.
전체적인 네비게이션 흐름과
AppCoordinator
의 사용이 앱의 아키텍처와 일관성이 있는지 확인하세요. 필요하다면 이 부분에 대한 설계 검토 세션을 가질 것을 제안합니다.🧰 Tools
🪛 SwiftLint
[Warning] 80-80: TODOs should be resolved (순서 재정의)
(todo)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (6)
Projects/DesignSystem/DesignCore/Resources/Images/Images.xcassets/name-paper.imageset/name-paper.png
is excluded by!**/*.png
Projects/DesignSystem/DesignCore/Resources/Images/Images.xcassets/name-paper.imageset/[email protected]
is excluded by!**/*.png
Projects/DesignSystem/DesignCore/Resources/Images/Images.xcassets/name-paper.imageset/[email protected]
is excluded by!**/*.png
Projects/DesignSystem/DesignCore/Resources/Images/Images.xcassets/pencil.imageset/pencil.png
is excluded by!**/*.png
Projects/DesignSystem/DesignCore/Resources/Images/Images.xcassets/pencil.imageset/[email protected]
is excluded by!**/*.png
Projects/DesignSystem/DesignCore/Resources/Images/Images.xcassets/pencil.imageset/[email protected]
is excluded by!**/*.png
📒 Files selected for processing (8)
- Projects/App/Sources/Navigation/NavigationStack.swift (1 hunks)
- Projects/Core/CommonKit/Sources/Path/PathTypes.swift (2 hunks)
- Projects/DesignSystem/DesignCore/Resources/Images/Images.xcassets/name-paper.imageset/Contents.json (1 hunks)
- Projects/DesignSystem/DesignCore/Resources/Images/Images.xcassets/pencil.imageset/Contents.json (1 hunks)
- Projects/DesignSystem/DesignCore/Sources/CTAButton/CTABottomButton.swift (1 hunks)
- Projects/Features/SignUp/Sources/AuthPhoneInput/AuthPhoneInputView.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthName/AuthNameInputView.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthProfileAge/AuthProfileAgeInputView.swift (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- Projects/DesignSystem/DesignCore/Resources/Images/Images.xcassets/name-paper.imageset/Contents.json
- Projects/DesignSystem/DesignCore/Resources/Images/Images.xcassets/pencil.imageset/Contents.json
🧰 Additional context used
🪛 SwiftLint
Projects/Features/SignUp/Sources/ProfileInput/AuthProfileAge/AuthProfileAgeInputView.swift
[Warning] 80-80: TODOs should be resolved (순서 재정의)
(todo)
🔇 Additional comments (9)
Projects/App/Sources/Navigation/NavigationStack.swift (1)
37-38
: LGTM! 이름 입력 뷰가 성공적으로 추가되었습니다.변경 사항이 PR의 목적과 일치하며 기존 코드 구조를 잘 따르고 있습니다. 다음 사항들을 고려해 보시기 바랍니다:
AuthNameInputView
가 다른 파일에 정의되어 있는지 확인하세요.- 필요한 경우
import
문을 추가했는지 확인하세요.- 새로운 뷰로의 네비게이션이 다른 곳에서 올바르게 처리되고 있는지 확인하세요.
다음 스크립트를 실행하여
AuthNameInputView
의 정의와 사용을 확인하세요:✅ Verification successful
변경 사항이 정상적으로 반영되었습니다.
AuthNameInputView
가 올바르게 정의되고NavigationStack.swift
에서 적절히 사용되고 있음을 확인했습니다. 추가적인 수정 사항은 필요하지 않습니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # AuthNameInputView의 정의와 사용을 확인합니다. echo "AuthNameInputView 정의 검색:" rg --type swift "struct AuthNameInputView" echo "\nAuthNameInputView 사용 검색:" rg --type swift "AuthNameInputView\(\)"Length of output: 524
Script:
#!/bin/bash # NavigationStack.swift에서 AuthNameInputView 사용을 위한 import 확인 echo "NavigationStack.swift에서 SignUp 모듈의 import 상태 확인:" rg --type swift "^import SignUp" Projects/App/Sources/Navigation/NavigationStack.swiftLength of output: 209
Projects/Features/SignUp/Sources/ProfileInput/AuthName/AuthNameInputView.swift (3)
9-16
: LGTM: 적절한 import 문과 구조체 선언import 문과 구조체 선언이 적절하게 작성되었습니다. SwiftUI 뷰에 필요한 모듈들을 가져오고 있으며, 구조체를 public으로 선언하여 모듈 외부에서도 사용할 수 있게 했습니다.
1-88
: 전체 리뷰 요약전반적으로
AuthNameInputView
의 구현이 잘 되어 있습니다. PR 목표에 맞게 이름 입력 뷰 UI가 적절히 구현되었습니다. 다음은 주요 포인트들입니다:
- 구조: SwiftUI를 사용한 뷰 구조가 명확하고 이해하기 쉽습니다.
- 디자인: DesignCore를 활용하여 일관된 디자인을 적용했습니다.
- 기능: 이름 입력 및 다음 버튼 활성화 로직이 구현되어 있습니다.
개선을 위한 제안:
- 접근성: VoiceOver 지원을 위한 접근성 레이블 추가
- 키보드 처리: 키보드가 입력 필드를 가리지 않도록 처리
- 코드 구조: 매직 넘버를 상수로 추출하여 가독성 향상
- 프리뷰: 다양한 상태와 다크 모드를 테스트할 수 있는 프리뷰 추가
마지막으로, 네이밍 컨벤션 준수 여부를 확인하기 위해 다음 스크립트를 실행해 보겠습니다:
이 검증을 통해 프로젝트의 네이밍 컨벤션 준수 여부를 확인할 수 있습니다.
✅ Verification successful
네이밍 컨벤션 검증 완료
AuthNameInputView
파일 내에서 변수 및 타입 네이밍이 프로젝트의 규칙을 준수하고 있음을 확인했습니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check naming conventions in the project # Test: Search for camelCase for variable and function names, PascalCase for type names rg --type swift '(let|var|func) [a-z][a-zA-Z0-9]*' Projects/Features/SignUp/Sources/ProfileInput/AuthName/AuthNameInputView.swift rg --type swift '(struct|class|enum) [A-Z][a-zA-Z0-9]*' Projects/Features/SignUp/Sources/ProfileInput/AuthName/AuthNameInputView.swiftLength of output: 413
36-80
: 🛠️ Refactor suggestion개선 제안 및 명확화 요청
전체적인 뷰 구조와 구현이 잘 되어 있습니다. 다음 사항들을 고려해 보시기 바랍니다:
접근성: VoiceOver 사용자를 위해 주요 UI 요소에 접근성 레이블을 추가하는 것이 좋습니다.
키보드 처리: 키보드가 텍스트 필드를 가리지 않도록 처리가 필요할 수 있습니다.
입력 유효성 검사: 현재 2자 이상으로만 제한되어 있는데, 추가적인 유효성 검사가 필요한지 확인해 주세요.
네비게이션: AppCoordinator를 통한 pop 동작의 의도와 동작 방식에 대해 설명해 주실 수 있나요?
접근성 개선을 위해 다음과 같은 수정을 제안합니다:
TextField( "김위브", text: $inputText ) .accessibilityLabel("이름 입력")키보드 처리와 관련하여 다음 스크립트로 확인해 보겠습니다:
이 결과를 바탕으로 키보드 처리 로직의 추가 여부를 결정할 수 있습니다.
✅ Verification successful
확인 완료: 키보드 처리가 적절히 구현되어 있습니다.
프로젝트 전반에 걸쳐
KeyboardResponder
를 사용하여 키보드 표시 시 텍스트 필드가 가려지지 않도록 처리되고 있습니다.AuthNameInputView.swift
에서도.ignoresSafeArea()
가 사용되어 추가적인 키보드 처리가 불필요합니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if keyboard avoidance is implemented elsewhere in the project # Test: Search for keyboard avoidance related code rg --type swift "keyboardAvoidance|ignoresSafeArea|UIResponder.keyboardWillShow"Length of output: 1808
Projects/Features/SignUp/Sources/AuthPhoneInput/AuthPhoneInputView.swift (1)
16-16
: 전화번호 입력 필드의 기본값 변경이 적절해 보입니다."010-"로 초기화하는 것은 한국 휴대폰 번호 입력을 위한 좋은 UX 개선입니다. 사용자 경험을 향상시키고 입력 오류를 줄일 수 있을 것 같습니다.
다만, 다음 사항들을 고려해 보시는 것이 어떨까요?:
- 일반 전화번호나 국제 전화번호를 입력해야 하는 경우
- 사용자가 "010-" 접두사를 지우려고 할 때의 동작
이러한 edge case들을 처리하는 로직이
PhoneTextInputView
에 이미 구현되어 있는지 확인해 주세요. 아래 스크립트로 관련 코드를 확인할 수 있습니다:✅ Verification successful
전화번호 입력 필드의 기본값 변경이 적절하게 검증되었습니다.
변경사항은 "010-"으로 초기화하여 한국 휴대폰 번호 입력에 적합하며, 다양한 엣지 케이스에 대한 유효성 검사 및 포매팅이 이미 구현되어 있어 문제되지 않습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for phone number formatting and validation logic # Test: Search for phone number formatting logic echo "Searching for phone number formatting logic:" rg --type swift 'formattedPhoneNumber|isValidPhoneNumber' -A 5Length of output: 13029
Projects/DesignSystem/DesignCore/Sources/CTAButton/CTABottomButton.swift (4)
12-14
: 제네릭 타입 제거로 코드가 단순화되었습니다.제네릭 파라미터를 제거하고
AnyShapeStyle
을 사용하여 코드가 간소화되었습니다. 이는 유지보수성을 향상시키고 코드의 복잡성을 줄일 수 있습니다.
21-21
: 사용되지 않는maxKeyboardHeight
변수 확인 필요
@State private var maxKeyboardHeight: CGFloat = 0
변수가 선언되었지만 코드 내에서 사용되지 않는 것으로 보입니다. 필요 없는 변수라면 제거를 고려해 주세요.
25-30
: 초기화 메서드의 파라미터 타입 변경으로 유연성 증가
backgroundStyle
의 파라미터 타입을 제네릭에서some ShapeStyle
로 변경하여 다양한ShapeStyle
을 전달할 수 있게 되었습니다. 이는 코드의 유연성을 높입니다.
35-37
:buttonBackgroundColor
의 반환 타입 변경으로 일관성 확보
buttonBackgroundColor
의 반환 타입을AnyShapeStyle
로 변경하여backgroundStyle
과 일관성을 유지하였습니다. 비활성 상태일 때 반환되는 색상을AnyShapeStyle
로 래핑하여 타입 일치성을 보장했습니다.
구현사항
스크린샷(선택)
Summary by CodeRabbit