Conversation
# Conflicts: # app/src/main/java/com/yapp/twix/main/MainActivity.kt
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
data/src/main/java/com/twix/data/repository/DefaultOnboardingRepository.kt (1)
14-17:⚠️ Potential issue | 🟠 MajorInviteCode 생성 시 팩토리 메서드를 사용하여 도메인 유효성 검증을 거쳐야 합니다
InviteCode는
create(value: String): Result<InviteCode>팩토리 메서드로만 안전하게 생성되어야 합니다. 현재 14-17줄의 직접 생성자 호출은 이 검증 로직을 완전히 우회하고 있습니다.문제점:
- InviteCode는 정확히 8자 길이 검증이 필수이나, 직접 생성자는 이 검증을 수행하지 않습니다
- 서버 응답이 항상 유효하다고 가정하지만, 도메인 계약을 보호하지 않습니다
- 에러 발생 시 도메인 레벨의 InviteCodeException으로 변환되지 않습니다
개선 방법:
override suspend fun fetchInviteCode(): InviteCode { val response = service.fetchInviteCode() return InviteCode.create(response.inviteCode) .getOrElse { throw it } // 또는 도메인 에러로 매핑 }또는 Result를 그대로 처리하는 구조를 원한다면, 서버 응답 검증 이유를 명확한 주석으로 남겨주세요.
🤖 Fix all issues with AI agents
In `@domain/src/main/java/com/twix/domain/model/invitecode/InviteCode.kt`:
- Around line 8-12: InviteCode.create 현재 require(...)가 실패할 때
IllegalArgumentException을 던져 실제 의도한 InviteCodeException.InvalidLengthException가
발생하지 않으므로, 길이 검증 실패 시 명시적으로 InviteCodeException.InvalidLengthException을 throw하도록
수정하세요; 즉 InviteCode.create의 검증 로직(InviteCode.create,
InviteCodeException.InvalidLengthException)을 require 대신 if 문 또는 직접 throw로 바꿔
runCatching이 의도한 InviteCodeException을 캡처하도록 만드세요.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/couple/component/ConnectButton.kt`:
- Around line 46-51: The border is drawn before the background in
ConnectButton.kt which allows the background to visually obscure the border;
swap the order so the background modifier is applied first and the border
modifier is applied last (i.e., apply background before border on the same
composable) so the border is rendered on top; update the modifier chain that
currently uses .border(...).background(...).noRippleClickable { onClickConnect()
} to apply .background(...) then .border(...) and keep .noRippleClickable {
onClickConnect() } unchanged.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/couple/CoupleConnectRoute.kt`:
- Around line 30-39: CoupleConnectRoute currently injects viewModel
(koinViewModel()) but never uses it; either remove the unused viewModel
parameter from the CoupleConnectRoute signature and call site if it’s not
needed, or if onClickSend is intended to be implemented later, keep the
viewModel and replace the empty lambda onClickSend = { } with a TODO comment
(e.g., // TODO: implement send using viewModel) and then reference viewModel
inside the future implementation; update the CoupleConnectScreen invocation
accordingly and ensure no unused-import or lint warnings remain.
- Around line 71-75: 현재 두 버튼이 동일한 핸들러를 사용하여 의도와 다르게 동작합니다: InvitationButton과
ConnectButton이 둘 다 onClickConnect를 호출하고 있어 초대 전송 로직(onClickSend)이 사용되지 않습니다.
CoupleConnectRoute에서 InvitationButton의 onClick 인자를 onClickSend로 변경하여 초대 전송 핸들러를
연결하고, ConnectButton은 그대로 onClickConnect를 유지하도록 수정하세요; 관련 식별자는 InvitationButton,
ConnectButton, onClickSend, onClickConnect입니다.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/invite/InviteCodeScreen.kt`:
- Around line 161-168: The click handler currently only uses
clipboardManager.setText(AnnotatedString(uiModel.myInviteCode)) and must also
dispatch the copy intent to the ViewModel so the ShowCopyInviteCodeSuccessToast
side effect runs; update the Modifier.noRippleClickable lambda (the Image
composable click) to dispatch the appropriate intent (e.g., an intent named
ShowCopyInviteCodeSuccessToast or CopyInviteCode) to the screen's ViewModel
after setting the clipboard, or instead move the clipboard copy logic into the
ViewModel and have the click handler only dispatch that CopyInviteCode intent;
ensure you reference the existing clipboardManager, uiModel.myInviteCode, and
the ViewModel intent name (ShowCopyInviteCodeSuccessToast / CopyInviteCode) when
making the change.
- Around line 69-82: Replace the current LaunchedEffect(viewModel.sideEffect)
block with a collector that collects viewModel.sideEffect Flow (e.g.,
LaunchedEffect(Unit) { viewModel.sideEffect.collect { effect -> when(effect) {
is OnBoardingSideEffect.InviteCode.ShowCopyInviteCodeSuccessToast ->
toastManager.tryShow(ToastData(message =
context.getString(R.string.onboarding_invite_code_copy), type =
ToastType.SUCCESS)) else -> Unit } } }) so the composable responds to emitted
side effects, and update the copy button handler (the Modifier.noRippleClickable
handler that currently calls clipboardManager.setText) to also dispatch the
intent to the ViewModel by calling
viewModel.dispatch(OnBoardingIntent.CopyInviteCode) before/after
clipboardManager.setText so the ViewModel can emit the
ShowCopyInviteCodeSuccessToast side effect.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/navigation/OnboardingNavGraph.kt`:
- Around line 30-44: The current popUpTo usage in the onNext handlers for
InviteCodeRoute and the prior step uses popUpTo(graphRoute.route) { inclusive =
true }, which fully removes previous steps and prevents back navigation; update
both occurrences (the onNext in the composable that navigates to
NavRoutes.InviteRoute.route and the onNext that navigates to
NavRoutes.ProfileRoute.route) to target the specific earlier step you want to
keep in the back stack—for example use
popUpTo(NavRoutes.CoupleConnectionRoute.route) { inclusive = false }—so previous
screens (Couple/Invite) remain reachable, or explicitly confirm and leave as-is
if the intent is to block back navigation entirely.
In
`@feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt`:
- Around line 15-18: The init block in OnBoardingViewModel currently has
fetchInviteCode() commented out which leaves myInviteCode empty and breaks the
CopyInviteCode intent; un-comment or re-enable fetchInviteCode() so it runs when
the onboarding screen is entered (or invoke fetchInviteCode() immediately after
successful login), and improve the error logging inside fetchInviteCode() by
replacing Log.d("FetchInviteCodeException", "$it") with a log that includes the
throwable/stacktrace (e.g., Log.e or pass the Exception to the logger or call
it.printStackTrace()) so failures are recorded with full diagnostics.
🧹 Nitpick comments (9)
app/src/main/java/com/yapp/twix/main/MainActivity.kt (1)
28-28: 주석 처리된 코드 정리 및 Safe Area 처리 확인 필요
.safeContentPadding()이 제거가 아닌 주석 처리되어 있습니다. 이는 두 가지 관점에서 검토가 필요합니다:
코드 품질 관점: 주석 처리된 코드는 제거하는 것이 좋습니다. 필요시 Git 히스토리를 통해 복구할 수 있으므로, 코드베이스에 남겨둘 필요가 없습니다.
UI 관점:
safeContentPadding()없이는 노치, 라운드 코너, 제스처 네비게이션 바 등이 있는 기기에서 콘텐츠가 시스템 UI와 겹칠 수 있습니다.개별 화면에서 Safe Area를 직접 처리하도록 변경한 것인지, 아니면 디버깅 중 임시로 비활성화한 것인지 확인 부탁드립니다. 의도적인 변경이라면 해당 라인을 완전히 삭제하는 것을 권장합니다.
♻️ 주석 처리된 코드 제거 제안
modifier = Modifier - // .safeContentPadding() .fillMaxSize(),feature/onboarding/src/main/res/drawable/ic_copy.xml (1)
8-14: 아이콘이 잘 추가되었습니다. 색상 관리에 대해 한 가지 제안드립니다.현재 하드코딩된 색상값(
#F4F4F4,#858585)을 사용하고 있습니다. 벡터 드로어블에서는 흔한 패턴이지만, 디자인 시스템의 색상 리소스를 참조하면 테마 변경 시 일관성을 유지하기 더 쉽습니다.다크 모드 지원이나 테마 변경이 예정되어 있다면,
@color/리소스 참조를 고려해 보시는 것은 어떨까요?feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt (1)
20-25: 실패 로그에 스택트레이스를 남겨 원인 파악을 쉽게 해주세요Log.d로 문자열만 남기면 장애 분석이 어렵습니다.
Log.e(tag, msg, throwable)형태로 스택트레이스를 남기거나, 필요 시 UI에 실패를 전달하는 쪽을 고려해 주세요.♻️ 제안 변경
- .onFailure { Log.d("FetchInviteCodeException", "$it") } + .onFailure { Log.e("FetchInviteCodeException", "fetchInviteCode 실패", it) }feature/onboarding/src/main/java/com/twix/onboarding/invite/component/InviteCodeTextField.kt (2)
62-68: 포커스된 박스만 깜빡임 애니메이션을 돌리면 리소스를 줄일 수 있어요현재 모든
CodeBox에서 무한 애니메이션이 동작합니다. 포커스된 박스에만 애니메이션을 적용하면 불필요한 recomposition/애니메이션 비용을 줄일 수 있습니다.♻️ 제안 변경
- val infiniteTransition = rememberInfiniteTransition() - val alpha by infiniteTransition.animateFloat( - 0f, - 1f, - infiniteRepeatable(tween(500), RepeatMode.Reverse), - ) + val alpha = + if (isFocused) { + val infiniteTransition = rememberInfiniteTransition() + val animatedAlpha by infiniteTransition.animateFloat( + 0f, + 1f, + infiniteRepeatable(tween(500), RepeatMode.Reverse), + ) + animatedAlpha + } else { + 0f + }
37-54: 초대 코드는 단일 라인/ASCII 키보드로 제한하는 게 UX에 유리합니다다중 라인 입력은 불필요하고 IME가 문자형으로 뜰 수 있습니다.
singleLine과keyboardOptions로 입력 경험을 고정해주세요.💡 제안 변경
import androidx.compose.foundation.text.BasicTextField import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.text.TextRange +import androidx.compose.ui.text.input.ImeAction +import androidx.compose.ui.text.input.KeyboardOptions +import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.text.input.TextFieldValue @@ BasicTextField( modifier = modifier, value = TextFieldValue(inviteCode, selection = TextRange(inviteCode.length)), + singleLine = true, + keyboardOptions = KeyboardOptions( + keyboardType = KeyboardType.Ascii, + imeAction = ImeAction.Done, + ), onValueChange = { if (it.text.length <= InviteCode.INVITE_CODE_LENGTH) { onValueChange(it.text) } },domain/src/test/java/com/twix/domain/model/InviteCodeTest.kt (1)
24-30: 실패 원인 타입까지 검증하면 회귀를 더 빨리 잡을 수 있습니다현재는
isFailure만 확인해 실패 원인이 바뀌어도 테스트가 통과합니다.InvalidLengthException여부까지 확인하면 의도가 더 명확해집니다.✅ 제안 변경
import com.twix.domain.model.invitecode.InviteCode +import com.twix.domain.model.invitecode.InviteCodeException import org.assertj.core.api.Assertions.assertThat @@ // when val result = InviteCode.create(invalidLengthCode) + val throwable = result.exceptionOrNull() // then assertThat(result.isFailure).isTrue() + assertThat(throwable) + .isInstanceOf(InviteCodeException.InvalidLengthException::class.java)domain/src/main/java/com/twix/domain/model/invitecode/InviteCode.kt (1)
3-6: 생성자가 공개면 검증을 우회할 수 있는데 의도된 건가요?현재는
InviteCode(value)로 유효성 체크 없이 생성 가능합니다. 도메인 불변식을 강제하려면 생성자를private로 두고create만 공개하는 방향을 검토해주세요. 의도적으로 열어둔 것인지 확인 부탁드립니다.🔒 제안 변경
`@JvmInline` -value class InviteCode( +value class InviteCode private constructor( val value: String, )feature/onboarding/src/main/java/com/twix/onboarding/model/OnBoardingUiState.kt (1)
4-4: 네이밍 컨벤션 확인:InViteCodeUiModel→InviteCodeUiModel
InViteCodeUiModel에서 중간에 대문자V가 사용되어 있습니다. 일반적인 PascalCase 컨벤션에 따르면InviteCodeUiModel이 더 적합합니다. 이 네이밍이 의도된 것인지 확인이 필요합니다.Also applies to: 11-11
feature/onboarding/src/main/java/com/twix/onboarding/couple/CoupleConnectRoute.kt (1)
79-88: Preview 함수 접근 제한자 권장Preview 함수는 외부에서 사용되지 않으므로
private으로 선언하는 것이 좋습니다.InviteCodeScreen.kt의 Preview와 일관성을 맞추는 것도 고려해주세요.♻️ 수정 제안
`@Preview`(showBackground = true) `@Composable` -fun CoupleConnectScreenPreview() { +private fun CoupleConnectScreenPreview() { TwixTheme {
feature/onboarding/src/main/java/com/twix/onboarding/couple/component/ConnectButton.kt
Outdated
Show resolved
Hide resolved
feature/onboarding/src/main/java/com/twix/onboarding/couple/CoupleConnectRoute.kt
Show resolved
Hide resolved
feature/onboarding/src/main/java/com/twix/onboarding/couple/CoupleConnectRoute.kt
Outdated
Show resolved
Hide resolved
| LaunchedEffect(viewModel.sideEffect) { | ||
| when (viewModel.sideEffect) { | ||
| OnBoardingSideEffect.InviteCode.ShowCopyInviteCodeSuccessToast -> { | ||
| toastManager.tryShow( | ||
| ToastData( | ||
| message = context.getString(R.string.onboarding_invite_code_copy), | ||
| type = ToastType.SUCCESS, | ||
| ), | ||
| ) | ||
| } | ||
|
|
||
| else -> Unit | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*InviteCode*" | head -20Repository: YAPP-Github/Twix-Android
Length of output: 550
🏁 Script executed:
cd feature/onboarding && find . -type f -name "*.kt" | grep -i viewmodel | head -20Repository: YAPP-Github/Twix-Android
Length of output: 129
🏁 Script executed:
cat feature/onboarding/src/main/java/com/twix/onboarding/invite/InviteCodeScreen.kt | head -100Repository: YAPP-Github/Twix-Android
Length of output: 4024
🏁 Script executed:
cat feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt | head -150Repository: YAPP-Github/Twix-Android
Length of output: 2182
🏁 Script executed:
cat feature/onboarding/src/main/java/com/twix/onboarding/invite/InviteCodeScreen.kt | tail -100Repository: YAPP-Github/Twix-Android
Length of output: 3569
🏁 Script executed:
find . -type f -name "BaseViewModel.kt" -o -name "*ViewModel.kt" | grep -E "base|Base" | head -10Repository: YAPP-Github/Twix-Android
Length of output: 125
🏁 Script executed:
rg -t kt "class BaseViewModel" --max-count 5Repository: YAPP-Github/Twix-Android
Length of output: 94
🏁 Script executed:
cat core/ui/src/main/java/com/twix/ui/base/BaseViewModel.ktRepository: YAPP-Github/Twix-Android
Length of output: 3340
🏁 Script executed:
cat core/ui/src/main/java/com/twix/ui/base/BaseViewModel.kt | grep -A 5 "SideEffectHolder"Repository: YAPP-Github/Twix-Android
Length of output: 255
🏁 Script executed:
rg "class SideEffectHolder" -A 10Repository: YAPP-Github/Twix-Android
Length of output: 746
SideEffect 수집 패턴 수정 필수
현재 구현에는 두 가지 근본적인 문제가 있습니다:
-
LaunchedEffect 키 패턴 오류:
sideEffect는 Channel을 통해 생성된 Flow 객체입니다. Flow 객체의 참조는 변경되지 않으므로,LaunchedEffect(viewModel.sideEffect)는 초기 구성 시 단 한 번만 실행되고 이후 발생하는 SideEffect에는 반응하지 않습니다. 또한when(viewModel.sideEffect)는 Flow 객체 자체를 SideEffect 값과 비교하려고 하므로else분기만 실행됩니다. -
복사 버튼 Intent 미발송: 현재 복사 아이콘의 클릭 핸들러(약 165-167줄)는
clipboardManager.setText()만 호출하고OnBoardingIntent.CopyInviteCode를 ViewModel에 전달하지 않습니다. 결과적으로 ViewModel의 Intent 핸들러가 실행되지 않아 SideEffect가 발생하지 않고 토스트가 나타나지 않습니다.
다음과 같이 수정해야 합니다:
🔧 수정 방법
1. LaunchedEffect를 Flow collect 패턴으로 변경:
LaunchedEffect(Unit) {
viewModel.sideEffect.collect { effect ->
when (effect) {
is OnBoardingSideEffect.InviteCode.ShowCopyInviteCodeSuccessToast -> {
toastManager.tryShow(
ToastData(
message = context.getString(R.string.onboarding_invite_code_copy),
type = ToastType.SUCCESS,
),
)
}
else -> Unit
}
}
}2. 복사 버튼에 Intent 발송 추가:
Modifier.noRippleClickable {
viewModel.dispatch(OnBoardingIntent.CopyInviteCode)
clipboardManager.setText(AnnotatedString(uiModel.myInviteCode))
}🤖 Prompt for AI Agents
In
`@feature/onboarding/src/main/java/com/twix/onboarding/invite/InviteCodeScreen.kt`
around lines 69 - 82, Replace the current LaunchedEffect(viewModel.sideEffect)
block with a collector that collects viewModel.sideEffect Flow (e.g.,
LaunchedEffect(Unit) { viewModel.sideEffect.collect { effect -> when(effect) {
is OnBoardingSideEffect.InviteCode.ShowCopyInviteCodeSuccessToast ->
toastManager.tryShow(ToastData(message =
context.getString(R.string.onboarding_invite_code_copy), type =
ToastType.SUCCESS)) else -> Unit } } }) so the composable responds to emitted
side effects, and update the copy button handler (the Modifier.noRippleClickable
handler that currently calls clipboardManager.setText) to also dispatch the
intent to the ViewModel by calling
viewModel.dispatch(OnBoardingIntent.CopyInviteCode) before/after
clipboardManager.setText so the ViewModel can emit the
ShowCopyInviteCodeSuccessToast side effect.
| Image( | ||
| imageVector = ImageVector.vectorResource(R.drawable.ic_copy), | ||
| contentDescription = null, | ||
| modifier = | ||
| Modifier.noRippleClickable { | ||
| clipboardManager.setText(AnnotatedString(uiModel.myInviteCode)) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
복사 액션에 Intent 디스패치 누락
클립보드 복사 후 성공 토스트를 표시하려면 ViewModel에 Intent를 디스패치해야 합니다. 현재 코드는 클립보드에 직접 복사만 하고, ViewModel에 알리지 않아 ShowCopyInviteCodeSuccessToast side effect가 발생하지 않습니다.
MVI 패턴에서는 사용자 액션 → Intent → ViewModel → SideEffect 순서로 처리되어야 합니다.
🔧 Intent 디스패치 추가 제안
Image(
imageVector = ImageVector.vectorResource(R.drawable.ic_copy),
contentDescription = null,
modifier =
Modifier.noRippleClickable {
clipboardManager.setText(AnnotatedString(uiModel.myInviteCode))
+ // ViewModel에 복사 완료 Intent 디스패치 필요
+ // onCopyInviteCode() 콜백 추가 고려
},
)또는 클립보드 복사 로직 자체를 ViewModel로 이동하고, Screen에서는 Intent만 디스패치하는 방식도 고려해보세요.
🤖 Prompt for AI Agents
In
`@feature/onboarding/src/main/java/com/twix/onboarding/invite/InviteCodeScreen.kt`
around lines 161 - 168, The click handler currently only uses
clipboardManager.setText(AnnotatedString(uiModel.myInviteCode)) and must also
dispatch the copy intent to the ViewModel so the ShowCopyInviteCodeSuccessToast
side effect runs; update the Modifier.noRippleClickable lambda (the Image
composable click) to dispatch the appropriate intent (e.g., an intent named
ShowCopyInviteCodeSuccessToast or CopyInviteCode) to the screen's ViewModel
after setting the clipboard, or instead move the clipboard copy logic into the
ViewModel and have the click handler only dispatch that CopyInviteCode intent;
ensure you reference the existing clipboardManager, uiModel.myInviteCode, and
the ViewModel intent name (ShowCopyInviteCodeSuccessToast / CopyInviteCode) when
making the change.
feature/onboarding/src/main/java/com/twix/onboarding/navigation/OnboardingNavGraph.kt
Show resolved
Hide resolved
feature/onboarding/src/main/java/com/twix/onboarding/vm/OnBoardingViewModel.kt
Show resolved
Hide resolved
| onComplete: () -> Unit, | ||
| ) { | ||
| val focusRequester = remember { FocusRequester() } | ||
| val clipboardManager = LocalClipboardManager.current |
There was a problem hiding this comment.
LocalClipboard 이걸 사용하라고 경고가 뜨고 있어요
| Text( | ||
| text = stringResource(R.string.onboarding_couple_connect_direct), | ||
| fontFamily = NanumSquareNeoFamily, | ||
| fontWeight = ExtraBold, | ||
| color = GrayColor.C500, | ||
| fontSize = 16.sp, | ||
| lineHeight = 22.2.sp, | ||
| ) |
There was a problem hiding this comment.
이게 아마 제가 올려놓은 공통 바텀시트 PR에는 디자인시스템 새로 추가해놨을 거예요. 그거 머지되면 시스템에 맞게 수정해주세요!
| }, | ||
| decorationBox = { | ||
| Row( | ||
| horizontalArrangement = Arrangement.spacedBy(4.29.dp), |
There was a problem hiding this comment.
이렇게 소수점으로 되어있는 것들은 나중에 디자이너 친구들이랑 확인해서 정수로 다 바꿔주세요!
2 participants
이슈 번호
#35
리뷰/머지 희망 기한 (선택)
아직 리뷰 미요청 !
작업내용
결과물
2132.mp4
리뷰어에게 추가로 요구하는 사항 (선택)