[FEAT/#141] Lottie 파일 적용 #142
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- ChallengeLoadingScreen - LoadingScreen
Walkthrough스플래시 및 챌린지 로딩 화면의 정적 이미지를 Lottie 애니메이션으로 교체하고, 네비게이션 사이드이펙트와 라우트/뷰모델/컴포저블 시그니처에 routineName을 전파하도록 관련 파일들을 업데이트했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant App as AppStart
participant Splash as SplashScreen
participant VM as SplashViewModel
participant Nav as Navigator
App->>Splash: 스플래시 화면 표시 (Lottie 재생 시작)
Splash->>VM: 자동 로그인/사이드이펙트 트리거
VM-->>Splash: emit NavigateToOnboarding
Splash->>Nav: 네비게이션 요청 전달
Nav-->>App: 온보딩 화면으로 전환
sequenceDiagram
participant RoutineScreen as RoutineScreen
participant RoutineVM as RoutineViewModel
participant SideEffect as ChallengeSideEffect
participant Nav as Navigator
participant Loading as ChallengeLoadingScreen
RoutineScreen->>RoutineVM: 사용자 루틴 선택 & 다음 클릭
RoutineVM-->>SideEffect: Emit NavigateToChallengeLoading(routineId, routineName)
RoutineScreen->>Nav: navigateToChallengeLoading(routineId, routineName)
Nav->>Loading: 로딩 화면(route args: routineId, routineName) 표시 (Lottie 재생)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/cherrish/android/presentation/splash/SplashScreen.kt`:
- Around line 44-49: The code uses
LifecycleEventEffect(Lifecycle.Event.ON_START) with rememberCoroutineScope and
scope.launch+delay(3000) which can re-run when the app returns to foreground and
may continue after the Composable is disposed; replace this with
LaunchedEffect(Unit) and move the delay(3000) and viewModel.isAutoLoginCheck()
into the LaunchedEffect so it runs only once on initial composition and is
automatically cancelled when the Composable is disposed; specifically replace
the block containing LifecycleEventEffect, Lifecycle.Event.ON_START,
rememberCoroutineScope/ scope.launch and delay with a LaunchedEffect(Unit) {
delay(3000); viewModel.isAutoLoginCheck() } to fix duplicate navigation and
cancellation issues.
In
`@app/src/main/java/com/cherrish/android/presentation/splash/SplashViewModel.kt`:
- Around line 22-30: The code calls tokenManager.getId() into a now-unused
variable id and leaves a commented-out auto-login branch before always emitting
SplashSideEffect.NavigateToOnboarding; either remove the dead code or restore
the branch: delete the commented if/else and the unused id variable (or
re-enable the conditional that emits SplashSideEffect.NavigateToHome when id !=
null) in SplashViewModel where _sideEffect.emit is called so there is no
leftover commented logic or unused tokenManager.getId() call.
Comment on lines
+44
to
49
| LifecycleEventEffect(Lifecycle.Event.ON_START) { | ||
| scope.launch { | ||
| delay(3000) | ||
| viewModel.isAutoLoginCheck() | ||
| } | ||
| } |
There was a problem hiding this comment.
ON_START 이벤트 사용 시 중복 실행 가능성이 있습니다.
Lifecycle.Event.ON_START는 앱이 백그라운드에서 포그라운드로 돌아올 때마다 다시 트리거됩니다. 사용자가 스플래시 화면에서 다른 앱으로 전환했다가 돌아오면 3초 딜레이 후 네비게이션이 다시 시도될 수 있습니다.
또한 rememberCoroutineScope로 시작된 코루틴은 Composable이 dispose되어도 자동으로 취소되지 않을 수 있어, 화면 전환 후에도 네비게이션 로직이 실행될 위험이 있습니다.
🔧 LaunchedEffect 사용 권장
- val scope = rememberCoroutineScope()
-
- LifecycleEventEffect(Lifecycle.Event.ON_START) {
- scope.launch {
- delay(3000)
- viewModel.isAutoLoginCheck()
- }
- }
+ LaunchedEffect(Unit) {
+ delay(3000)
+ viewModel.isAutoLoginCheck()
+ }LaunchedEffect(Unit)은 최초 composition 시 한 번만 실행되며, recomposition이나 lifecycle 변경에 영향받지 않습니다. 또한 Composable이 dispose될 때 자동으로 취소됩니다.
🤖 Prompt for AI Agents
In `@app/src/main/java/com/cherrish/android/presentation/splash/SplashScreen.kt`
around lines 44 - 49, The code uses
LifecycleEventEffect(Lifecycle.Event.ON_START) with rememberCoroutineScope and
scope.launch+delay(3000) which can re-run when the app returns to foreground and
may continue after the Composable is disposed; replace this with
LaunchedEffect(Unit) and move the delay(3000) and viewModel.isAutoLoginCheck()
into the LaunchedEffect so it runs only once on initial composition and is
automatically cancelled when the Composable is disposed; specifically replace
the block containing LifecycleEventEffect, Lifecycle.Event.ON_START,
rememberCoroutineScope/ scope.launch and delay with a LaunchedEffect(Unit) {
delay(3000); viewModel.isAutoLoginCheck() } to fix duplicate navigation and
cancellation issues.
Comment on lines
22
to
30
| val id = tokenManager.getId() | ||
| _sideEffect.emit( | ||
| if (id != null) { | ||
| SplashSideEffect.NavigateToHome | ||
| } else { | ||
| SplashSideEffect.NavigateToOnboarding | ||
| } | ||
| // if (id != null) { | ||
| // SplashSideEffect.NavigateToHome | ||
| // } else { | ||
| // SplashSideEffect.NavigateToOnboarding | ||
| // } | ||
|
|
||
| SplashSideEffect.NavigateToOnboarding |
There was a problem hiding this comment.
주석 처리된 코드와 미사용 변수가 있습니다.
tokenManager.getId() 호출 결과(id)가 더 이상 사용되지 않으며, 자동 로그인 분기 로직이 주석 처리되어 항상 NavigateToOnboarding만 emit됩니다.
이 PR의 목적은 Lottie 애니메이션 적용인데, 자동 로그인 로직 변경은 범위를 벗어난 것으로 보입니다. 의도된 변경이라면 주석 처리된 코드를 완전히 제거하고 미사용 id 변수도 삭제해 주세요. 임시 디버깅용이라면 별도 이슈로 관리하는 것이 좋겠습니다.
🔧 주석 및 미사용 코드 제거 제안
fun isAutoLoginCheck() {
viewModelScope.launch {
- val id = tokenManager.getId()
_sideEffect.emit(
-// if (id != null) {
-// SplashSideEffect.NavigateToHome
-// } else {
-// SplashSideEffect.NavigateToOnboarding
-// }
-
SplashSideEffect.NavigateToOnboarding
)
}
}또는 자동 로그인 로직이 필요하다면:
fun isAutoLoginCheck() {
viewModelScope.launch {
val id = tokenManager.getId()
_sideEffect.emit(
-// if (id != null) {
-// SplashSideEffect.NavigateToHome
-// } else {
-// SplashSideEffect.NavigateToOnboarding
-// }
-
- SplashSideEffect.NavigateToOnboarding
+ if (id != null) {
+ SplashSideEffect.NavigateToHome
+ } else {
+ SplashSideEffect.NavigateToOnboarding
+ }
)
}
}📝 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.
Suggested change
| val id = tokenManager.getId() | |
| _sideEffect.emit( | |
| if (id != null) { | |
| SplashSideEffect.NavigateToHome | |
| } else { | |
| SplashSideEffect.NavigateToOnboarding | |
| } | |
| // if (id != null) { | |
| // SplashSideEffect.NavigateToHome | |
| // } else { | |
| // SplashSideEffect.NavigateToOnboarding | |
| // } | |
| SplashSideEffect.NavigateToOnboarding | |
| _sideEffect.emit( | |
| SplashSideEffect.NavigateToOnboarding | |
| ) |
Suggested change
| val id = tokenManager.getId() | |
| _sideEffect.emit( | |
| if (id != null) { | |
| SplashSideEffect.NavigateToHome | |
| } else { | |
| SplashSideEffect.NavigateToOnboarding | |
| } | |
| // if (id != null) { | |
| // SplashSideEffect.NavigateToHome | |
| // } else { | |
| // SplashSideEffect.NavigateToOnboarding | |
| // } | |
| SplashSideEffect.NavigateToOnboarding | |
| val id = tokenManager.getId() | |
| _sideEffect.emit( | |
| if (id != null) { | |
| SplashSideEffect.NavigateToHome | |
| } else { | |
| SplashSideEffect.NavigateToOnboarding | |
| } | |
| ) |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/presentation/splash/SplashViewModel.kt`
around lines 22 - 30, The code calls tokenManager.getId() into a now-unused
variable id and leaves a commented-out auto-login branch before always emitting
SplashSideEffect.NavigateToOnboarding; either remove the dead code or restore
the branch: delete the commented if/else and the unused id variable (or
re-enable the conditional that emits SplashSideEffect.NavigateToHome when id !=
null) in SplashViewModel where _sideEffect.emit is called so there is no
leftover commented logic or unused tokenManager.getId() call.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issue 🛠
Work Description ✏️
Screenshot 📸
Screen_Recording_20260122_213245_Cherrish.mp4
Uncompleted Tasks 😅
N/A
To Reviewers 📢
로띠 파일을 적용하면서 디쌤에게 옆에서 사이즈를 계속 물어봤는데 QA로 보면서 진행하시는게 좋을 것 같다고 하셨어요 ,, !!
Summary by CodeRabbit
새로운 기능
시각적 개선
UI 텍스트
리팩터
✏️ Tip: You can customize this high-level summary in your review settings.