Conversation
📝 WalkthroughWalkthrough온보딩 기능을 위한 신규 feature/onboarding 모듈이 추가되었습니다. 커플 연결, 프로필, D-day 화면을 포함한 네비게이션 그래프와 관련 라우트(NavRoutes) 및 NavGraphContributor가 도입되었고, Koin DI 모듈(onBoardingModule)과 ViewModel(OnBoardingViewModel), 상태/인텐트/사이드이펙트 모델, 프로필 입력 UI 컴포넌트(NameTextField, OnBoardingTopbar, ProfileRoute 등), 도메인 NickName 값 클래스와 예외 타입이 추가되었습니다. 리소스(drawables, strings)와 Gradle/settings 변경(app 모듈 의존성 등록 포함)도 포함됩니다. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@feature/onboarding/src/main/java/com/twix/onboarding/couple/CoupleConnectRoute.kt`:
- Around line 3-24: Replace hardcoded Korean strings in CoupleConnectRoute with
string resources: use stringResource(R.string.onboarding_couple_connect_title)
for the screen title (fix the typo to "커플 등록 화면") and
stringResource(R.string.onboarding_next) for the button label; add the import
androidx.compose.ui.res.stringResource and reference the resource IDs
onboarding_couple_connect_title and onboarding_next. Also add a simple Preview
composable (e.g., CoupleConnectRoutePreview) that calls CoupleConnectRoute {} so
the UI can be previewed during development.
In `@feature/onboarding/src/main/java/com/twix/onboarding/dday/DdayRouete.kt`:
- Around line 12-14: Rename the misspelled composable and file: change the
function name DdayRouete to DdayRoute and rename the file DdayRouete.kt to
DdayRoute.kt; update all references/usages and imports where DdayRouete is
referenced (including any navigation registration or tests) so they point to
DdayRoute, and run a quick compile to ensure no remaining symbol references
remain.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/profile/ProfileScreen.kt`:
- Around line 151-160: The AppButton currently looks disabled visually but
remains clickable; update its behavior so it is actually disabled when
uiModel.isValid is false by wiring the enabled state (or passing null/NO-OP for
onClick) from uiModel.isValid into the button control; locate the AppButton
usage in ProfileScreen.kt and either set an enabled parameter (or conditionally
supply onClick = if (uiModel.isValid) { onCompleted } else { null }) so visual
and interactive states match, ensuring any ripple/press behavior is suppressed
when disabled.
🧹 Nitpick comments (12)
core/design-system/src/main/res/drawable/ic_arrow1_m_left.xml (1)
11-11: Dark Mode 지원을 위해 테마 색상 사용을 고려해 주세요.현재
#171717로 하드코딩된 색상은 다크 모드에서 어두운 배경과 대비가 부족하여 아이콘이 보이지 않을 수 있습니다.개선 방안:
android:strokeColor="?attr/colorOnSurface"와 같은 테마 속성을 사용하거나@color/icon_primary같은 색상 리소스를 정의하여values-night폴더에서 다크 모드 대응 색상을 지정할 수 있습니다.- android:strokeColor="#171717" + android:strokeColor="@color/icon_primary"다크 모드 지원이 현재 범위에 포함되지 않는다면, 추후 대응을 위해 이슈로 트래킹하는 것도 좋은 방법입니다.
core/design-system/src/main/res/drawable/ic_check_success.xml (1)
8-8: 디자인 시스템 일관성을 위해 색상 리소스 사용을 권장합니다.성공 색상
#1ED45A가 하드코딩되어 있습니다. 이 녹색은 밝은/어두운 테마 모두에서 비교적 잘 보일 수 있지만, 디자인 시스템의 일관성과 향후 유지보수를 위해 색상 리소스로 분리하는 것이 좋습니다.이점:
- 디자인 시스템에서 success 색상을 한 곳에서 관리할 수 있습니다
- 브랜드 색상 변경 시 일괄 수정이 가능합니다
- 접근성 요구사항에 따른 대비 조정이 용이합니다
♻️ 개선 제안
colors.xml에 색상 정의:<color name="success_green">#1ED45A</color>drawable에서 참조:
- android:fillColor="#1ED45A"/> + android:fillColor="@color/success_green"/>feature/onboarding/src/main/java/com/twix/onboarding/dday/DdayRouete.kt (1)
12-36: Preview Composable 추가를 고려해주세요.코딩 가이드라인에 따르면 Compose UI에는 Preview Composable이 제공되어야 합니다. 현재는 placeholder 화면이지만, 향후 실제 구현 시 UI 검증을 위해 Preview를 추가하면 개발 편의성이 향상됩니다.
♻️ Preview 추가 예시
`@Preview`(showBackground = true) `@Composable` private fun DdayRoutePreview() { TwixTheme { DdayRoute( onComplete = {}, onBack = {}, ) } }feature/onboarding/src/main/java/com/twix/onboarding/profile/component/OnBoardingTopbar.kt (2)
28-37: 접근성 개선:contentDescription을 리소스 문자열로 변경해주세요.스크린 리더 사용자를 위해
contentDescription이 하드코딩된 영문 "back" 대신 다국어 지원이 가능한 리소스 문자열을 사용하는 것이 좋습니다. 디자인 시스템 리뷰 가이드라인에서 접근성(Accessibility) 고려를 권장하고 있습니다.♻️ 수정 제안
strings.xml에 추가:<string name="content_description_back">뒤로 가기</string>코드 수정:
Icon( painter = painterResource(id = DesR.drawable.ic_arrow1_m_left), - contentDescription = "back", + contentDescription = stringResource(id = R.string.content_description_back), modifier = Modifier .size(44.dp) .padding(start = 10.dp) .noRippleClickable { onBack() } .align(Alignment.CenterStart), )
17-39: Preview Composable 추가를 권장합니다.코딩 가이드라인에 따르면 Compose UI에는 Preview가 제공되어야 합니다. TopBar 컴포넌트는 여러 화면에서 재사용될 수 있으므로 Preview를 통해 독립적으로 UI를 검증하면 좋겠습니다.
♻️ Preview 추가 예시
`@Preview`(showBackground = true) `@Composable` private fun OnBoardingTopbarPreview() { TwixTheme { OnBoardingTopbar( onBack = {}, ) } }core/design-system/src/main/java/com/twix/designsystem/components/button/AppButton.kt (2)
19-49: 비활성화 상태(enabled = false)에 대한 시각적 피드백이 필요합니다.현재 구현에서는
enabled = false일 때도 동일한backgroundColor가 사용됩니다. 사용자가 버튼이 비활성화되었음을 인지할 수 있도록 시각적 차이를 제공하는 것이 좋습니다.♻️ 비활성화 상태 처리 예시
`@Composable` fun AppButton( modifier: Modifier = Modifier, text: String, textColor: Color = CommonColor.White, backgroundColor: Color = GrayColor.C500, + disabledBackgroundColor: Color = GrayColor.C200, + disabledTextColor: Color = GrayColor.C300, enabled: Boolean = true, cornerRadius: Dp = 12.dp, border: BorderStroke? = null, onClick: () -> Unit = {}, ) { + val actualBackgroundColor = if (enabled) backgroundColor else disabledBackgroundColor + val actualTextColor = if (enabled) textColor else disabledTextColor + Surface( - color = backgroundColor, + color = actualBackgroundColor, shape = RoundedCornerShape(cornerRadius), border = border, onClick = onClick, enabled = enabled, modifier = modifier, ) { AppText( text = text, - color = textColor, + color = actualTextColor, style = AppTextStyle.T2, textAlign = TextAlign.Center, modifier = Modifier .fillMaxWidth() .padding(vertical = 14.dp), ) } }
19-49: Preview Composable과 Dark Mode 지원을 고려해주세요.디자인 시스템 가이드라인에서 Dark Mode 지원과 Preview 제공을 권장하고 있습니다. AppButton은 여러 곳에서 재사용될 공통 컴포넌트이므로 다양한 상태의 Preview를 제공하면 디자인 검증에 도움이 됩니다.
♻️ Preview 추가 예시
`@Preview`(showBackground = true, name = "Enabled") `@Preview`(showBackground = true, name = "Disabled") `@Composable` private fun AppButtonPreview() { TwixTheme { Column { AppButton( text = "다음", enabled = true, onClick = {}, ) AppButton( text = "다음", enabled = false, onClick = {}, ) } } }feature/onboarding/src/main/res/values/strings.xml (1)
3-9: 중복 문자열 재사용과 다국어 지원 여부를 확인해주세요.동일 문구가
onboarding_name_*와onboarding_profile_*로 중복되어 있어 추후 문구 변경 시 불일치가 생길 수 있습니다. 공통 키로 통합하거나 한쪽이 다른 키를 참조하도록 정리하는 방법을 검토해 주세요. 다국어 지원 계획이 있다면 values-en 등 번역 리소스도 추가할지 확인해 볼까요?
As per coding guidelines, "다국어 지원을 고려했는가?".feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt (1)
3-40: 사이드이펙트를 새 코루틴에 분리하면 순서 보장이 약해질 수 있어요.
handleIntent가 이미 suspend인데viewModelScope.launch로 분리하면 이후 Intent가 먼저 처리되어 네비게이션/토스트 순서가 뒤바뀔 여지가 있습니다.handleSubmitNickname을 suspend로 변경하고emitSideEffect를 직접 호출해 순차 흐름을 유지하는 방향은 어떨까요?🔧 제안 수정안
- private fun handleSubmitNickname() { + private suspend fun handleSubmitNickname() { val sideEffect = if (uiState.value.isValidNickName) { OnBoardingSideEffect.ProfileSetting.NavigateToNext } else { OnBoardingSideEffect.ProfileSetting.ShowInvalidNickNameToast } - viewModelScope.launch { - emitSideEffect(sideEffect) - } + emitSideEffect(sideEffect) }As per coding guidelines, "단방향 데이터 플로우(Intent → ViewModel → State → View)가 유지되는가?".
feature/onboarding/src/main/java/com/twix/onboarding/couple/CoupleConnectRoute.kt (1)
12-27: 간단한 Preview를 추가하면 화면 확인이 더 쉬워집니다.현재는 앱 실행 없이 레이아웃을 확인하기 어려워요. 기본 상태용
@Preview를하나 두면 빠르게 확인할 수 있습니다. 필요하시면 추가해볼까요?🔧 제안 수정안
+import androidx.compose.ui.tooling.preview.Preview + +@Preview(showBackground = true) +@Composable +private fun CoupleConnectRoutePreview() { + CoupleConnectRoute(onNext = {}) +}As per coding guidelines, "Preview Composable이 제공되는가?".
domain/src/main/java/com/twix/domain/model/nickname/NickNameException.kt (1)
3-4: Throwable 대신 Exception 상속 의도가 있는지 확인 부탁드립니다.
Throwable은 Error까지 포함하는 최상위 타입이라 catch 범위가 넓어질 수 있습니다. 특별한 목적이 없다면Exception(또는RuntimeException) 상속으로 좁히는 편이 일반적입니다—에러까지 잡아야 하는 시나리오가 있을까요?🔧 제안 수정안
-sealed class NickNameException : Throwable() { +sealed class NickNameException : Exception() { data object InvalidLengthException : NickNameException() }feature/onboarding/src/main/java/com/twix/onboarding/navigation/OnboardingNavGraph.kt (1)
10-11:DdayRouete명칭은 의도된 건가요?
오탈자라면DdayRoute로 정리하면 검색성과 유지보수성이 좋아집니다. 리네이밍 의도가 있는지 확인해도 될까요?Also applies to: 38-40
feature/onboarding/src/main/java/com/twix/onboarding/couple/CoupleConnectRoute.kt
Show resolved
Hide resolved
feature/onboarding/src/main/java/com/twix/onboarding/profile/ProfileScreen.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@feature/onboarding/src/main/java/com/twix/onboarding/navigation/OnboardingNavGraph.kt`:
- Line 10: Rename the misspelled route symbols from DdayRouete to DdayRoute:
rename the file DdayRouete.kt to DdayRoute.kt, update the composable function
declaration fun DdayRouete(...) to fun DdayRoute(...), change the import
com.twix.onboarding.dday.DdayRouete to com.twix.onboarding.dday.DdayRoute, and
update any call sites that reference DdayRouete (the composable
registration/usage that should match NavRoutes.DdayRoute) so names are
consistent.
feature/onboarding/src/main/java/com/twix/onboarding/navigation/OnboardingNavGraph.kt
Outdated
Show resolved
Hide resolved
|
|
||
| // TODO : 디자인 시스템 UnderlineTextField로 수정 | ||
|
|
2 participants
이슈 번호
#35
리뷰/머지 희망 기한 (선택)
작업내용
결과물
default.mp4
리뷰어에게 추가로 요구하는 사항 (선택)