Conversation
Closed
Walkthrough온보딩 정보 화면을 Scaffold에서 LazyColumn 기반으로 재구성하여 IME/포커스/bring-into-view, 포인터/제스처 처리, sticky 헤더 및 액션 버튼 위치를 변경했습니다. 스플래시 관련으로는 TokenManager 및 navigateToHome 경로와 자동 로그인 분기 로직을 제거했습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Screen as OnboardingInformationScreen
participant Field as TextField (Name/Age)
participant Bring as BringIntoViewRequester
participant Layout as LazyColumn
participant IME as IME/Keyboard
User->>Field: 터치/클릭
Field->>Screen: pointerInput 이벤트
Screen->>Screen: FocusRequester.requestFocus()
Screen->>Bring: bringIntoView()
Bring->>Layout: 스크롤 요청
Layout->>Layout: 대상 필드 노출
Screen->>IME: 키보드 표시 요청
IME->>Screen: WindowInsets.ime 값 제공
Screen->>Screen: animateDpAsState로 bottom inset 적용
Screen->>Layout: 하단 패딩/레이아웃 조정
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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: 1
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/cherrish/android/presentation/onboarding/information/OnboardingInformationScreen.kt`:
- Around line 127-131: The root Column in OnboardingInformationScreen.kt is
missing application of the system padding; apply paddingValues to the Column
modifier first (i.e., include .padding(paddingValues) on the Column's modifier
before .fillMaxSize()/.background()) so window insets (status/nav bars) are
respected rather than only applying bottom inset to the button; update the
Column modifier where Column(...) is declared to use .padding(paddingValues)
then chain the existing .fillMaxSize() and .background().
🧹 Nitpick comments (1)
app/src/main/java/com/cherrish/android/presentation/onboarding/information/OnboardingInformationScreen.kt (1)
122-125: IME 표시 시 버튼이 가려질 가능성 확인 필요
Line 122-125에서 IME가 뜨면 bottomInset을 0.dp로 만들고 있어 edge-to-edge 구성에서는 Line 231-240의 버튼이 키보드에 가려질 수 있습니다. 실제 설정에서 동작 확인 후 필요 시 IME 인셋을 반영하도록 조정해 주세요.⚙️ 조정 예시
- val targetBottomInset = if (imeBottomDp > 0.dp) 0.dp else paddingValues.calculateBottomPadding() + val targetBottomInset = maxOf(imeBottomDp, paddingValues.calculateBottomPadding())Also applies to: 231-240
Comment on lines
+127
to
+131
| Column( | ||
| modifier = modifier | ||
| .fillMaxSize() | ||
| .background(color = CherrishTheme.colors.gray0) | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the target file
cat -n app/src/main/java/com/cherrish/android/presentation/onboarding/information/OnboardingInformationScreen.ktRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 15266
루트 Column에 paddingValues 적용 필수
현재 코드는 루트 Column에 .padding(paddingValues)를 적용하지 않고 있습니다. 대신 paddingValues에서 하단 패딩만 추출하여 버튼에만 적용되고 있어, 상태바와 내비게이션 바의 패딩이 제대로 처리되지 않을 수 있습니다. 루트 Column 수정자에 .padding(paddingValues)를 먼저 적용하세요.
✅ 적용 예시
Column(
modifier = modifier
.fillMaxSize()
.background(color = CherrishTheme.colors.gray0)
+ .padding(paddingValues)
) {📝 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
| Column( | |
| modifier = modifier | |
| .fillMaxSize() | |
| .background(color = CherrishTheme.colors.gray0) | |
| ) { | |
| Column( | |
| modifier = modifier | |
| .fillMaxSize() | |
| .background(color = CherrishTheme.colors.gray0) | |
| .padding(paddingValues) | |
| ) { |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/presentation/onboarding/information/OnboardingInformationScreen.kt`
around lines 127 - 131, The root Column in OnboardingInformationScreen.kt is
missing application of the system padding; apply paddingValues to the Column
modifier first (i.e., include .padding(paddingValues) on the Column's modifier
before .fillMaxSize()/.background()) so window insets (status/nav bars) are
respected rather than only applying bottom inset to the button; update the
Column modifier where Column(...) is declared to use .padding(paddingValues)
then chain the existing .fillMaxSize() and .background().
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/cherrish/android/presentation/onboarding/information/OnboardingInformationViewModel.kt`:
- Around line 17-22: The onboarding response contains a user id that must be
persisted to TokenManager but the ViewModel currently never saves it; in
OnboardingInformationViewModel update the success handler inside onNextClicked()
(where onboardingProfileRepository is used) to call
tokenManager.saveId(response.id) after a successful response so AuthInterceptor
can later retrieve the id via tokenManager.getId(); ensure TokenManager is
injected/available in the ViewModel and invoke saveId with the response.id in
the success branch.
🧹 Nitpick comments (1)
app/src/main/java/com/cherrish/android/presentation/splash/SplashScreen.kt (1)
40-47:LaunchedEffect내부에서scope.launch중첩은 불필요합니다.
LaunchedEffect는 이미 코루틴 스코프를 제공하므로 내부에서delay와 함수 호출을 직접 수행할 수 있습니다.rememberCoroutineScope()는 이 경우 필요하지 않으며 제거 가능합니다.♻️ 권장 수정안
- val scope = rememberCoroutineScope() - - LaunchedEffect (Unit) { - scope.launch { - delay(3000) - viewModel.navigateToOnboarding() - } + LaunchedEffect(Unit) { + delay(3000) + viewModel.navigateToOnboarding() }
Comment on lines
17
to
22
| import javax.inject.Inject | ||
|
|
||
| @HiltViewModel | ||
| class OnboardingInformationViewModel @Inject constructor( | ||
| private val onboardingProfileRepository: OnboardingProfileRepository, | ||
| private val tokenManager: TokenManager | ||
| private val onboardingProfileRepository: OnboardingProfileRepository | ||
| ) : ViewModel() { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for TokenManager usage and id-related methods
echo "=== TokenManager usage ==="
rg -n --type=kt '\bTokenManager\b' -C 2
echo -e "\n=== getId/saveId/userId patterns ==="
rg -n --type=kt '\bgetId\b|\bsaveId\b|\buserId\b' -C 2
echo -e "\n=== OnboardingInformationViewModel file ==="
cat -n app/src/main/java/com/cherrish/android/presentation/onboarding/information/OnboardingInformationViewModel.kt | head -80Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 2850
🏁 Script executed:
#!/bin/bash
# Broader search for TokenManager
echo "=== TokenManager literal search ==="
rg -n 'TokenManager' -C 2
echo -e "\n=== getId/saveId patterns ==="
rg -n 'getId|saveId' -C 2
echo -e "\n=== usesId or userId patterns ==="
rg -n 'userId|user.*[Ii]d' -C 2 | head -100
echo -e "\n=== Check all kotlin files in onboarding package ==="
find app/src/main/java/com/cherrish/android/presentation/onboarding -name "*.kt" -type f
echo -e "\n=== Check OnboardingProfileRepository for id handling ==="
find app/src/main -name "*OnboardingProfileRepository*" -type fRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 18725
🏁 Script executed:
#!/bin/bash
echo "=== OnboardingProfileRepositoryImpl ==="
cat app/src/main/java/com/cherrish/android/data/repositoryimpl/OnboardingProfileRepositoryImpl.kt
echo -e "\n=== Check response DTO ==="
find app/src/main -name "*OnboardingProfile*ResponseDto*" -o -name "*OnboardingProfile*Response*" | head -20
fd OnboardingProfile app/src/main --type f -name "*.kt" | grep -i response
echo -e "\n=== Look for id in onboarding responses ==="
find app/src/main/java/com/cherrish/android/data/remote/dto -name "*.kt" | xargs grep -l "onboarding\|Onboarding" -iRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 1807
🏁 Script executed:
#!/bin/bash
echo "=== OnboardingProfileResponseDto ==="
cat app/src/main/java/com/cherrish/android/data/remote/dto/response/OnboardingProfileResponseDto.kt
echo -e "\n=== OnboardingProfileResponseModel ==="
cat app/src/main/java/com/cherrish/android/data/model/OnboardingProfileResponseModel.kt
echo -e "\n=== Check previous version in git history ==="
git log --oneline -n 20 -- app/src/main/java/com/cherrish/android/presentation/onboarding/information/OnboardingInformationViewModel.ktRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 927
🏁 Script executed:
#!/bin/bash
echo "=== Check git diff for the onboarding file ==="
git show 5c302cc -- app/src/main/java/com/cherrish/android/presentation/onboarding/information/OnboardingInformationViewModel.kt | head -150
echo -e "\n=== Also check if there are other places where saveId is called ==="
rg 'saveId' -C 3Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 4342
onboardingProfileRepository 응답에서 받은 사용자 id를 TokenManager에 저장해야 합니다.
응답 DTO에서 id를 수신하지만, 현재 코드에서는 이를 저장하지 않습니다. AuthInterceptor가 모든 API 요청에서 tokenManager.getId()를 호출하므로, id가 저장되지 않으면 이후 네트워크 요청이 실패하거나 사용자 정보 없이 진행됩니다.
onNextClicked()의 성공 핸들러에서 tokenManager.saveId(response.id)를 호출하세요.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/presentation/onboarding/information/OnboardingInformationViewModel.kt`
around lines 17 - 22, The onboarding response contains a user id that must be
persisted to TokenManager but the ViewModel currently never saves it; in
OnboardingInformationViewModel update the success handler inside onNextClicked()
(where onboardingProfileRepository is used) to call
tokenManager.saveId(response.id) after a successful response so AuthInterceptor
can later retrieve the id via tokenManager.getId(); ensure TokenManager is
injected/available in the ViewModel and invoke saveId with the response.id in
the success branch.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/cherrish/android/presentation/splash/SplashScreen.kt`:
- Around line 40-47: Remove the unnecessary rememberCoroutineScope() and the
internal scope.launch inside the LaunchedEffect in SplashScreen; LaunchedEffect
already provides a coroutine scope and cancellation, so call delay(3000)
directly inside LaunchedEffect and then invoke viewModel.navigateToOnboarding(),
and delete references to scope and rememberCoroutineScope to ensure proper
cancellation when the composable leaves composition.
Comment on lines
40
to
47
| val scope = rememberCoroutineScope() | ||
|
|
||
| LifecycleEventEffect(Lifecycle.Event.ON_START) { | ||
| LaunchedEffect(Unit) { | ||
| scope.launch { | ||
| delay(3000) | ||
| viewModel.isAutoLoginCheck() | ||
| viewModel.navigateToOnboarding() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
LaunchedEffect 내부의 불필요한 scope.launch 제거 필요
LaunchedEffect는 이미 코루틴 스코프를 제공하므로, 내부에서 rememberCoroutineScope()를 통한 scope.launch는 불필요합니다. 또한 이 패턴은 컴포저블이 컴포지션을 벗어날 때 delay가 제대로 취소되지 않을 수 있습니다.
♻️ 수정 제안
- val scope = rememberCoroutineScope()
-
- LaunchedEffect(Unit) {
- scope.launch {
- delay(3000)
- viewModel.navigateToOnboarding()
- }
- }
+ LaunchedEffect(Unit) {
+ delay(3000)
+ viewModel.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 scope = rememberCoroutineScope() | |
| LifecycleEventEffect(Lifecycle.Event.ON_START) { | |
| LaunchedEffect(Unit) { | |
| scope.launch { | |
| delay(3000) | |
| viewModel.isAutoLoginCheck() | |
| viewModel.navigateToOnboarding() | |
| } | |
| } | |
| LaunchedEffect(Unit) { | |
| delay(3000) | |
| viewModel.navigateToOnboarding() | |
| } |
🤖 Prompt for AI Agents
In `@app/src/main/java/com/cherrish/android/presentation/splash/SplashScreen.kt`
around lines 40 - 47, Remove the unnecessary rememberCoroutineScope() and the
internal scope.launch inside the LaunchedEffect in SplashScreen; LaunchedEffect
already provides a coroutine scope and cancellation, so call delay(3000)
directly inside LaunchedEffect and then invoke viewModel.navigateToOnboarding(),
and delete references to scope and rememberCoroutineScope to ensure proper
cancellation when the composable leaves composition.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 📸
Uncompleted Tasks 😅
N/A
To Reviewers 📢
한 번 봐주세요오 ,, 마지막이길
Summary by CodeRabbit
버그 수정
새로운 기능
리팩터/동작 변경
✏️ Tip: You can customize this high-level summary in your review settings.