온보딩 화면 구현 #67
온보딩 화면 구현 #67
Conversation
# Conflicts: # app/src/main/java/com/yapp/twix/main/MainActivity.kt
…sk-certification-comment
초대코드 입력 화면(`InviteCodeScreen`)에 뒤로가기 기능을 구현하고, 키보드가 닫혔을 때의 상단 UI를 조정했습니다.
- 초대 코드 조회 실패 시 에러 토스트를 표시합니다. - 잘못된 초대 코드 입력 시 에러 토스트를 표시합니다. - 기념일 설정 실패 시 에러 토스트를 표시합니다. - API 통신 로직을 `launchResult`로 리팩터링하여 예외 처리를 구현합니다.
📝 Walkthrough코드 리뷰 분석Walkthrough이번 변경은 새로운 Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 주요 관찰사항 및 제안✅ 긍정적 측면1. 일관된 에러 처리 전략 2. 도메인 모델 재설계 3. 모듈 구조의 명확한 분리
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Merge Conflict Detection |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | PR 제목은 '온보딩 화면 구현'으로 변경 사항의 핵심을 명확하게 반영하고 있습니다. 커플 연결, 프로필 설정, 디데이 설정 등 온보딩 기능 구현이라는 주요 변경 내용을 잘 요약했습니다. |
| Description check | ✅ Passed | PR 설명은 이슈 번호(#35), 작업 내용(커플 연결, 프로필 설정, 디데이 설정), 그리고 PR을 다시 작성한 배경을 포함하고 있어 변경 사항과 충분히 관련이 있습니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feat/#35-profile-setting
⚔️ Resolve merge conflicts (beta)
- Auto-commit resolved conflicts to branch
feat/#35-profile-setting - Create stacked PR with resolved conflicts
- Post resolved changes as copyable diffs in a comment
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/login/src/main/java/com/twix/login/LoginViewModel.kt (1)
24-39:⚠️ Potential issue | 🟠 Major
authRepository.login()호출에 대한 에러 처리가 필요합니다.
authRepository.login(result.idToken, result.type)은 네트워크 요청(googleLogin)과 토큰 저장(saveToken)을 수행하는 suspend 함수로서 예외를 던질 수 있습니다. 현재 코드에서 이 호출이 예외를 발생시키면checkOnboardingStatus()가 실행되지 않고 사용자에게 어떤 피드백도 전달되지 않아 로그인 실패를 인지하기 어려워집니다.같은 파일의
checkOnboardingStatus()에서 이미launchResult패턴을 활용해onBoardingRepository.fetchOnboardingStatus()의 성공/실패를 처리하고 있으므로, 일관성 있게authRepository.login()호출도 동일한 패턴을 적용하는 것을 권장합니다. 이를 통해 로그인 실패 시에도LoginSideEffect.ShowLoginFailToast같은 적절한 사용자 피드백을 제공할 수 있습니다.
🤖 Fix all issues with AI agents
In `@data/src/main/java/com/twix/data/repository/DefaultOnboardingRepository.kt`:
- Around line 16-20: The fetchInviteCode implementation in
DefaultOnboardingRepository bypasses InviteCode validation by calling the public
constructor (InviteCode(response.inviteCode)); change it to use the domain
factory method (InviteCode.create(response.inviteCode)) so the 8-character rule
is enforced, or if you intend to trust the server make the constructor private
and document that decision; update the fetchInviteCode call site to invoke
InviteCode.create(...) and handle its result/validation accordingly (and/or
adjust InviteCode visibility) to ensure consistent object creation across
layers.
In `@feature/login/src/main/java/com/twix/login/navigation/LoginNavGraph.kt`:
- Around line 32-46: The navigateToOnBoarding lambda in LoginNavGraph currently
navigates directly to nested routes (NavRoutes.CoupleConnectionRoute,
ProfileRoute, DdayRoute) which breaks the onboarding graph structure; change it
to navigate to NavRoutes.OnboardingGraph.route instead and pass the onboarding
status (or no args) so OnboardingNavGraph can decide the correct
startDestination or handle a deep-link; update
OnboardingNavGraph/OnBoardingViewModel to accept the status (or read it on
entry) and route internally (e.g., compute startDestination or perform
navigation to CoupleConnectionRoute/InviteRoute/ProfileRoute/DdayRoute in the
intended order) so ViewModel initialization and graph hierarchy are preserved.
In `@feature/login/src/main/res/values/strings.xml`:
- Line 5: The string resource fetch_onboarding_status_fail_message incorrectly
says "로그인에 실패했습니다" which is misleading and duplicates login_fail_message; either
update fetch_onboarding_status_fail_message to a context-accurate message such
as "정보를 불러오는 데 실패했습니다. 다시 시도해주세요" or remove it and reference the existing
login_fail_message where appropriate to avoid duplication and support
localization consistency; locate the resource by the name
fetch_onboarding_status_fail_message (and login_fail_message) in strings.xml and
apply the chosen change.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/couple/CoupleConnectRoute.kt`:
- Around line 136-143: The dismiss animation is not running because the
CommonBottomSheet is conditionally removed by the surrounding if
(showRestoreSheet); remove that if wrapper and keep CommonBottomSheet in
composition always, controlling its visibility via the visible parameter (change
visible = true to visible = showRestoreSheet) so internal exit animation
(internalVisible → delay → rendering false) can run; keep onDismissRequest =
onDismissSheet and the same content RestoreCoupleBottomSheetContent().
In
`@feature/onboarding/src/main/java/com/twix/onboarding/dday/component/DdayTopBar.kt`:
- Around line 31-38: 현재 Image composable for the back button sets
contentDescription = null which makes it invisible to screen readers; replace
the null with a meaningful, localized description (e.g., use
stringResource(R.string.back_button) or a suitable "뒤로가기" entry) and add the
corresponding string resource in strings.xml, ensuring you import
androidx.compose.ui.res.stringResource; keep the existing Image, modifier, and
navigateToBack/noRippleClickable usage as-is so the accessibility label is the
only change.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/invite/InviteCodeScreen.kt`:
- Line 109: 클래스명 InViteCodeUiModel이 대문자 V로 camelCase 규칙을 위반하므로 클래스 이름을
InviteCodeUiModel로 리네임하고 관련된 모든 참조를 함께 변경하세요; 구체적으로 클래스
선언(InViteCodeUiModel.kt)과 해당 타입을 참조하는 OnBoardingUiState의 프로퍼티, InviteCodeScreen의
파라미터/인스턴스화 및 관련 import 문을 모두 InviteCodeUiModel로 업데이트하고 컴파일/직렬화 어노테이션(예:
`@Serializable`), 팩토리 메서드, equals/hashCode/parcelable 관련 이름 의존성도 함께 수정하여 빌드 오류가
없도록 하십시오.
- Around line 198-213: The copy button currently writes directly to the
clipboard in InviteCodeScreen instead of dispatching the
OnBoardingIntent.CopyInviteCode intent, so the ViewModel cannot emit the
ShowCopyInviteCodeSuccessToast side effect; add an onCopyInviteCode callback
parameter to InviteCodeScreen and replace the direct clipboard write in the
Image/noRippleClickable handler with a call to onCopyInviteCode(), then update
InviteCodeRoute to pass a lambda that calls
viewModel.dispatch(OnBoardingIntent.CopyInviteCode) so the action flows through
Intent → ViewModel → SideEffect and the toast is shown.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/invite/InViteCodeUiModel.kt`:
- Line 7: 클래스명 InViteCodeUiModel의 잘못된 대문자 사용을 수정해 InViteCodeUiModel을
InviteCodeUiModel로 리네임하고 파일명도 InviteCodeUiModel.kt로 변경하세요; 관련 모든 참조(임포트, 생성자 호출,
타입 선언 등)에서 InViteCodeUiModel을 InviteCodeUiModel로 바꾸고 빌드/테스트가 통과하는지 확인하세요.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/model/OnBoardingSideEffect.kt`:
- Around line 26-30: ShowAnniversarySetupFailToast is incorrectly declared as
implementing ProfileSetting inside the DdaySetting sealed interface; change its
supertype to DdaySetting so it participates in DdaySetting exhaustiveness
checks. Locate the sealed interface DdaySetting and the data object
ShowAnniversarySetupFailToast and replace its declaration to implement
DdaySetting (not ProfileSetting), ensuring it remains a member of
OnBoardingSideEffect via DdaySetting.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/navigation/OnboardingNavGraph.kt`:
- Around line 76-78: The navigateToBack lambda in OnboardingNavGraph.kt
currently calls navController.navigate(NavRoutes.ProfileRoute.route), which
pushes a new ProfileRoute instead of going back; change navigateToBack to call
navController.popBackStack() (or pass navController::popBackStack) matching the
InviteRoute pattern so the back stack is popped instead of adding a duplicate
ProfileRoute.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/profile/ProfileScreen.kt`:
- Around line 158-168: The AppButton currently only changes visual styling based
on uiModel.isValid but remains interactable; update the AppButton invocation
(the one using text = stringResource(...), onClick = { onCompleted() },
backgroundColor = ..., textColor = ...) to also set enabled = uiModel.isValid so
the button is disabled when validation fails, ensuring the visual disabled state
matches behavior and prevents clicks when uiModel.isValid is false.
- Around line 59-76: Replace the manual LaunchedEffect + collect usage on
viewModel.sideEffect with the lifecycle-aware ObserveAsEvents pattern: import
com.twix.ui.base.ObserveAsEvents, call viewModel.sideEffect.ObserveAsEvents()
(or use the helper extension used in other routes) and iterate the emitted
events the same way you currently do, routing
OnBoardingSideEffect.ProfileSetting cases to
toastManager.tryShow(ToastData(...)), navigateToHome(), and navigateToDday();
ensure you remove the direct LaunchedEffect and collect so navigation calls
occur only when the UI is STARTED and match the other routes' lifecycle-safe
behavior.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt`:
- Around line 100-122: The onError lambda in fetchOnboardingStatus is left
empty, so failures from onBoardingRepository.fetchOnboardingStatus() leave the
UI stuck; add error handling that runs on the viewModelScope (similar to
onSuccess) and emits a side effect or state to inform the UI (e.g.,
emitSideEffect(OnBoardingSideEffect.ShowError(message)) or
emitSideEffect(OnBoardingSideEffect.ShowRetry)) and/or trigger a retry flow;
update the onError block inside launchResult in fetchOnboardingStatus to launch
a coroutine, map the throwable to a user-friendly message, and call
emitSideEffect with a new error/retry side effect (use existing emitSideEffect
and OnBoardingSideEffect types to locate where to add the new error variant).
- Around line 62-77: In handleCoupleConnectException, avoid branching only on
server error message strings (INVALID_INVITE_CODE_MESSAGE,
ALREADY_USED_INVITE_CODE_MESSAGE); instead check for a server-side error code on
AppError.Http (if present) or introduce explicit enum/constant codes to map to
OnBoardingSideEffect.InviteCode.ShowInvalidInviteCodeToast and NavigateToNext,
and add a fallback else branch that emits a generic error side effect (e.g., a
general toast/navigation) when a 404 is returned but the message/code doesn't
match so the user always gets feedback; update any message-based checks to
prefer error.code and keep message comparison only as a last-resort fallback.
- Around line 93-98: The profileSetup function currently calls
onBoardingRepository.profileSetup(...) and immediately calls
fetchOnboardingStatus() without checking success or errors; update profileSetup
to use the existing launchResult pattern (like other API calls) to await and
inspect the result of onBoardingRepository.profileSetup, call
fetchOnboardingStatus() only on success, and handle failures by logging or
updating UI state (e.g., set an error field or emit a toast) instead of
proceeding silently; locate the logic in profileSetup, viewModelScope.launch,
and the repository call onBoardingRepository.profileSetup to implement the
change.
🧹 Nitpick comments (30)
feature/onboarding/src/main/res/drawable/ic_direct.xml (1)
11-11: 색상 값을 디자인 시스템의 컬러 리소스로 참조하는 것을 권장합니다.현재
#171717(다크 그레이)와#F4F4F4(라이트 그레이) 색상이 하드코딩되어 있습니다.왜 문제가 될 수 있나요?
- 디자인 시스템이 변경될 때 여러 파일을 일일이 수정해야 합니다
- 다크모드 등 테마 대응이 어렵습니다
- 같은 색상이 다른 drawable에서도 사용될 경우 일관성 유지가 어렵습니다
개선 방법:
colors.xml또는 테마 속성에 색상을 정의하고 참조하는 방식으로 변경할 수 있습니다.♻️ 제안하는 개선 방법
1단계:
colors.xml또는themes.xml에 색상 정의<!-- colors.xml --> <color name="icon_stroke_dark">#171717</color> <color name="icon_background_light">#F4F4F4</color>2단계: drawable에서 색상 참조
<path android:pathData="M23,45V44C11.402,44 2,34.598 2,23H1H0C0,35.702 10.297,46 23,46V45ZM45,23H44C44,34.598 34.598,44 23,44V45V46C35.702,46 46,35.702 46,23H45ZM23,1V2C34.598,2 44,11.402 44,23H45H46C46,10.297 35.702,0 23,0V1ZM23,1V0C10.297,0 0,10.297 0,23H1H2C2,11.402 11.402,2 23,2V1Z" - android:fillColor="#171717"/> + android:fillColor="@color/icon_stroke_dark"/> </group> <path android:pathData="M45,23C45,35.15 35.15,45 23,45C10.85,45 1,35.15 1,23C1,10.85 10.85,1 23,1C35.15,1 45,10.85 45,23Z" - android:fillColor="#F4F4F4"/> + android:fillColor="@color/icon_background_light"/> <path android:pathData="M24.241,14.959L31.146,23.245M31.146,23.245L24.241,31.53M31.146,23.245L14.574,23.245" android:strokeLineJoin="round" android:strokeWidth="1.2" android:fillColor="#00000000" - android:strokeColor="#171717" + android:strokeColor="@color/icon_stroke_dark" android:strokeLineCap="round"/>As per coding guidelines: "색상과 테마가 디자인 시스템에 포함되는가?"를 확인해주세요.
Also applies to: 15-15, 21-21
feature/onboarding/src/main/res/drawable/ic_copy.xml (1)
6-14: 하드코딩된 색상 값이 디자인 시스템 색상 리소스를 참조하지 않고 있습니다.
#F4F4F4(배경 fill)와#858585(stroke)가 직접 하드코딩되어 있습니다. 이 경우 다크 모드 지원 시 아이콘이 테마에 맞게 변경되지 않으며, 디자인 시스템의 색상이 변경될 때 이 파일도 별도로 수정해야 합니다.개선 방법:
@color/리소스를 정의하고 참조하거나, 사용처에서app:tint를 적용하는 방식을 고려해 보세요.- 예를 들어,
fillColor와strokeColor를 디자인 시스템의 색상 리소스(@color/icon_background,@color/icon_stroke등)로 대체하면 테마 변경 시 일괄 반영이 가능합니다.다만, Android vector drawable에서
?attr/형태의 테마 속성 참조는 API 레벨에 따라 제한이 있을 수 있으므로, 프로젝트의 minSdk와 호환성을 확인해 주세요.AppCompatResources를 통해 inflate하면 대부분의 경우 테마 속성 참조가 가능합니다.As per coding guidelines, "색상과 테마가 디자인 시스템에 포함되는가?"
core/network/src/main/java/com/twix/network/service/OnboardingService.kt (1)
13-16:anniversarySetup의 반환 타입이Unit인 점 참고현재
anniversarySetup,coupleConnection,profileSetup세 개의 POST 메서드가 모두 반환 타입 없이(Unit) 선언되어 있습니다. 서버가 성공 시 응답 본문 없이 2xx만 반환한다면 문제없지만, 만약 서버가 생성된 리소스 ID나 상태 정보를 응답으로 내려준다면 해당 데이터를 활용할 수 없게 됩니다.이 부분은 이번 PR의 변경 범위는 아니지만, 향후 API 응답 스펙과 맞는지 확인해보시면 좋겠습니다.
core/design-system/src/main/res/drawable/ic_check_success.xml (1)
1-9: 하드코딩된 색상 값 — Dark Mode 대응 검토가 필요합니다.
fillColor="#1ED45A"가 직접 하드코딩되어 있어, 다크 모드에서도 동일한 색상이 사용됩니다. 성공 아이콘의 녹색은 테마에 관계없이 동일할 수 있지만, 디자인 시스템의 일관성을 위해 색상 리소스(@color/)를 참조하거나, 사용처에서tint를 적용하는 방식을 권장합니다.이렇게 하면:
- 다크 모드에서 배경과의 대비(contrast)를 조정할 수 있고,
- 디자인 토큰이 변경될 때 한 곳에서만 수정하면 됩니다.
As per coding guidelines,
core/design-system/**: "Dark Mode 지원 여부",**/res/**: "색상과 테마가 디자인 시스템에 포함되는가?"core/design-system/src/main/res/drawable/ic_calendar.xml (1)
1-16: 디자인 시스템 아이콘은 다크 모드를 고려한 색상 정의가 필요합니다.현재
ic_calendar.xml의strokeColor와fillColor가#171717(거의 검정)로 하드코딩되어 있으며, 사용처인DDayField.kt에서 별도의 tint나 colorFilter를 적용하지 않고 있습니다.코딩 가이드라인에서
core/design-system/**파일에 대해 "Dark Mode 지원 여부"를 확인하도록 요구하고 있습니다. 현재는 앱에 다크 모드 지원이 없지만, 향후 추가될 경우 이 아이콘들은 다크 배경에서 보이지 않는 문제가 발생할 수 있습니다.개선 방향:
사용처에서 tint 적용 (권장): 아이콘 사용 시점에
tint파라미터를 명시적으로 지정하여 테마 색상을 적용합니다.Image( imageVector = ImageVector.vectorResource(DesR.drawable.ic_calendar), tint = LocalContentColor.current, // 또는 특정 색상 // ... )drawable에서 색상 제거:
strokeColor와fillColor를 제거하거나android:tint속성을 활용하여 사용처의 tint에 의존하도록 변경합니다. 이 방식이 더 유연합니다.같은 패턴이
ic_arrow1_m_left.xml등 다른 아이콘들에도 적용되어 있으므로, 한 번에 통일된 방식으로 개선하시면 좋겠습니다.core/design-system/src/main/res/drawable/ic_clear_text.xml (1)
1-15: 다크 모드에서의 색상 대응을 고려해 보시면 좋겠습니다현재
#C6C6C6(회색 원)과#ffffff(흰색 X)가 하드코딩되어 있습니다. 만약 Compose에서tint를 통해 색상을 오버라이드하고 있다면 문제 없지만, 이 드로어블이 그대로 사용될 경우 다크 모드에서 배경과의 대비가 부족할 수 있습니다.왜 문제가 될 수 있는가? → 다크 테마 배경에서
#C6C6C6원이 충분히 구분되지 않을 수 있습니다.개선 방향: Compose 사용처에서
tint로 테마 색상을 적용하고 있는지 확인하거나, 필요 시 night 리소스 디렉토리에 별도 버전을 두는 방법을 고려해 주세요. As per coding guidelines, 디자인 시스템 리소스는 Dark Mode 지원 여부를 고려해야 합니다.core/design-system/src/main/java/com/twix/designsystem/keyboard/KeyBoardAsState.kt (1)
10-10: TODO 코멘트 확인:core:ui모듈로의 이동 계획이 있군요.이 유틸리티는 디자인 시스템보다는 UI 유틸리티 성격이 강하므로, 향후
core:ui로 이동하는 것이 적절해 보입니다. 이 작업을 추적할 이슈를 생성해 두시면 좋겠습니다.이 이동 작업을 위한 이슈를 생성해 드릴까요?
feature/onboarding/src/main/java/com/twix/onboarding/dday/component/DdayTopBar.kt (1)
18-40: Preview Composable이 누락되어 있습니다.코딩 가이드라인에 따르면 Composable 컴포넌트에는 Preview가 제공되어야 합니다.
DdayTopBar는 독립적인 UI 컴포넌트이므로 Preview를 추가하면 디자인 검증에 도움이 됩니다.♻️ Preview 추가 제안
`@Preview`(showBackground = true) `@Composable` private fun DdayTopBarPreview() { TwixTheme { DdayTopBar(navigateToBack = {}) } }As per coding guidelines,
feature/**: "Preview Composable이 제공되는가?"feature/onboarding/src/main/java/com/twix/onboarding/couple/component/InvitationButton.kt (1)
28-43: 접근성 개선: 버튼 시맨틱(semantics) 추가를 권장합니다.
Box를 버튼으로 사용하고 있지만, 접근성 서비스(TalkBack 등)에서 이 요소를 버튼으로 인식하지 못합니다.semantics { role = Role.Button }을 추가하면 스크린 리더가 "버튼"으로 안내할 수 있습니다. 이 패턴은ConnectButton에도 동일하게 적용됩니다.♻️ 시맨틱 역할 추가 제안
+import androidx.compose.ui.semantics.Role +import androidx.compose.ui.semantics.role +import androidx.compose.ui.semantics.semantics ... Box( modifier = modifier .fillMaxWidth() .height(86.dp) .padding(horizontal = 36.dp) .background(color = GrayColor.C500, shape = RoundedCornerShape(12.dp)) - .noRippleClickable(onClick = onClick), + .semantics { role = Role.Button } + .noRippleClickable(onClick = onClick), contentAlignment = Alignment.Center, ) {As per coding guidelines,
core/design-system/**: "접근성(Accessibility) 고려 여부"feature/onboarding/src/main/java/com/twix/onboarding/couple/component/ConnectButton.kt (2)
69-76: 디자인 시스템 일관성:Text대신AppText를 사용하는 것이 좋습니다.같은 파일 내에서
AppText(Line 58, 78)와 rawText(Line 69)를 혼용하고 있습니다.AppText는fontScale = 1f를 강제하여 일관된 텍스트 크기를 보장하는데, rawText는 이 보호 장치를 우회합니다. 사용자가 시스템 글꼴 크기를 변경했을 때 이 텍스트만 크기가 달라질 수 있습니다.
AppTextStyle에 해당하는 스타일이 없다면, 새로운 스타일을 추가하거나AppText내부에서 커스텀 스타일을 지원하는 방법을 검토해 보시겠어요?
88-91:ic_direct아이콘의contentDescription이null입니다.이 아이콘은 텍스트와 함께 표시되는 장식적(decorative) 요소라면
null이 허용됩니다. 다만, 전체Row가 하나의 클릭 가능한 버튼 역할을 하므로,InvitationButton과 마찬가지로semantics { role = Role.Button }을 추가하여 접근성을 개선하는 것을 권장합니다.As per coding guidelines,
core/design-system/**: "접근성(Accessibility) 고려 여부"domain/src/main/java/com/twix/domain/model/invitecode/InviteCodeException.kt (1)
3-5: 도메인 레이어에 적합한 깔끔한 예외 계층 구조입니다. 👍
sealed class를 활용한 닫힌 예외 계층과data object를 통한 싱글톤 패턴이 잘 적용되어 있습니다. Android/외부 프레임워크 의존성 없이 순수 Kotlin으로 구성된 점도 도메인 레이어 원칙에 부합합니다.한 가지 제안: 디버깅 편의를 위해 기본 메시지를 추가하면 로그 추적 시 유용할 수 있습니다.
💡 디버깅용 메시지 추가 제안
sealed class InviteCodeException : Throwable() { - data object InvalidLengthException : InviteCodeException() + data object InvalidLengthException : InviteCodeException() { + private fun readResolve(): Any = InvalidLengthException + override val message: String get() = "Invite code length is invalid" + } }As per coding guidelines,
domain/**: "비즈니스 로직이 명확히 표현되어 있는가?"feature/onboarding/src/main/java/com/twix/onboarding/dday/component/DDayField.kt (1)
61-68: 접근성: 클릭 가능한 캘린더 아이콘에contentDescription추가를 고려해 주세요.현재
contentDescription = null로 설정되어 있지만, 이 아이콘은noRippleClickable로 날짜 선택 동작을 트리거하는 인터랙티브 요소입니다. 스크린 리더 사용자가 이 요소의 역할을 인식할 수 있도록 의미 있는contentDescription을 제공하면 접근성이 향상됩니다.domain/src/main/java/com/twix/domain/repository/OnBoardingRepository.kt (1)
8-16:AppResult래핑으로 에러 처리를 표준화한 점이 좋습니다 👍모든 메서드가 일관되게
AppResult<T>를 반환하도록 변경되어, 호출부에서 에러 처리 패턴이 통일됩니다.한 가지 개선 제안:
anniversarySetup(request: String),coupleConnection(request: String),profileSetup(request: String)모두 매개변수가request: String으로 동일한데, 각각의 의미(날짜 문자열, 초대 코드, 닉네임)가 다릅니다. 타입 수준에서 구분이 되지 않으므로 실수로 잘못된 값을 전달해도 컴파일 타임에 잡히지 않습니다. 도메인 모델(InviteCode처럼)을 활용하거나, 최소한 파라미터 이름을date,inviteCode,nickname처럼 명확하게 하면 가독성과 안전성이 향상될 수 있습니다.feature/onboarding/src/main/java/com/twix/onboarding/couple/component/RestoreCoupleBottomSheetContent.kt (2)
29-30: 하드코딩된 색상이 디자인 시스템 외부에 정의되어 있습니다.
RestoreBgColor(0xFFF6F7F7)와BulletColor(0xFF999999)가 파일 내에 직접 정의되어 있습니다. 리소스 가이드라인에 따르면 색상은 디자인 시스템에 포함되어야 합니다. 이 색상들이 디자인 시스템의GrayColor등에 이미 존재하거나, 추가할 수 있는지 확인해 주세요. 디자인 시스템에 통합하면 테마 변경 시 일괄 반영이 가능하고 일관성을 유지할 수 있습니다. As per coding guidelines, "색상과 테마가 디자인 시스템에 포함되는가?"
91-97: Preview 함수의 가시성을private으로 변경하는 것을 권장합니다.
RestoreCoupleBottomSheetContentPreview()가public으로 선언되어 있지만, 이 PR의 다른 Preview 함수들(DDayFieldPreview,InviteCodeScreenPreview)은private입니다. 또한RestoreCoupleBottomSheetContent자체가internal이므로, Preview도 외부에 노출될 필요가 없습니다.♻️ 수정 제안
`@Preview`(showBackground = true) `@Composable` -fun RestoreCoupleBottomSheetContentPreview() { +private fun RestoreCoupleBottomSheetContentPreview() {feature/onboarding/src/main/java/com/twix/onboarding/invite/component/InviteCodeTextField.kt (2)
62-68: 모든CodeBox에서rememberInfiniteTransition이 생성되고 있어 불필요한 리소스 소비가 발생합니다.현재 8개의
CodeBox모두에서 무한 애니메이션이 생성되지만, 실제로 사용하는 것은isFocused == true인 단 하나의 박스뿐입니다. 포커스된 박스에서만 애니메이션을 생성하면 불필요한 리소스 소비를 줄일 수 있습니다.♻️ 조건부 애니메이션 적용 예시
`@Composable` private fun CodeBox( index: Int, code: String, ) { val isFocused = code.length == index - val infiniteTransition = rememberInfiniteTransition() - val alpha by infiniteTransition.animateFloat( - 0f, - 1f, - infiniteRepeatable(tween(500), RepeatMode.Reverse), - ) + val alpha = if (isFocused) { + val infiniteTransition = rememberInfiniteTransition(label = "cursorBlink") + val animatedAlpha by infiniteTransition.animateFloat( + initialValue = 0f, + targetValue = 1f, + animationSpec = infiniteRepeatable(tween(500), RepeatMode.Reverse), + label = "cursorAlpha", + ) + animatedAlpha + } else { + 0f + }
31-35:InviteCodeTextField의 가시성을internal로 변경하는 것을 고려해 주세요.이 PR의 다른 컴포저블들(
DDayField,RestoreCoupleBottomSheetContent등)은internal로 선언되어 모듈 외부 노출을 제한하고 있습니다.InviteCodeTextField도 온보딩 모듈 내부에서만 사용되므로, 일관성을 위해internal로 변경하면 좋겠습니다.feature/onboarding/src/main/res/values/strings.xml (1)
4-8: 중복된 문자열 리소스가 있습니다.
onboarding_name_placeholder(Line 4)과onboarding_profile_placeholder(Line 7)의 값이 동일하고,onboarding_name_helper(Line 5)와onboarding_profile_helper(Line 8)의 값도 동일합니다. 의도적으로 분리한 것인지(향후 다른 값으로 변경 예정), 아니면 하나로 통합할 수 있는지 확인해 주세요. 동일한 문자열이 여러 키로 존재하면 나중에 수정 시 누락될 수 있습니다.domain/src/main/java/com/twix/domain/model/invitecode/InviteCode.kt (1)
3-17: 깔끔한 팩토리 패턴 설계입니다 👍
Result기반 팩토리 메서드로 유효성 검사를 캡슐화한 점이 좋습니다. 도메인 모델이 불변이고 외부 프레임워크 의존성이 없어 가이드라인에 잘 부합합니다.한 가지 짚고 넘어갈 점: value class는 언어 설계상 public 주 생성자를 필수로 가져야 하므로,
private constructor로 변경하는 것은 불가능합니다. 따라서InviteCode("ab")처럼create()를 우회할 가능성은 value class 패러다임 내에서는 피할 수 없는 제약입니다.대신
init블록을 추가하여 직접 생성 시에도 검증이 실행되도록 개선할 수 있습니다:♻️ init 블록으로 검증 강화
`@JvmInline` value class InviteCode( val value: String, ) { + init { + require(value.length == INVITE_CODE_LENGTH) { + "InviteCode length must be $INVITE_CODE_LENGTH" + } + } + companion object { fun create(value: String): Result<InviteCode> = runCatching { - if (value.length != INVITE_CODE_LENGTH) { - throw InviteCodeException.InvalidLengthException - } InviteCode(value) }이 방식이면 어떻게 생성하든 검증이 보장됩니다. 의견은 어떠신가요?
data/src/main/java/com/twix/data/repository/DefaultOnboardingRepository.kt (1)
28-36: 메서드 포맷팅이 일관되지 않습니다.
anniversarySetup은 멀티라인으로 작성되어 있고,profileSetup은 싱글라인으로 작성되어 있습니다. 가독성과 일관성을 위해 동일한 포맷을 사용하면 좋겠습니다.♻️ 일관된 포맷 제안
- override suspend fun profileSetup(request: String): AppResult<Unit> = safeApiCall { service.profileSetup(ProfileRequest(request)) } + override suspend fun profileSetup(request: String): AppResult<Unit> = + safeApiCall { service.profileSetup(ProfileRequest(request)) }domain/src/test/java/com/twix/domain/model/InviteCodeTest.kt (2)
10-15: 유효한 코드 테스트에서도InviteCode.create()를 사용하는 것이 일관성 있습니다.Line 12에서
InviteCode("AB12CD34")로 생성자를 직접 호출하고 있지만, 유효하지 않은 코드 테스트(Line 26)에서는InviteCode.create()를 사용합니다. 만약create()가 유효성 검증을 포함하는 공식 생성 API라면, happy path 테스트도 동일한 API를 통해result.isSuccess를 검증하는 것이 더 의미 있는 테스트가 됩니다.♻️ create() API를 사용한 일관된 테스트 제안
`@Test` fun `유효한 초대 코드는 정상적으로 생성된다`() { - val inviteCode = InviteCode("AB12CD34") + // given + val code = "AB12CD34" + + // when + val result = InviteCode.create(code) - assertThat(inviteCode.value).isEqualTo("AB12CD34") + // then + assertThat(result.isSuccess).isTrue() + assertThat(result.getOrNull()?.value).isEqualTo(code) }
24-30: Given-When-Then 구조가 거의 갖춰져 있지만, Given 섹션이 빠져 있습니다.
// when과// then주석은 있지만// given주석이 누락되어 있습니다.@ValueSource로 주입되는 파라미터가 given 역할을 하므로, 명시적으로 표기하면 테스트 가이드라인과 더 잘 부합합니다. 사소한 부분이지만 일관성 차원에서 참고해 주세요.As per coding guidelines, "Given-When-Then 구조를 따르는가?"
app/src/main/java/com/yapp/twix/main/MainActivity.kt (1)
28-28: 주석 처리된 코드 제거를 권장합니다.
safeContentPadding()이 주석 처리되어 있는데, 더 이상 사용하지 않는다면 완전히 제거하는 것이 깔끔합니다. 주석 처리된 코드는 나중에 혼란을 유발할 수 있으며, 필요 시 Git 히스토리에서 복원할 수 있습니다.다만,
safeContentPadding()을 제거하면 루트Box에서 시스템 바(상태바, 네비게이션 바)에 대한 inset 처리가 사라지게 됩니다. 개별 화면에서 inset을 처리하고 있다면 괜찮지만, 그렇지 않다면 콘텐츠가 시스템 바와 겹칠 수 있으므로 확인이 필요합니다.,
feature/login/src/main/java/com/twix/login/LoginViewModel.kt (1)
41-63:onSuccess내부의 중첩viewModelScope.launch— 개선 가능한 부분입니다.
launchResult의onSuccess콜백이 non-suspend((D) -> Unit)이기 때문에emitSideEffect(suspend 함수)를 호출하기 위해viewModelScope.launch로 한 번 더 감싸고 있습니다. 이 방식은 동작은 하지만 몇 가지 우려가 있습니다:
- 실행 순서 보장 불가: 중첩된 코루틴은
launchResult의try-finally블록과 독립적으로 실행되므로,onFinally가 사이드 이펙트 전에 실행될 수 있습니다.- 에러 전파 단절: 중첩 코루틴 내부에서 예외가 발생하면
launchResult의 에러 핸들링으로 전파되지 않습니다.현재 코드에서는
onFinally를 사용하지 않고emitSideEffect가 예외를 던지지 않으므로 실질적 문제는 없지만,BaseViewModel의onSuccess시그니처를suspend로 변경하는 것을 팀 차원에서 검토해 보시는 것은 어떨까요?onError는 이미suspend로 되어 있어 비대칭이 존재합니다.현재 구조에서의 대안으로,
onSuccess를 사용하지 않고block결과를 직접 처리하는 방법도 있습니다:💡 중첩 launch 없이 처리하는 대안
private fun checkOnboardingStatus() { - launchResult( - block = { onBoardingRepository.fetchOnboardingStatus() }, - onSuccess = { onboardingStatus -> - viewModelScope.launch { - val sideEffect = - when (onboardingStatus) { - OnboardingStatus.COUPLE_CONNECTION, - OnboardingStatus.PROFILE_SETUP, - OnboardingStatus.ANNIVERSARY_SETUP, - -> LoginSideEffect.NavigateToOnBoarding(onboardingStatus) - - OnboardingStatus.COMPLETED -> LoginSideEffect.NavigateToHome - } - - emitSideEffect(sideEffect) - } - }, - onError = { - emitSideEffect(LoginSideEffect.ShowFetchOnBoardingStatusFailToast) - }, - ) + viewModelScope.launch { + when (val result = onBoardingRepository.fetchOnboardingStatus()) { + is AppResult.Success -> { + val sideEffect = + when (result.data) { + OnboardingStatus.COUPLE_CONNECTION, + OnboardingStatus.PROFILE_SETUP, + OnboardingStatus.ANNIVERSARY_SETUP, + -> LoginSideEffect.NavigateToOnBoarding(result.data) + + OnboardingStatus.COMPLETED -> LoginSideEffect.NavigateToHome + } + emitSideEffect(sideEffect) + } + is AppResult.Error -> { + emitSideEffect(LoginSideEffect.ShowFetchOnBoardingStatusFailToast) + } + } + } }이 대안은
launchResult의 공통 에러 로깅(handleError)을 잃게 되므로, 팀의 에러 처리 전략에 따라 선택하시면 됩니다.feature/onboarding/src/main/java/com/twix/onboarding/navigation/OnboardingNavGraph.kt (1)
59-63:navigateToHome로직 중복ProfileRoute와 DdayRoute에서 동일한
navigateToHome로직이 반복됩니다. 현재는 두 곳뿐이지만, 가독성과 유지보수를 위해 로컬 함수로 추출하는 것을 고려해 보시겠어요?♻️ 리팩터링 제안
registerGraph함수 상단에 로컬 함수를 추출:fun navigateToHome() { navController.navigate(NavRoutes.MainGraph.route) { popUpTo(graphRoute.route) { inclusive = true } } }그리고 각 destination에서
navigateToHome = ::navigateToHome으로 참조합니다.Also applies to: 71-75
domain/src/main/java/com/twix/domain/model/nickname/NickName.kt (1)
8-14: 예외를 제어 흐름으로 사용하는 것보다Result.success/Result.failure직접 반환을 권장합니다현재
runCatching내부에서throw를 사용하고 있는데, 이는 예외가 아닌 예상된 유효성 검사 실패입니다.runCatching은 본래 예기치 못한 예외를 안전하게 감싸기 위한 API이며, 의도적으로 throw하는 패턴은 가독성과 의도 전달 면에서 아쉽습니다.또한
runCatching은Throwable을 모두 catch하므로,OutOfMemoryError같은 치명적 에러까지 삼키게 됩니다.♻️ 리팩터링 제안
companion object { fun create(value: String): Result<NickName> = - runCatching { - if (value.length !in MIN_LENGTH..MAX_LENGTH) { - throw NickNameException.InvalidLengthException - } - NickName(value) - } + if (value.length !in MIN_LENGTH..MAX_LENGTH) { + Result.failure(NickNameException.InvalidLengthException) + } else { + Result.success(NickName(value)) + }feature/onboarding/src/main/java/com/twix/onboarding/couple/CoupleConnectRoute.kt (1)
83-90:CoupleConnectScreen의 visibility를private으로 제한하는 것을 고려해 보세요
CoupleConnectRoute가 외부에 노출되는 진입점이고,CoupleConnectScreen은 순수 UI 컴포넌트입니다.ProfileScreen처럼private으로 선언하면 모듈 외부에서의 의도치 않은 사용을 방지할 수 있습니다. 마찬가지로 Line 149의CoupleConnectScreenPreview도private이 적절합니다.feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt (1)
83-91:handleSubmitNickname에서도viewModelScope.launch사용이 불필요할 수 있습니다.Line 87의
viewModelScope.launch가 앞서 언급한 동일한 일관성 문제에 해당합니다.handleSubmitNickname은handleIntent에서 호출되는데,handleIntent자체가suspend fun이므로 이미 코루틴 컨텍스트 안에서 실행됩니다.emitSideEffect가 suspend 함수라면 별도의 launch 없이 직접 호출이 가능합니다.feature/onboarding/src/main/java/com/twix/onboarding/dday/DdayRoute.kt (1)
121-132:onClick람다를 간소화할 수 있습니다.
onClick = { onCompleted() }는onClick = onCompleted로 직접 함수 참조를 전달하면 불필요한 래핑 람다 생성을 피할 수 있습니다. 사소한 부분이지만 일관성을 위해 참고해주세요.♻️ 수정 제안
AppButton( text = stringResource(R.string.onboarding_profile_button_title), - onClick = { onCompleted() }, + onClick = onCompleted, backgroundColor = if (uiModel.isSelected) GrayColor.C500 else GrayColor.C100,
data/src/main/java/com/twix/data/repository/DefaultOnboardingRepository.kt
Show resolved
Hide resolved
feature/onboarding/src/main/java/com/twix/onboarding/couple/CoupleConnectRoute.kt
Outdated
Show resolved
Hide resolved
feature/onboarding/src/main/java/com/twix/onboarding/dday/component/DdayTopBar.kt
Show resolved
Hide resolved
feature/onboarding/src/main/java/com/twix/onboarding/profile/ProfileScreen.kt
Outdated
Show resolved
Hide resolved
feature/onboarding/src/main/java/com/twix/onboarding/profile/ProfileScreen.kt
Show resolved
Hide resolved
feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt
Show resolved
Hide resolved
feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt
Show resolved
Hide resolved
feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt
Show resolved
Hide resolved
dogmania
left a comment
dogmania
left a comment
There was a problem hiding this comment.
지금 테스트해보고 있는데 A 계정 초대 코드를 B 계정에 입력하다가 B 계정 코드를 A 계정에 입력하려고 하니까 B 계정에서는 더이상 초대코드 화면이 안 뜨더라구요. 이게 아마 온보딩 단계 중간에 나가면 나간 시점부터 이어서 할 수 있게 구현한 거 같은데 뒤로가기로 초대 코드를 다시 볼 수는 있어야 할 거 같아요 이거 먼저 수정해주세요!
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@feature/login/src/main/java/com/twix/login/LoginViewModel.kt`:
- Around line 33-38: In the LoginViewModel, the LoginResult.Failure branch
constructs a LoginSideEffect.ShowToast but never calls emitSideEffect, so the
toast never appears; modify the LoginResult.Failure branch in the function
handling login results to call emitSideEffect(LoginSideEffect.ShowToast(...))
instead of just creating the object, matching the pattern used in
checkOnboardingStatus()'s onError block (use the same emitSideEffect call with
message = R.string.login_fail_message and type = ToastType.ERROR).
In
`@feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt`:
- Around line 62-78: handleCoupleConnectException currently only handles
AppError.Http with status == 404; add fallback branches to surface feedback for
all other errors: for AppError.Http with non-404 statuses emit a generic failure
side effect (e.g.,
OnBoardingSideEffect.InviteCode.ShowConnectCoupleConnectFailToast or a new
status-aware failure side effect), and for non-Http/AppError network or
unexpected errors emit a network/error side effect (e.g.,
OnBoardingSideEffect.InviteCode.ShowNetworkErrorToast or a generic
ShowErrorToast). Update the function handleCoupleConnectException to check for
error type first (AppError.Http vs others) and ensure every path emits an
appropriate emitSideEffect so users get feedback for 5xx, timeouts, and unknown
errors.
In `@feature/onboarding/src/main/res/values/strings.xml`:
- Around line 4-8: Remove the two unused string resources
onboarding_profile_placeholder and onboarding_profile_helper from the
strings.xml file; locate the entries with those resource names and delete them,
and before committing run a project-wide search for
onboarding_profile_placeholder and onboarding_profile_helper to ensure nothing
references them (ProfileScreen.kt already uses onboarding_name_placeholder and
onboarding_name_helper), adding or renaming keys only if a future distinct
profile placeholder/helper string is required.
🧹 Nitpick comments (10)
feature/onboarding/src/main/res/values/strings.xml (3)
20-20: 고객센터 이메일이 문자열 내에 하드코딩되어 있습니다.왜 문제가 되는지: 이메일 주소(
[email protected])가 문자열 리소스 내부에 직접 포함되어 있어, 이메일 변경 시 이 리소스를 직접 찾아 수정해야 합니다. 또한, 이메일 주소가 다른 화면(예: 설정, 문의하기 등)에서도 사용된다면 여러 곳에서 중복 관리해야 하는 문제가 생깁니다.개선 방향: 이메일 부분을
%s포맷 인자로 분리하거나, 이메일 자체를 별도 문자열 리소스로 추출하면 유지보수가 수월해집니다.♻️ 포맷 인자 활용 예시
- <string name="onboarding_couple_restore_bottom_sheet_content">아래 내용을 포함하여 문의해 주시기 바랍니다.\n고객센터 메일 - [email protected]</string> + <string name="onboarding_couple_restore_bottom_sheet_content">아래 내용을 포함하여 문의해 주시기 바랍니다.\n고객센터 메일 - %s</string> + <string name="customer_service_email" translatable="false">[email protected]</string>Kotlin 사용 시:
stringResource(R.string.onboarding_couple_restore_bottom_sheet_content, stringResource(R.string.customer_service_email))
32-32: 키 네이밍 패턴이 일관되지 않습니다.이 섹션의 다른 키들은
onboarding_invite_code_*접두사를 사용하고 있는데, 이 키만onboarding_invite_invalid_invite_code_fail로_code없이_invite에서 바로 분기됩니다. 사소한 부분이지만, 리소스가 많아질수록 검색과 정리가 어려워질 수 있습니다.♻️ 제안
- <string name="onboarding_invite_invalid_invite_code_fail">잘못된 초대 코드입니다</string> + <string name="onboarding_invite_code_invalid_fail">잘못된 초대 코드입니다</string>
1-38: 다국어 지원 관련 참고 사항현재 한국어 문자열이 기본
values/strings.xml에 위치해 있습니다. 한국어만 지원하는 앱이라면 문제없지만, 향후 다국어 지원 계획이 있다면 기본values/strings.xml에는 영어(또는 기본 언어)를 두고, 한국어는values-ko/strings.xml로 분리하는 구조가 일반적입니다.현재 단계에서 당장 필요한 작업은 아니지만, 리소스 가이드라인에서 다국어 지원 고려를 권장하고 있어 참고 차원에서 남깁니다. 전체적으로 문자열 리소스가 잘 구조화되어 있고, 섹션별 주석 구분도 깔끔합니다 👍
As per coding guidelines,
**/res/**: "다국어 지원을 고려했는가?"feature/login/src/main/java/com/twix/login/LoginViewModel.kt (1)
48-61:onSuccess내부의viewModelScope.launch— 의도는 맞지만 개선 여지가 있습니다
BaseViewModel.launchResult의onSuccess가 non-suspend 콜백이므로emitSideEffect()(suspend 함수)를 호출하기 위해viewModelScope.launch로 감싸는 것은 동작상 정확합니다. 다만 이 패턴이 반복된다면,BaseViewModel에 suspend 버전의onSuccess콜백을 추가하거나 별도 헬퍼를 제공하는 것을 고려해 볼 수 있습니다.현재 PR 범위에서는 문제되지 않으므로 참고 사항으로 남깁니다.
feature/login/src/main/java/com/twix/login/LoginScreen.kt (1)
48-49:rememberUpdatedState(context)는 여기서 불필요할 수 있습니다
rememberUpdatedState는 주로LaunchedEffect등 장기 실행 코루틴 내에서 최신 콜백/값을 참조하기 위해 사용됩니다. 그런데LocalContext.current는 Composition 시점에 항상 최신 값을 반환하고,ObserveAsEvents는 내부적으로LaunchedEffect를 사용하므로 recomposition 시 재시작됩니다.다만 Configuration 변경(예: 언어 변경) 시
context가 갱신되는 경우를 방어하려는 의도라면 이해할 수 있습니다. 의도된 방어 코드인지 확인 부탁드립니다.feature/onboarding/src/main/java/com/twix/onboarding/profile/ProfileScreen.kt (2)
134-138: Clear 버튼의 접근성(contentDescription) 설정을 권장합니다
contentDescription = null로 설정되어 있어 스크린 리더 사용자가 이 버튼의 역할을 알 수 없습니다. 닉네임 입력 필드의 텍스트를 지우는 기능이므로, "입력 지우기" 등의 설명을 추가하면 접근성이 향상됩니다. As per coding guidelines, 접근성(Accessibility) 고려 여부를 검토해야 합니다.♿ 수정 제안
Image( imageVector = ImageVector.vectorResource(DesR.drawable.ic_clear_text), - contentDescription = null, + contentDescription = stringResource(R.string.onboarding_clear_nickname), modifier = Modifier.noRippleClickable { onChangeNickName("") }, )
strings.xml에 해당 문자열 리소스 추가가 필요합니다.
110-115: Dark Mode 지원을 위해서는 먼저 디자인 시스템 개선이 필요합니다
CommonColor.White가 하드코딩되어 있는 문제는 실제로 발생하고 있으며, 이는 ProfileScreen뿐 아니라 CoupleConnectRoute, DdayRoute, InviteCodeScreen에서도 동일한 패턴입니다.현재 앱의 TwixTheme는 타이포그래피만 제공하고 있어 MaterialTheme.colorScheme을 활용할 수 없는 상황입니다. Dark Mode를 지원하려면 다음과 같은 단계가 필요합니다:
- 디자인 시스템 확장: Colors.kt에
background색상을 추가하고, 라이트/다크 모드 변형을 정의- TwixTheme 업데이트: 색상을 CompositionLocal로 제공하도록 개선
- 온보딩 화면 리팩토링:
CommonColor.White→ 디자인 시스템의 배경색으로 변경확인된 하드코딩 사항
ProfileScreen.kt:114 CoupleConnectRoute.kt:95 DdayRoute.kt:98 InviteCodeScreen.kt:148현재는 디자인 시스템이 준비되지 않았다면 온보딩 화면이 라이트 전용이더라도 문제없으나, 장기적으로는 위의 개선안을 고려해 두시기 바랍니다.
feature/onboarding/src/main/java/com/twix/onboarding/couple/CoupleConnectRoute.kt (1)
83-90:CoupleConnectScreen의onClickSend파라미터가 현재 사용되지 않고 있습니다.Line 116에서
InvitationButton이 주석 처리되어 있어,onClickSend콜백이 어디에서도 호출되지 않습니다. PR 목표에 따르면 초대장 전송 기능이 임시 주석 상태인 것은 인지하고 있지만, 사용되지 않는 파라미터가 공개 API에 남아있으면 혼란을 줄 수 있습니다.두 가지 접근을 제안합니다:
- 해당 파라미터를 제거하고, 추후
InvitationButton을 활성화할 때 함께 추가하거나,- 유지하되
// TODO: InvitationButton 활성화 시 사용 예정등의 주석을 남겨 의도를 명확히 표현하는 것이 좋겠습니다.feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt (1)
84-92:handleSubmitNickname에서 불필요한viewModelScope.launch사용
handleSubmitNickname은handleIntent(suspend 함수)에서 호출되므로, 이미 코루틴 컨텍스트 내에서 실행됩니다. 따라서emitSideEffect를 호출하기 위해 별도의viewModelScope.launch가 필요하지 않습니다.참고로
onSuccess람다 내의viewModelScope.launch(Lines 54, 106, 134)는BaseViewModel.launchResult의onSuccess가 non-suspend 람다이므로 필요합니다. 하지만 이 경우는 다릅니다.♻️ 수정 제안
private fun handleSubmitNickname() { if (currentState.isValidNickName) { profileSetup() } else { - viewModelScope.launch { - emitSideEffect(OnBoardingSideEffect.ProfileSetting.ShowInvalidNickNameToast) - } + // handleIntent가 suspend이므로 직접 호출 가능하도록 suspend로 변경 필요 } }단, 이를 적용하려면
handleSubmitNickname을private suspend fun으로 변경해야 합니다:private suspend fun handleSubmitNickname() { if (currentState.isValidNickName) { profileSetup() } else { emitSideEffect(OnBoardingSideEffect.ProfileSetting.ShowInvalidNickNameToast) } }이렇게 하면
handleIntent의 코루틴 컨텍스트를 재활용하여 불필요한 코루틴 생성을 피할 수 있습니다.feature/onboarding/src/main/java/com/twix/onboarding/invite/InviteCodeScreen.kt (1)
144-262: Back 버튼과 Column이Box내에서 겹쳐 있는 레이아웃 구조를 확인해주세요.현재
Box안에 Back 버튼(Lines 150-166, 높이 72dp)과 메인Column(Lines 168-246)이 독립적으로 배치되어 있습니다.Column은Spacer(8.dp)+padding(top = 80.dp)으로 시작하여 실질적으로 88dp 이후에 컨텐츠가 표시되므로 현재는 겹침이 발생하지 않습니다.다만, 이러한 매직넘버 기반의 간격 조정은 향후 Back 버튼 높이가 변경되면 컨텐츠가 겹칠 수 있는 취약한 구조입니다. 나중에 리팩터링 시 Back 버튼을 Column 내부로 이동하거나,
Scaffold/TopAppBar패턴을 활용하면 더 안정적인 레이아웃이 될 수 있습니다. 지금 당장은 문제가 아니므로 참고용으로 남깁니다.
| is LoginResult.Failure -> { | ||
| emitSideEffect(LoginSideEffect.ShowLoginFailToast) | ||
| LoginSideEffect.ShowToast( | ||
| message = R.string.login_fail_message, | ||
| type = ToastType.ERROR, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🐛 로그인 실패 시 토스트가 표시되지 않습니다 — emitSideEffect 호출 누락
LoginResult.Failure 분기에서 LoginSideEffect.ShowToast 객체를 생성만 하고 emitSideEffect()를 호출하지 않고 있습니다. 결과적으로 로그인 실패 시 사용자에게 아무런 피드백이 전달되지 않습니다.
checkOnboardingStatus()의 onError 블록(Line 63-70)에서는 올바르게 emitSideEffect()를 호출하고 있으므로, 동일한 패턴을 적용해야 합니다.
🐛 수정 제안
is LoginResult.Failure -> {
- LoginSideEffect.ShowToast(
- message = R.string.login_fail_message,
- type = ToastType.ERROR,
- )
+ emitSideEffect(
+ LoginSideEffect.ShowToast(
+ message = R.string.login_fail_message,
+ type = ToastType.ERROR,
+ ),
+ )
}🤖 Prompt for AI Agents
In `@feature/login/src/main/java/com/twix/login/LoginViewModel.kt` around lines 33
- 38, In the LoginViewModel, the LoginResult.Failure branch constructs a
LoginSideEffect.ShowToast but never calls emitSideEffect, so the toast never
appears; modify the LoginResult.Failure branch in the function handling login
results to call emitSideEffect(LoginSideEffect.ShowToast(...)) instead of just
creating the object, matching the pattern used in checkOnboardingStatus()'s
onError block (use the same emitSideEffect call with message =
R.string.login_fail_message and type = ToastType.ERROR).
| private suspend fun handleCoupleConnectException(error: AppError) { | ||
| if (error is AppError.Http && error.status == 404) { | ||
| /** | ||
| * 초대 코드를 잘못 입력한 경우 | ||
| * */ | ||
| if (error.message == INVALID_INVITE_CODE_MESSAGE) { | ||
| emitSideEffect(OnBoardingSideEffect.InviteCode.ShowInvalidInviteCodeToast) | ||
| } else if (error.message == ALREADY_USED_INVITE_CODE_MESSAGE) { | ||
| /** | ||
| * 상대방이 이미 연결한 경우 | ||
| * */ | ||
| emitSideEffect(OnBoardingSideEffect.InviteCode.NavigateToNext) | ||
| } else { | ||
| emitSideEffect(OnBoardingSideEffect.InviteCode.ShowConnectCoupleConnectFailToast) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
404 이외의 HTTP 에러나 네트워크 에러가 무시됩니다.
handleCoupleConnectException이 AppError.Http && status == 404만 처리하고 있어, 서버 500 에러나 네트워크 타임아웃 등 다른 에러 타입은 사용자에게 아무런 피드백 없이 무시됩니다.
이전 리뷰에서 지적된 문자열 기반 분기에 대한 else 분기 추가(Line 74-76)는 잘 반영되었습니다. 추가로 404가 아닌 에러에 대한 fallback 처리도 고려해보시면 좋겠습니다.
🛡️ 수정 제안
private suspend fun handleCoupleConnectException(error: AppError) {
if (error is AppError.Http && error.status == 404) {
if (error.message == INVALID_INVITE_CODE_MESSAGE) {
emitSideEffect(OnBoardingSideEffect.InviteCode.ShowInvalidInviteCodeToast)
} else if (error.message == ALREADY_USED_INVITE_CODE_MESSAGE) {
emitSideEffect(OnBoardingSideEffect.InviteCode.NavigateToNext)
} else {
emitSideEffect(OnBoardingSideEffect.InviteCode.ShowConnectCoupleConnectFailToast)
}
+ } else {
+ emitSideEffect(OnBoardingSideEffect.InviteCode.ShowConnectCoupleConnectFailToast)
}
}🤖 Prompt for AI Agents
In
`@feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt`
around lines 62 - 78, handleCoupleConnectException currently only handles
AppError.Http with status == 404; add fallback branches to surface feedback for
all other errors: for AppError.Http with non-404 statuses emit a generic failure
side effect (e.g.,
OnBoardingSideEffect.InviteCode.ShowConnectCoupleConnectFailToast or a new
status-aware failure side effect), and for non-Http/AppError network or
unexpected errors emit a network/error side effect (e.g.,
OnBoardingSideEffect.InviteCode.ShowNetworkErrorToast or a generic
ShowErrorToast). Update the function handleCoupleConnectException to check for
error type first (AppError.Http vs others) and ensure every path emits an
appropriate emitSideEffect so users get feedback for 5xx, timeouts, and unknown
errors.
| <string name="onboarding_name_placeholder">닉네임을 입력해 주세요.</string> | ||
| <string name="onboarding_name_helper">닉네임 2~8자</string> | ||
| <string name="onboarding_profile_title">짝꿍에게 보일\n내 이름을 입력해 주세요</string> | ||
| <string name="onboarding_profile_placeholder">닉네임을 입력해 주세요.</string> | ||
| <string name="onboarding_profile_helper">닉네임 2~8자</string> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 중복 키들의 사용처 확인
echo "=== onboarding_name_placeholder 사용처 ==="
rg "onboarding_name_placeholder" --type xml --type kotlin
echo ""
echo "=== onboarding_profile_placeholder 사용처 ==="
rg "onboarding_profile_placeholder" --type xml --type kotlin
echo ""
echo "=== onboarding_name_helper 사용처 ==="
rg "onboarding_name_helper" --type xml --type kotlin
echo ""
echo "=== onboarding_profile_helper 사용처 ==="
rg "onboarding_profile_helper" --type xml --type kotlinRepository: YAPP-Github/Twix-Android
Length of output: 1009
🏁 Script executed:
# strings.xml 파일 전체 내용 확인
cat -n feature/onboarding/src/main/res/values/strings.xmlRepository: YAPP-Github/Twix-Android
Length of output: 2688
사용되지 않는 문자열 리소스를 제거하세요.
onboarding_profile_placeholder(Line 7)와 onboarding_profile_helper(Line 8)는 현재 코드에서 참조되지 않는 미사용 리소스입니다. ProfileScreen.kt에서는 onboarding_name_placeholder와 onboarding_name_helper만 사용되고 있습니다.
왜 문제가 되는지: 사용되지 않는 리소스는 유지보수 비용을 증가시키고, 코드베이스의 혼란을 가져올 수 있습니다. 또한 개발 중 실수로 추가되었을 가능성이 있으므로 정리가 필요합니다.
개선 방향: Lines 7-8의 onboarding_profile_placeholder와 onboarding_profile_helper를 제거하세요. 향후 프로필과 이름 입력 필드에 다른 문구가 필요하다면, 그때 새로운 키를 추가하는 것이 좋습니다.
🤖 Prompt for AI Agents
In `@feature/onboarding/src/main/res/values/strings.xml` around lines 4 - 8,
Remove the two unused string resources onboarding_profile_placeholder and
onboarding_profile_helper from the strings.xml file; locate the entries with
those resource names and delete them, and before committing run a project-wide
search for onboarding_profile_placeholder and onboarding_profile_helper to
ensure nothing references them (ProfileScreen.kt already uses
onboarding_name_placeholder and onboarding_name_helper), adding or renaming keys
only if a future distinct profile placeholder/helper string is required.
2 participants
이슈 번호
#35
리뷰/머지 희망 기한 (선택)
작업내용
결과물
리뷰어에게 추가로 요구하는 사항 (선택)
토끼가 너무 뛰노는 바람에 기존에 PR이 너무 엉망이 되서 다시 만들었어 !