[feat] #80 스플래시~이용약관 프로세스 구성#91
Conversation
…m-2-Android into feat/#80-login-onboarding
…m-2-Android into feat/#80-login-onboarding
Ojongseok
force-pushed
the
feat/#80-login-onboarding
branch
from
556bc51 to
90f7f1b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/common/src/main/java/com/neki/android/core/common/kakao/KakaoAuthHelper.kt (1)
17-17:⚠️ Potential issue | 🟠 Major
token.idToken!!강제 언래핑은 NPE 위험이 있습니다.Kakao SDK의
OAuthToken.idToken은 nullable 타입입니다. OIDC 설정이 누락되었거나 토큰에idToken이 포함되지 않은 경우 런타임에NullPointerException이 발생합니다. Line 25도 동일한 문제가 있습니다.🛡️ null 처리 제안
- onSuccess(token.idToken!!) + val idToken = token.idToken + if (idToken != null) { + onSuccess(idToken) + } else { + onFailure("ID 토큰을 가져올 수 없습니다.") + }Line 25에도 동일하게 적용해 주세요.
🤖 Fix all issues with AI agents
In
`@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginScreen.kt`:
- Line 27: The collected but unused uiState in LoginScreen (val uiState by
viewModel.store.uiState.collectAsStateWithLifecycle()) should be removed or
actually used: either delete that line to avoid unnecessary state collection and
extra recompositions, or pass uiState into the composable UI/child components or
reference it inside LoginScreen (e.g., use uiState.isLoading / uiState.error) so
the collected flow is consumed; locate the declaration in LoginScreen and update
accordingly.
🧹 Nitpick comments (2)
core/common/src/main/java/com/neki/android/core/common/kakao/KakaoAuthHelper.kt (1)
12-28: KakaoTalk/KakaoAccount 로그인 콜백 로직이 중복됩니다.두 분기의 콜백 처리가 동일하므로 헬퍼 함수로 추출하면 유지보수성이 향상됩니다.
♻️ 중복 제거 제안
+ private fun handleLoginResult( + token: com.kakao.sdk.auth.model.OAuthToken?, + error: Throwable?, + onSuccess: (String) -> Unit, + onFailure: (String) -> Unit, + ) { + if (error != null) { + onFailure(error.message ?: "카카오 로그인에 실패했습니다.") + } else if (token != null) { + val idToken = token.idToken + if (idToken != null) { + onSuccess(idToken) + } else { + onFailure("ID 토큰을 가져올 수 없습니다.") + } + } + } + fun login( context: Context, onSuccess: (String) -> Unit, onFailure: (String) -> Unit, ) { if (UserApiClient.instance.isKakaoTalkLoginAvailable(context)) { - UserApiClient.instance.loginWithKakaoTalk(context) { token, error -> - if (error != null) { - onFailure(error.message ?: "카카오 로그인에 실패했습니다.") - } else if (token != null) { - onSuccess(token.idToken!!) - } - } + UserApiClient.instance.loginWithKakaoTalk(context) { token, error -> + handleLoginResult(token, error, onSuccess, onFailure) + } } else { - UserApiClient.instance.loginWithKakaoAccount(context) { token, error -> - if (error != null) { - onFailure(error.message ?: "카카오 로그인에 실패했습니다.") - } else if (token != null) { - onSuccess(token.idToken!!) - } - } + UserApiClient.instance.loginWithKakaoAccount(context) { token, error -> + handleLoginResult(token, error, onSuccess, onFailure) + } } }feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginScreen.kt (1)
48-49:else -> {}분기에서 일부 side effect가 무시될 수 있습니다.
LoginViewModel은LoginSideEffect.NavigateBack,LoginSideEffect.NavigateUrl등의 side effect도 발행할 수 있습니다. 공유 ViewModel 특성상 Term 화면에서 처리될 수 있지만,else분기로 무시하면 디버깅이 어렵습니다. 의도적으로 무시하는 것이라면 주석을 남기거나, 명시적으로 나열하는 것을 권장합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginScreen.kt (1)
35-37:⚠️ Potential issue | 🟠 Major카카오 로그인 실패 시
LoginIntent.FailLogin이 디스패치되지 않습니다.
onFailure콜백에서Timber.d로 로깅만 하고FailLogin인텐트를 전달하지 않습니다. ViewModel에서FailLogin은 사용자에게 토스트 메시지를 표시하도록 구현되어 있으므로, 현재 로그인 실패 시 사용자에게 아무런 피드백이 제공되지 않습니다.🐛 수정 제안
onFailure = { message -> Timber.d("로그인 실패 $message") + viewModel.store.onIntent(LoginIntent.FailLogin) },
🤖 Fix all issues with AI agents
In
`@core/designsystem/src/main/java/com/neki/android/core/designsystem/logo/NekiAppLogo.kt`:
- Around line 47-53: The WhiteNekiAppLogoPreview renders a white logo on a white
NekiTheme background so it's invisible; update the preview function
WhiteNekiAppLogoPreview to wrap the call to WhiteNekiAppLogo with a dark
background preview container (e.g., use the already-imported ComponentPreview or
wrap in a Box with a non-white background) while keeping NekiTheme so the rest
of theming remains applied; ensure the preview composable still uses `@Preview`
and the function name WhiteNekiAppLogoPreview to keep tooling recognition.
🧹 Nitpick comments (5)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/splash/component/SplashBackground.kt (1)
37-38: 소수점 dp 값이 지나치게 정밀합니다.
73.89.dp,80.41.dp처럼 소수점 두 자리까지 지정하는 것은 실제 렌더링에서 의미 있는 차이를 만들지 않습니다. 디자인 스펙 원본 값을 반올림하여74.dp x 80.dp등으로 정리하면 가독성이 높아집니다.feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginScreen.kt (1)
44-45:else -> {}가 처리되지 않은 사이드 이펙트를 무시합니다.
LoginViewModel은NavigateBack,NavigateUrl등의 사이드 이펙트도 발행할 수 있습니다. 현재else -> {}는 이들을 조용히 무시하므로, 디버깅 시 문제 추적이 어려울 수 있습니다. 공유 ViewModel 구조에서 예기치 않은 사이드 이펙트가 여기로 전달될 가능성이 있으므로, 최소한 로깅을 추가하는 것을 권장합니다.♻️ 리팩터 제안
- else -> {} + else -> Timber.d("LoginRoute에서 처리되지 않은 sideEffect: $sideEffect")core/designsystem/src/main/java/com/neki/android/core/designsystem/logo/NekiAppLogo.kt (1)
14-25:contentDescription이null로 하드코딩되어 있어 접근성 지원이 불가합니다.앱 로고는 장식 요소가 아닌 브랜딩 의미를 전달하므로, 스크린 리더 사용자를 위해 콘텐츠 설명이 필요합니다. 또한 공개 API(
WhiteNekiAppLogo,PrimaryNekiAppLogo)에서도contentDescription파라미터를 노출하지 않아 호출부에서 재정의할 수 없습니다.♻️ 개선 제안
`@Composable` private fun NekiAppLogo( color: Color, modifier: Modifier = Modifier, + contentDescription: String? = null, ) { Icon( imageVector = ImageVector.vectorResource(R.drawable.icon_neki_logo_white), - contentDescription = null, + contentDescription = contentDescription, tint = color, modifier = modifier, ) }공개 composable에도
contentDescription파라미터를 전달할 수 있도록 확장하면 좋습니다.feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginViewModel.kt (2)
85-105:onSuccess블록 내 예외 발생 시isLoading이true로 고정될 수 있습니다.
tokenRepository.saveTokens()또는authRepository.setCompletedOnboarding()에서 예외가 발생하면, Line 104의reduce { copy(isLoading = false) }에 도달하지 못합니다.try-finally또는invokeOnCompletion으로isLoading해제를 보장하는 것이 안전합니다.♻️ 수정 제안
private fun loginWithKakao( idToken: String, reduce: (LoginState.() -> LoginState) -> Unit, postSideEffect: (LoginSideEffect) -> Unit, ) = viewModelScope.launch { reduce { copy(isLoading = true) } - authRepository.loginWithKakao(idToken) - .onSuccess { - tokenRepository.saveTokens( - accessToken = it.accessToken, - refreshToken = it.refreshToken, - ) - authRepository.setCompletedOnboarding(true) - postSideEffect(LoginSideEffect.NavigateToMain) - } - .onFailure { exception -> - Timber.e(exception) - postSideEffect(LoginSideEffect.ShowToastMessage("로그인에 실패했습니다. 다시 시도해주세요.")) - } - reduce { copy(isLoading = false) } + try { + authRepository.loginWithKakao(idToken) + .onSuccess { + tokenRepository.saveTokens( + accessToken = it.accessToken, + refreshToken = it.refreshToken, + ) + authRepository.setCompletedOnboarding(true) + postSideEffect(LoginSideEffect.NavigateToMain) + } + .onFailure { exception -> + Timber.e(exception) + postSideEffect(LoginSideEffect.ShowToastMessage("로그인에 실패했습니다. 다시 시도해주세요.")) + } + } finally { + reduce { copy(isLoading = false) } + } }
43-49:ClickAgreeAll이 필수 약관만 토글합니다 — 의도된 동작인지 확인 필요.
ClickAgreeAll처리에서isRequired항목만agreedTerms에 추가하고, 해제 시에는 전체(persistentSetOf())를 비웁니다. 선택 약관이 존재하는 경우 "전체 동의" 해제 시 개별적으로 동의했던 선택 약관도 함께 초기화됩니다. 의도된 UX라면 문제없지만, 확인이 필요합니다.
There was a problem hiding this comment.
Q. 현재 메인화면에서의 화면 전환에 사용되는 Navigator/NavigatorImpl을 MainNavigator/MainNavigatorImpl로 네이밍을 변경할까요?
A. 기존 NavigationState 와 동일한 Level 의 AuthNavigationState 가 생겼으니,
MainNavigationState로 네이밍은 구별하는 건 어떨까요
MainNavigator/MainNavigatorImpl에 더해 NavigationState 또한 MainNavigationState로 변경하자는 말씀이신거 맞나요?
HiltSharedViewModelStoreNavEntryDecorator와 관련해 확인해주셔 감사합니다!
onPop{}을 제어할 방법을 못찾았었는데 onPop{}은 리컴포지션 시 호출되는 블럭이었군요..!
로그로 확인해보니 ViewModel 정상적으로 제거되었고, 메인 <-> 로그인 이동할 때 작성해주신 방법으로 메인쪽의 ViewModel도 onCleaed되는 것 확인했습니다!
해당 PR에서 같이 반영되면 좋을 것 같아 수정하였습니다. e4feb77
ikseong00
left a comment
ikseong00
left a comment
There was a problem hiding this comment.
리코멘트 남겼습니다!
Image 는 이후 PR에서 답변온 뒤에 반영하실 예정인가요??
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginScreen.kt (1)
38-40:⚠️ Potential issue | 🟠 Major카카오 로그인 실패 시
LoginIntent.FailLogin이 디스패치되지 않습니다.
onFailure콜백에서 Timber 로그만 남기고viewModel.store.onIntent(LoginIntent.FailLogin)을 호출하지 않고 있습니다.LoginViewModel의FailLogin인텐트는ShowToastMessage("카카오 로그인에 실패했습니다.")를 발행하도록 구현되어 있으므로, 현재 상태에서는 로그인 실패 시 사용자에게 아무런 피드백이 제공되지 않습니다.🐛 수정 제안
onFailure = { message -> Timber.d("로그인 실패 $message") + viewModel.store.onIntent(LoginIntent.FailLogin) },
🧹 Nitpick comments (5)
feature/mypage/impl/src/main/java/com/neki/android/feature/mypage/impl/main/MyPageViewModel.kt (1)
191-196:onSuccess블록 내 suspend 호출의 예외 처리 부재
clearTokens()와setCompletedOnboarding(false)모두 suspend 함수로, DataStore I/O 예외가 발생할 수 있습니다.onSuccess블록 내에서 예외가 발생하면 코루틴이 조용히 실패하여 UI가 로딩 상태에 머물 수 있습니다.
clearTokens()의 경우 기존 패턴이므로 이 PR 범위를 벗어나지만, 후속 작업으로 이 블록 전체를try-catch로 감싸거나, 각 호출의 실패를 독립적으로 처리하는 것을 권장합니다.♻️ 제안: 예외 처리 추가
.onSuccess { - tokenRepository.clearTokens() - authRepository.setCompletedOnboarding(false) - reduce { copy(isLoading = false) } - postSideEffect(MyPageEffect.UnlinkWithKakao) + try { + tokenRepository.clearTokens() + authRepository.setCompletedOnboarding(false) + } catch (e: Exception) { + Timber.e(e) + } + reduce { copy(isLoading = false) } + postSideEffect(MyPageEffect.UnlinkWithKakao) }feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/component/LoginBackground.kt (2)
44-48:title24Bold스타일에fontSize = 32.sp를 오버라이드하고 있습니다.디자인 시스템의
title24Bold타이포그래피를 사용하면서fontSize를 32sp로 덮어쓰면, line-height·letter-spacing 등 나머지 속성이 24sp 기준 값으로 남아 의도치 않은 레이아웃이 될 수 있습니다. PR 설명에 Title 28 폰트가 추가되었다고 언급되어 있으니, 32sp에 맞는 타이포그래피 스타일(title32Bold등)을 디자인 시스템에 추가하거나, 이미 존재한다면 해당 스타일을 직접 사용하는 것이 좋겠습니다.♻️ 제안
Text( text = "네컷의 순간이 \n이어지는 곳", - style = NekiTheme.typography.title24Bold, - fontSize = 32.sp, + style = NekiTheme.typography.title32Bold, // 디자인 시스템에 정의 필요 color = NekiTheme.colorScheme.white, )
44-55: 하드코딩된 한국어 문자열은 string resource로 분리하는 것을 권장합니다.
"네컷의 순간이 \n이어지는 곳","위치, 포즈, 아카이빙까지"등 UI 텍스트가 코드에 직접 작성되어 있습니다. 향후 다국어 지원이나 텍스트 변경 관리를 위해strings.xml로 추출하는 것이 좋습니다.core/navigation/src/main/java/com/neki/android/core/navigation/NavigationState.kt (1)
56-58:toMutableStateList()가 매 리컴포지션마다 새 리스트를 생성합니다.
flatMap+toMutableStateList()는 리컴포지션 시마다 새SnapshotStateList인스턴스를 할당합니다. 현재 동작에 문제는 없지만, 호출 측에서 불필요한 리컴포지션을 유발할 수 있습니다. 필요 시remember/derivedStateOf로 감싸는 것을 고려해 볼 수 있습니다.feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginScreen.kt (1)
47-48:else -> {}가 처리되지 않은 사이드 이펙트를 조용히 무시합니다.
LoginViewModel이 Login과 Term 화면 간에 공유되므로, Term 전용 사이드 이펙트(NavigateBack,NavigateUrl)가 여기서 무시될 수 있습니다. 현재는 문제가 없지만, 향후 새 사이드 이펙트 추가 시 처리 누락을 발견하기 어려울 수 있습니다. 로그를 남기거나, exhaustivewhen을 사용하여 명시적으로 각 케이스를 나열하는 것을 고려해 주세요.
약관과 관련해 API 추가로 연결해야 할 부분이 생겨 이후 PR에서 함께 반영하겠습니다! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginScreen.kt`:
- Line 25: The code stores a NekiToast with remember { NekiToast(context) }
which can capture a stale Context across configuration changes; update the
remember call to use context as a key (e.g., remember(context) {
NekiToast(context) }) so a new NekiToast is created when context changes,
targeting the nekiToast variable in LoginScreen.kt.
🧹 Nitpick comments (4)
core/data/src/main/java/com/neki/android/core/data/repository/impl/TokenRepositoryImpl.kt (2)
20-23: companion object의 키를private으로 제한하세요.
ACCESS_TOKEN과REFRESH_TOKEN은 이 리포지토리의 내부 구현 세부사항입니다. 현재val로 선언되어 companion object 외부에서 접근 가능합니다. 불필요한 노출을 방지하기 위해private으로 변경하는 것이 좋습니다.♻️ 수정 제안
companion object { - val ACCESS_TOKEN = stringPreferencesKey("access_token") - val REFRESH_TOKEN = stringPreferencesKey("refresh_token") + private val ACCESS_TOKEN = stringPreferencesKey("access_token") + private val REFRESH_TOKEN = stringPreferencesKey("refresh_token") }
36-43:hasTokens()에서 불필요한 복호화 수행.토큰 존재 여부만 확인하면 되는데 매번
CryptoManager.decrypt()를 호출하고 있습니다. 암호화된 값 자체가 null이 아니고 blank가 아닌지만 확인하면 충분합니다. 복호화는 비용이 있는 연산이므로, 단순 존재 확인에서는 생략할 수 있습니다.♻️ 수정 제안
override fun hasTokens(): Flow<Boolean> { return dataStore.data.map { preferences -> - val accessToken = preferences[ACCESS_TOKEN]?.let { CryptoManager.decrypt(it) } - val refreshToken = preferences[REFRESH_TOKEN]?.let { CryptoManager.decrypt(it) } - - !accessToken.isNullOrBlank() && !refreshToken.isNullOrBlank() + !preferences[ACCESS_TOKEN].isNullOrBlank() && !preferences[REFRESH_TOKEN].isNullOrBlank() } }feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginScreen.kt (2)
30-40: 로그인 실패 시 사용자에게 피드백 없음
onFailure에서Timber.d로 로그만 남기고 있어, 사용자는 로그인 실패 원인을 알 수 없습니다. Toast나 에러 UI를 통해 실패를 알리는 것이 좋습니다.🛠️ 예시
onFailure = { message -> Timber.d("로그인 실패 $message") + nekiToast.showToast(text = message ?: "로그인에 실패했습니다.") },
46-47:else -> {}공백 분기를 제거하고 모든LoginSideEffect케이스를 명시적으로 처리
LoginSideEffect는 sealed interface이므로else분기 없이when을 사용하면 컴파일 타임에 모든 케이스를 강제로 처리하게 됩니다. 현재 코드는NavigateToMain,NavigateBack,NavigateUrl케이스를 처리하지 않으면서 공백else분기로 무시하고 있습니다. 향후 새로운 side effect가 추가되더라도 조용히 무시될 수 있으므로, 모든 케이스를 명시적으로 처리하거나 불필요한 케이스에 대해 의도적인 처리를 추가해야 합니다.
|
|
||
| val context = LocalContext.current | ||
| val kakaoAuthHelper = remember { KakaoAuthHelper(context) } | ||
| val nekiToast = remember { NekiToast(context) } |
There was a problem hiding this comment.
remember에 context 키가 누락되어 stale context 참조 위험
remember { NekiToast(context) }는 context를 키로 사용하지 않으므로, Configuration 변경(예: 화면 회전) 시 이전 context를 캡처한 NekiToast 인스턴스가 재사용됩니다. 이는 메모리 릭 또는 잘못된 Toast 표시로 이어질 수 있습니다.
🛠️ 수정 제안
- val nekiToast = remember { NekiToast(context) }
+ val nekiToast = remember(context) { NekiToast(context) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val nekiToast = remember { NekiToast(context) } | |
| val nekiToast = remember(context) { NekiToast(context) } |
🤖 Prompt for AI Agents
In
`@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginScreen.kt`
at line 25, The code stores a NekiToast with remember { NekiToast(context) }
which can capture a stale Context across configuration changes; update the
remember call to use context as a key (e.g., remember(context) {
NekiToast(context) }) so a new NekiToast is created when context changes,
targeting the nekiToast variable in LoginScreen.kt.
2 participants
🔗 관련 이슈
📙 작업 설명
📸 스크린샷 또는 시연 영상 (선택)
KakaoTalk_Video_2026-02-08-15-51-45.mp4
KakaoTalk_Video_2026-02-08-15-52-27.mp4
KakaoTalk_Video_2026-02-08-15-58-58.mp4
KakaoTalk_Video_2026-02-08-15-58-30.mp4
💬 추가 설명 or 리뷰 포인트 (선택)
AuthNavigationState,AuthNavigator,AuthScreen를 정의하고, AuthScreen 내 별도NavDisplay()를 이용했습니다.LoginViewModel을 공유하고, Splash는 스플래시 이후의 화면 전환과 이후 딥링크 처리를 위해 별도SplashViewModel을 사용합니다.HiltSharedViewModelStoreNavEntryDecorator만으로 ViewModel이clear()되지 않고, 남아있는 현상이 있습니다.ex. 로그인/약관 동의 후 메인화면에 진입했을 때 LoginViewModel이 살아있는 현상이 있습니다. 반대로 메인화면에서 로그아웃/탈퇴 후 스플래시 화면으로 이동했을 때 탑레벨의 뷰모델이 모두 살아있는 현상이 있구요.
-> 이부분은 해당 pr에서 수정하면 변경사항이 너무 많을 것 같아 별도 이슈로 등록하여 수정하겠습니다!
Q. 현재 메인화면에서의 화면 전환에 사용되는 Navigator/NavigatorImpl을 MainNavigator/MainNavigatorImpl로 네이밍을 변경할까요?
Summary by CodeRabbit
새로운 기능
디자인 개선
기타