-
Notifications
You must be signed in to change notification settings - Fork 0
[Fix] '다음' CTA 버튼 스크롤에 관계없이 고정 #187 #188
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
Conversation
Walkthrough두 로그인 온보딩 화면에서 내부 Column에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as LoginElderInfoScreen
participant VM as ViewModel
participant API as Backend
Note over UI: 사용자 CTA 버튼 클릭
UI->>UI: 필드별 유효성 검사
alt validation 실패
UI-->>User: Snackbar(필요 메시지)
else validation 성공
UI->>VM: initElderHealthData(data)
VM->>API: postElderBulk(data)
API-->>VM: 200 OK
VM-->>UI: navigateToRegisterElderHealth()
end
sequenceDiagram
autonumber
participant UI2 as LoginElderMedInfoScreen
participant VM2 as ViewModel
participant API2 as Backend
Note over UI2: 사용자 CTA 버튼 클릭
UI2->>UI2: (필드 유효성 검사)
alt validation 실패
UI2-->>User: Snackbar(메시지)
else validation 성공
UI2->>VM2: postElderHealthInfoBulk(data)
VM2->>API2: postElderHealthInfoBulk
API2-->>VM2: 200 OK
VM2-->>UI2: navigateToCareCallSetting()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderInfoScreen.kt (1)
195-237: CTAButton 위치가 올바르게 수정되었습니다.스크롤 영역 밖으로 버튼을 이동시켜서 하단에 고정되도록 했네요. 유효성 검사 로직도 각 필드별로 적절히 구현되어 있습니다.
참고: 현재 여러 개의 if-else 체인으로 구성된 유효성 검사 로직을 함수로 추출하면 가독성이 더 좋아질 수 있어요. 하지만 현재 구현도 충분히 동작하므로 선택사항입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderInfoScreen.kt(3 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderMedInfoScreen.kt(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderMedInfoScreen.kt (1)
app/src/main/java/com/konkuk/medicarecall/ui/common/component/CTAButton.kt (1)
CTAButton(24-75)
app/src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderInfoScreen.kt (1)
app/src/main/java/com/konkuk/medicarecall/ui/common/component/CTAButton.kt (1)
CTAButton(24-75)
🪛 GitHub Actions: Android CI
app/src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderMedInfoScreen.kt
[error] 183-183: Detekt: Unexpected blank line(s) before '***' [NoBlankLineBeforeRbrace]
app/src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderInfoScreen.kt
[error] 13-13: Detekt: Unused import [NoUnusedImports]
[error] 191-191: Detekt: Unexpected blank line(s) before '***' [NoBlankLineBeforeRbrace]
[error] 194-194: Detekt: Needless blank line(s) [NoConsecutiveBlankLines]
🔇 Additional comments (3)
app/src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderMedInfoScreen.kt (2)
74-78: 레이아웃 개선 잘 적용되었습니다!스크롤 가능한 Column에
weight(1f)를 적용해서 사용 가능한 공간을 모두 차지하도록 했네요. 이렇게 하면 CTAButton이 하단에 고정될 수 있어요. PR 목표에 딱 맞는 수정입니다.
185-195: CTAButton 배치가 올바르게 수정되었습니다.버튼을 스크롤 가능한 Column 밖으로 이동시켜서 스크롤과 관계없이 하단에 고정되도록 했네요. PR의 의도대로 잘 구현되었습니다.
app/src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderInfoScreen.kt (1)
87-91: 레이아웃 수정이 적절합니다.
weight(1f)적용으로 스크롤 가능한 영역이 사용 가능한 공간을 차지하게 되어, CTAButton을 하단에 고정할 수 있게 되었어요.
...src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderInfoScreen.kt
Outdated
Show resolved
Hide resolved
...src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderInfoScreen.kt
Outdated
Show resolved
Hide resolved
...src/main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderInfoScreen.kt
Outdated
Show resolved
Hide resolved
.../main/java/com/konkuk/medicarecall/ui/feature/login/senior/screen/LoginElderMedInfoScreen.kt
Outdated
Show resolved
Hide resolved
alswlekk
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.
👍
librawish808
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.
좋습니다!
🔗 관련 이슈
📙 작업 설명
📸 스크린샷 또는 시연 영상 (선택)
Summary by CodeRabbit
출시 노트
스타일
버그 수정