[Feat] Splash 화면 컴포즈 마이그레이션 #36
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough개요스플래시 화면을 Compose 기반 UI 상태 흐름으로 마이그레이션했습니다. 기존의 명령형 옵저버 기반 네비게이션을 Kotlin Flow와 StateFlow를 활용한 선언형 상태 관리로 전환하고, 새로운 대화상자 컴포넌트를 추가하며 테마 설정을 업데이트했습니다. 변경 사항
예상 코드 리뷰 노력🎯 4 (복잡함) | ⏱️ ~50분 연관 이슈
연관 PR
제안 검토자
시
🚥 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. 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 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: 2
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/daedan/festabook/presentation/splash/SplashViewModel.kt`:
- Around line 36-44: checkFestivalId currently calls
festivalLocalDataSource.getFestivalId() on the main thread which can block UI;
move the SharedPreferences read onto an IO dispatcher: wrap the call in a
coroutine (e.g., viewModelScope.launch(Dispatchers.IO) or using
withContext(Dispatchers.IO)) inside SplashViewModel.checkFestivalId, read
festivalId on IO, then post the result back to the main thread before updating
_uiState (ensure you set _uiState.value = SplashUiState.NavigateToExplore or
SplashUiState.NavigateToMain(festivalId) from the main thread after the IO
read).
In `@app/src/main/res/values/strings.xml`:
- Around line 151-154: The string update_notice_message contains "\n " (a
newline followed by a space) which can create an unintended indent; update the
value for update_notice_message to remove the space after the newline so it uses
"\n" (or replace the newline+space with an explicit non-breaking space or
desired spacing) to ensure no accidental leading whitespace is rendered.
🧹 Nitpick comments (4)
app/src/main/java/com/daedan/festabook/presentation/splash/SplashActivity.kt (2)
26-34: 업데이트 취소/실패 분기 UX를 한 번 더 확인해 주세요
RESULT_OK 외의 모든 케이스가 실패로 처리되어 사용자가 업데이트를 취소해도 네트워크 오류 다이얼로그로 보일 수 있습니다. 의도된 동작인지 확인하고 필요 시 분기 처리를 고려해 주세요.
51-64: 업데이트 체크 지연 시 스플래시가 무기한 유지될 수 있습니다
Loading 동안 OS 스플래시를 유지하고 있는데, getIsAppUpdateAvailable()가 지연/정지되면 화면이 계속 멈춰 있을 수 있어요. 호출에 타임아웃/실패 처리가 있는지 확인해 주세요.⏱️ 예시: 타임아웃 추가
+import kotlinx.coroutines.withTimeoutOrNull ... - LaunchedEffect(Unit) { - // 앱 실행 시 즉시 앱 버전 업데이트의 필요 유무 확인 - val result = appVersionManager.getIsAppUpdateAvailable() - viewModel.handleVersionCheckResult(result) - } + LaunchedEffect(Unit) { + // 앱 실행 시 즉시 앱 버전 업데이트의 필요 유무 확인 + val result = withTimeoutOrNull(3_000) { + appVersionManager.getIsAppUpdateAvailable() + } + viewModel.handleVersionCheckResult( + result ?: Result.failure(Exception("Update check timeout")), + ) + }app/src/main/java/com/daedan/festabook/presentation/splash/component/SplashDialogs.kt (1)
54-115: Dialog 비닫힘 의도를 명시하면 더 명확합니다
현재 onDismissRequest를 빈 람다로 두어 닫힘을 막고 있는데, DialogProperties로 명시하면 의도가 더 분명해집니다.🧩 제안 수정안
+import androidx.compose.ui.window.DialogProperties ... - Dialog(onDismissRequest = {}) { + Dialog( + onDismissRequest = {}, + properties = DialogProperties( + dismissOnBackPress = false, + dismissOnClickOutside = false, + ), + ) {app/src/test/java/com/daedan/festabook/splash/SplashViewModelTest.kt (1)
43-94: 비동기 상태 갱신 시 테스트가 플러시를 필요로 할 수 있어요
handleVersionCheckResult가 viewModelScope에서 갱신된다면 assertion 전에 dispatcher를 advanceUntilIdle/runCurrent로 플러시해야 간헐 실패를 피할 수 있습니다. 또한 FestivalLocalDataSource.getFestivalId가 suspend 함수라면 coEvery 사용이 필요합니다. 시그니처를 확인해 주세요.🧪 예시: dispatcher 플러시
- splashViewModel.handleVersionCheckResult(updateResult) + splashViewModel.handleVersionCheckResult(updateResult) + testDispatcher.scheduler.advanceUntilIdle()
| private fun checkFestivalId() { | ||
| val festivalId = festivalLocalDataSource.getFestivalId() | ||
| Timber.d("festival ID : $festivalId") | ||
| Timber.d("현재 접속중인 festival ID : $festivalId") | ||
|
|
||
| if (festivalId == null) { | ||
| _navigationState.setValue(NavigationState.NavigateToExplore) | ||
| _uiState.value = SplashUiState.NavigateToExplore | ||
| } else { | ||
| _navigationState.setValue(NavigationState.NavigateToMain(festivalId)) | ||
| _uiState.value = SplashUiState.NavigateToMain(festivalId) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# FestivalLocalDataSource와 getFestivalId 구현 확인
rg -n --type=kt -C3 'class\s+FestivalLocalDataSource|interface\s+FestivalLocalDataSource'Repository: festabook/android
Length of output: 87
🏁 Script executed:
#!/bin/bash
# getFestivalId 메서드 구현 확인
rg -n --type=kt -C5 'fun\s+getFestivalId\s*\('Repository: festabook/android
Length of output: 87
🏁 Script executed:
#!/bin/bash
# FestivalLocalDataSource 파일 위치 찾기
fd -t f FestivalLocalDataSourceRepository: festabook/android
Length of output: 240
🏁 Script executed:
cat -n app/src/main/java/com/daedan/festabook/data/datasource/local/FestivalLocalDataSource.ktRepository: festabook/android
Length of output: 322
🏁 Script executed:
cat -n app/src/main/java/com/daedan/festabook/data/datasource/local/FestivalLocalDataSourceImpl.ktRepository: festabook/android
Length of output: 1647
🏁 Script executed:
cat -n app/src/main/java/com/daedan/festabook/presentation/splash/SplashViewModel.ktRepository: festabook/android
Length of output: 1994
SharedPreferences I/O로 인한 메인 스레드 블로킹 위험을 반드시 해결하세요.
getFestivalId()는 SharedPreferences의 동기 연산(getLong())을 사용하므로 메인 스레드를 블로킹합니다. 특히 첫 실행 시 디스크에서 읽기가 발생할 수 있어 스플래시 화면에서 UI 지연이 발생할 수 있습니다. 아래와 같이 IO 디스패처로 이동해 주세요.
🔧 수정 예시
+import androidx.lifecycle.viewModelScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
...
- private fun checkFestivalId() {
- val festivalId = festivalLocalDataSource.getFestivalId()
- Timber.d("현재 접속중인 festival ID : $festivalId")
-
- if (festivalId == null) {
- _uiState.value = SplashUiState.NavigateToExplore
- } else {
- _uiState.value = SplashUiState.NavigateToMain(festivalId)
- }
- }
+ private fun checkFestivalId() {
+ viewModelScope.launch(Dispatchers.IO) {
+ val festivalId = festivalLocalDataSource.getFestivalId()
+ Timber.d("현재 접속중인 festival ID : $festivalId")
+ _uiState.value =
+ if (festivalId == null) SplashUiState.NavigateToExplore
+ else SplashUiState.NavigateToMain(festivalId)
+ }
+ }🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/splash/SplashViewModel.kt`
around lines 36 - 44, checkFestivalId currently calls
festivalLocalDataSource.getFestivalId() on the main thread which can block UI;
move the SharedPreferences read onto an IO dispatcher: wrap the call in a
coroutine (e.g., viewModelScope.launch(Dispatchers.IO) or using
withContext(Dispatchers.IO)) inside SplashViewModel.checkFestivalId, read
festivalId on IO, then post the result back to the main thread before updating
_uiState (ensure you set _uiState.value = SplashUiState.NavigateToExplore or
SplashUiState.NavigateToMain(festivalId) from the main thread after the IO
read).
There was a problem hiding this comment.
@parkjiminnnn 감사합니다! 😊 스플래시 화면에서 UI 블로킹이 발생하지 않도록 SharedPreferences 읽기를 IO 디스패처로 옮기시면 더 부드러운 사용자 경험을 제공하실 수 있을 거예요.
| <string name="update_notice_message">새로운 버전이 출시되었어요.\n 더 나은 사용을 위해 앱을 업데이트해 주세요.</string> | ||
| <string name="update_notice_confirm">업데이트</string> | ||
| <string name="update_failed_title">서버 통신 실패</string> | ||
| <string name="update_failed_message">앱 서버에 접속할 수 없습니다. 다시 시도해주세요</string> | ||
| <string name="update_failed_message">앱 서버에 접속할 수 없습니다. 다시 시도해주세요.</string> |
There was a problem hiding this comment.
줄바꿈 뒤 선행 공백으로 들여쓰기 발생 가능
update_notice_message의 \n 뒤 공백이 의도치 않은 들여쓰기를 만들 수 있어요. 의도한 여백이 아니라면 공백을 제거해 주세요.
✂️ 제안 수정안
- <string name="update_notice_message">새로운 버전이 출시되었어요.\n 더 나은 사용을 위해 앱을 업데이트해 주세요.</string>
+ <string name="update_notice_message">새로운 버전이 출시되었어요.\n더 나은 사용을 위해 앱을 업데이트해 주세요.</string>📝 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.
| <string name="update_notice_message">새로운 버전이 출시되었어요.\n 더 나은 사용을 위해 앱을 업데이트해 주세요.</string> | |
| <string name="update_notice_confirm">업데이트</string> | |
| <string name="update_failed_title">서버 통신 실패</string> | |
| <string name="update_failed_message">앱 서버에 접속할 수 없습니다. 다시 시도해주세요</string> | |
| <string name="update_failed_message">앱 서버에 접속할 수 없습니다. 다시 시도해주세요.</string> | |
| <string name="update_notice_message">새로운 버전이 출시되었어요.\n더 나은 사용을 위해 앱을 업데이트해 주세요.</string> | |
| <string name="update_notice_confirm">업데이트</string> | |
| <string name="update_failed_title">서버 통신 실패</string> | |
| <string name="update_failed_message">앱 서버에 접속할 수 없습니다. 다시 시도해주세요.</string> |
🤖 Prompt for AI Agents
In `@app/src/main/res/values/strings.xml` around lines 151 - 154, The string
update_notice_message contains "\n " (a newline followed by a space) which can
create an unintended indent; update the value for update_notice_message to
remove the space after the newline so it uses "\n" (or replace the newline+space
with an explicit non-breaking space or desired spacing) to ensure no accidental
leading whitespace is rendered.
parkjiminnnn
left a comment
parkjiminnnn
left a comment
There was a problem hiding this comment.
고생하셧씁니다 타마
코드를 보니 신경을 많이 써주신 것 같네요!
코멘트 몇개만 확인해주세용
| private fun checkFestivalId() { | ||
| val festivalId = festivalLocalDataSource.getFestivalId() | ||
| Timber.d("festival ID : $festivalId") | ||
| Timber.d("현재 접속중인 festival ID : $festivalId") | ||
|
|
||
| if (festivalId == null) { | ||
| _navigationState.setValue(NavigationState.NavigateToExplore) | ||
| _uiState.value = SplashUiState.NavigateToExplore | ||
| } else { | ||
| _navigationState.setValue(NavigationState.NavigateToMain(festivalId)) | ||
| _uiState.value = SplashUiState.NavigateToMain(festivalId) | ||
| } |
| @DrawableRes iconResId: Int? = null, | ||
| onConfirm: () -> Unit, | ||
| ) { | ||
| Dialog(onDismissRequest = {}) { |
There was a problem hiding this comment.
DialogProperties(
dismissOnBackPress = false,
dismissOnClickOutside = false
)다음 코드를 추가해서 안전하게 다이어로그가 안닫히게 하는건 어떠신가용?
There was a problem hiding this comment.
그 부분은 생각하지 못했네요! 바로 적용했습니다~
feat(Splash): 스플래시 다이얼로그 외부 클릭 및 뒤로가기 방지
#️⃣ 이슈 번호
🛠️ 작업 내용
core-splashscreen API와 Compose 기반으로 마이그레이션했습니다.SplashUiState를 추가하여SplashViewModel에서 상태를 관리하도록 했습니다.LaunchedEffect를 통해 상태 변화를 감지하고, 저장된 축제 ID 유무에 따라 메인 화면 또는 탐색 화면으로의 자동 전환을 처리했습니다.🙇🏻 중점 리뷰 요청
📸 이미지 첨부 (Optional)
Summary by CodeRabbit
릴리스 노트
새 기능
개선 사항
✏️ Tip: You can customize this high-level summary in your review settings.