Conversation
- 검색 결과 목록을 `ExploreSearchResultList` 컴포저블로 분리했습니다. - 검색어 유무에 따라 검색창과 로고의 위치가 애니메이션과 함께 동적으로 변경되도록 수정했습니다. - 검색 실행 시 키보드가 자동으로 숨겨지도록 구현했습니다. - 화면 전체적인 레이아웃과 패딩을 조정하여 UI를 개선했습니다.
|
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
✨ Finishing touches🧪 Generate unit tests (beta)
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: 3
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreBackHeader.kt`:
- Around line 34-37: The Icon composable in ExploreBackHeader.kt uses a
hardcoded contentDescription "뒤로가기"; replace it with a localized string by
adding <string name="explore_back">뒤로가기</string> to strings.xml and calling
stringResource(R.string.explore_back) for the Icon's contentDescription (locate
the Icon call using ImageVector.vectorResource and R.drawable.ic_arrow_back).
In
`@app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreSearchBar.kt`:
- Around line 100-103: The KeyboardActions onSearch currently only hides the
keyboard (keyboardController?.hide()) and never triggers the component's search
callback; update the onSearch handler in ExploreSearchBar.kt (the
KeyboardActions onSearch block) to first hide the keyboard and then invoke the
component's search submission function (e.g., call the existing
onSearch/onSearchSubmit/submitSearch lambda or method that performs the search,
passing the current query if required) so the IME search action actually runs
the search logic; also ensure the KeyboardOptions imeAction is set to Search if
not already.
In
`@app/src/main/java/com/daedan/festabook/presentation/explore/ExploreViewModel.kt`:
- Around line 44-50: checkFestivalId() performs synchronous SharedPreferences
I/O by calling exploreRepository.getFestivalId() on the main thread; wrap the
call in viewModelScope.launch(iODispatcher) (same pattern as SplashViewModel) so
getFestivalId() runs on the IO dispatcher and then update _uiState (via
_uiState.update { ... }) from that coroutine, avoiding blocking the main thread.
🧹 Nitpick comments (3)
app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreScreen.kt (2)
81-114:modifier파라미터가 사용되지 않고 있습니다.
modifier파라미터가 선언되었지만Scaffold에 적용되지 않아 외부에서 전달한 modifier가 무시됩니다.♻️ 수정 제안
`@Composable` fun ExploreSearchScreen( query: String, searchState: SearchUiState, onQueryChange: (String) -> Unit, onUniversitySelect: (SearchResultUiModel) -> Unit, onBackClick: () -> Unit, modifier: Modifier = Modifier, ) { Scaffold( + modifier = modifier, containerColor = Color.White, topBar = {
116-134:modifier파라미터가 사용되지 않고 있습니다.
ExploreSearchScreen과 동일하게modifier파라미터가Scaffold에 적용되어야 합니다.♻️ 수정 제안
`@Composable` fun ExploreLandingScreen( query: String, searchState: SearchUiState, onQueryChange: (String) -> Unit, onUniversitySelect: (SearchResultUiModel) -> Unit, modifier: Modifier = Modifier, ) { // ... Scaffold( + modifier = modifier, containerColor = Color.White, ) { innerPadding ->app/src/main/java/com/daedan/festabook/presentation/explore/ExploreViewModel.kt (1)
84-89:viewModelScope.launch가 불필요합니다.
tryEmit은 non-suspending 함수이므로 코루틴 내에서 실행할 필요가 없습니다.♻️ 수정 제안
fun onUniversitySelected(university: SearchResultUiModel) { exploreRepository.saveFestivalId(university.festivalId) - viewModelScope.launch { - _sideEffect.tryEmit(ExploreSideEffect.NavigateToMain(university)) - } + _sideEffect.tryEmit(ExploreSideEffect.NavigateToMain(university)) }
| Icon( | ||
| imageVector = ImageVector.vectorResource(id = R.drawable.ic_arrow_back), | ||
| contentDescription = "뒤로가기", | ||
| tint = Color.Unspecified, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the ExploreBackHeader.kt file
find . -name "ExploreBackHeader.kt" -type fRepository: festabook/android
Length of output: 153
🏁 Script executed:
# Check if strings.xml exists and look for existing localization entries
find . -name "strings.xml" -type f | head -20Repository: festabook/android
Length of output: 98
🏁 Script executed:
# Look for any existing "explore_back" or similar entries in strings.xml files
rg "explore_back|뒤로가기" --type xml -B 2 -A 2Repository: festabook/android
Length of output: 454
🏁 Script executed:
# Read the ExploreBackHeader.kt file to verify the hardcoded string
cat -n ./app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreBackHeader.kt | head -50Repository: festabook/android
Length of output: 1862
🏁 Script executed:
# Check if stringResource is already imported or available in the file
rg "import.*stringResource|stringResource" ./app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreBackHeader.ktRepository: festabook/android
Length of output: 43
🏁 Script executed:
# Check the current imports in ExploreBackHeader.kt
head -30 ./app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreBackHeader.ktRepository: festabook/android
Length of output: 1067
🏁 Script executed:
# Check strings.xml to see the exact structure and what entries exist
cat -n ./app/src/main/res/values/strings.xml | head -100Repository: festabook/android
Length of output: 5157
하드코딩된 문자열을 stringResource로 현지화하세요.
현재 "뒤로가기"가 하드코딩되어 있어 다국어 대응이 불가능합니다. stringResource를 사용하여 리소스로 관리해야 합니다.
🔧 제안 수정
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.vector.ImageVector
import androidx.compose.ui.res.vectorResource
+import androidx.compose.ui.res.stringResource
@@
Icon(
imageVector = ImageVector.vectorResource(id = R.drawable.ic_arrow_back),
- contentDescription = "뒤로가기",
+ contentDescription = stringResource(R.string.explore_back),
tint = Color.Unspecified,
)app/src/main/res/values/strings.xml의 "탐색 화면" 섹션에 다음 항목을 추가하세요:
<string name="explore_back">뒤로가기</string>🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreBackHeader.kt`
around lines 34 - 37, The Icon composable in ExploreBackHeader.kt uses a
hardcoded contentDescription "뒤로가기"; replace it with a localized string by
adding <string name="explore_back">뒤로가기</string> to strings.xml and calling
stringResource(R.string.explore_back) for the Icon's contentDescription (locate
the Icon call using ImageVector.vectorResource and R.drawable.ic_arrow_back).
| KeyboardActions( | ||
| onSearch = { | ||
| keyboardController?.hide() | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ExploreSearchBar.kt" -type fRepository: festabook/android
Length of output: 152
🏁 Script executed:
head -150 app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreSearchBar.ktRepository: festabook/android
Length of output: 4557
🏁 Script executed:
sed -n '95,110p' app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreSearchBar.ktRepository: festabook/android
Length of output: 439
🏁 Script executed:
# Search for the onSearch parameter definition to understand its signature
rg "onSearch\s*[:=]" app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreSearchBar.kt -A 2Repository: festabook/android
Length of output: 339
🏁 Script executed:
# Look for the function signature and parameter definitions
rg "fun ExploreSearchBar|@Composable" app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreSearchBar.kt -A 5Repository: festabook/android
Length of output: 624
IME 검색 시 onSearch 콜백이 호출되지 않습니다.
KeyboardActions의 onSearch 핸들러가 키보드 숨김만 수행하고 실제 검색 함수를 호출하지 않아 검색 기능이 작동하지 않습니다.
제안 수정
keyboardActions =
KeyboardActions(
onSearch = {
+ onSearch(query)
keyboardController?.hide()
},
),📝 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.
| KeyboardActions( | |
| onSearch = { | |
| keyboardController?.hide() | |
| }, | |
| KeyboardActions( | |
| onSearch = { | |
| onSearch(query) | |
| keyboardController?.hide() | |
| }, |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreSearchBar.kt`
around lines 100 - 103, The KeyboardActions onSearch currently only hides the
keyboard (keyboardController?.hide()) and never triggers the component's search
callback; update the onSearch handler in ExploreSearchBar.kt (the
KeyboardActions onSearch block) to first hide the keyboard and then invoke the
component's search submission function (e.g., call the existing
onSearch/onSearchSubmit/submitSearch lambda or method that performs the search,
passing the current query if required) so the IME search action actually runs
the search logic; also ensure the KeyboardOptions imeAction is set to Search if
not already.
| private fun checkFestivalId() { | ||
| val festivalId = exploreRepository.getFestivalId() | ||
| Timber.d("festival ID : $festivalId") | ||
| if (festivalId != null) { | ||
| _uiState.update { it.copy(hasFestivalId = true) } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find ExploreRepository and getFestivalId implementation
find . -type f -name "*.kt" | xargs grep -l "fun getFestivalId"Repository: festabook/android
Length of output: 406
🏁 Script executed:
# Look for ExploreRepository file
fd -e "\.kt$" ExploreRepositoryRepository: festabook/android
Length of output: 43
🏁 Script executed:
# Check ExploreViewModel.kt to see the current implementation
fd -e "\.kt$" ExploreViewModelRepository: festabook/android
Length of output: 43
🏁 Script executed:
# Search for SplashViewModel to compare patterns
fd -e "\.kt$" SplashViewModelRepository: festabook/android
Length of output: 43
🏁 Script executed:
# Find all ViewModel files
find . -type f -name "*ViewModel.kt"Repository: festabook/android
Length of output: 790
🏁 Script executed:
# Look at ExploreRepositoryImpl to understand getFestivalId
cat ./app/src/main/java/com/daedan/festabook/data/repository/ExploreRepositoryImpl.ktRepository: festabook/android
Length of output: 1398
🏁 Script executed:
# Check the ExploreRepository interface
cat ./app/src/main/java/com/daedan/festabook/domain/repository/ExploreRepository.ktRepository: festabook/android
Length of output: 331
🏁 Script executed:
# Search for ExploreViewModel in the codebase
rg "class ExploreViewModel" -A 50Repository: festabook/android
Length of output: 11420
🏁 Script executed:
# Check FestivalLocalDataSource implementation
cat ./app/src/main/java/com/daedan/festabook/data/datasource/local/FestivalLocalDataSourceImpl.ktRepository: festabook/android
Length of output: 1360
🏁 Script executed:
# Check the interface
cat ./app/src/main/java/com/daedan/festabook/data/datasource/local/FestivalLocalDataSource.ktRepository: festabook/android
Length of output: 259
🏁 Script executed:
# Check SplashViewModel to see the claimed pattern
cat ./app/src/main/java/com/daedan/festabook/presentation/splash/SplashViewModel.ktRepository: festabook/android
Length of output: 1996
checkFestivalId()가 메인 스레드에서 동기적으로 SharedPreferences I/O를 수행합니다.
getFestivalId()는 SharedPreferences.getLong()을 호출하는 동기 작업입니다. SplashViewModel에서는 동일한 작업을 viewModelScope.launch(iODispatcher)로 감싸서 실행합니다. 메인 스레드 블로킹을 피하려면 이 패턴을 따르세요.
🛡️ 수정 제안
+import kotlinx.coroutines.CoroutineDispatcher
+import kotlinx.coroutines.Dispatchers
+
class ExploreViewModel(
private val exploreRepository: ExploreRepository,
+ private val iODispatcher: CoroutineDispatcher = Dispatchers.IO,
) : ViewModel() {
private fun checkFestivalId() {
+ viewModelScope.launch(iODispatcher) {
val festivalId = exploreRepository.getFestivalId()
Timber.d("festival ID : $festivalId")
if (festivalId != null) {
_uiState.update { it.copy(hasFestivalId = true) }
}
+ }
}
}📝 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.
| private fun checkFestivalId() { | |
| val festivalId = exploreRepository.getFestivalId() | |
| Timber.d("festival ID : $festivalId") | |
| if (festivalId != null) { | |
| _uiState.update { it.copy(hasFestivalId = true) } | |
| } | |
| } | |
| private fun checkFestivalId() { | |
| viewModelScope.launch { | |
| val festivalId = withContext(iODispatcher) { | |
| exploreRepository.getFestivalId() | |
| } | |
| Timber.d("festival ID : $festivalId") | |
| if (festivalId != null) { | |
| _uiState.update { it.copy(hasFestivalId = true) } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/explore/ExploreViewModel.kt`
around lines 44 - 50, checkFestivalId() performs synchronous SharedPreferences
I/O by calling exploreRepository.getFestivalId() on the main thread; wrap the
call in viewModelScope.launch(iODispatcher) (same pattern as SplashViewModel) so
getFestivalId() runs on the IO dispatcher and then update _uiState (via
_uiState.update { ... }) from that coroutine, avoiding blocking the main thread.
There was a problem hiding this comment.
@parkjiminnnn 알겠습니다! 곧 제거될 코드라면 굳이 수정할 필요 없죠. 👍
(\_/)
( •_•)
/ >🗑️ 곧 안녕~
🧠 Learnings used
Learnt from: oungsi2000
Repo: festabook/android PR: 40
File: app/src/main/java/com/daedan/festabook/presentation/main/MainViewModel.kt:34-39
Timestamp: 2026-01-27T06:07:16.677Z
Learning: In the Festabook Android codebase, MainViewModel's `navigateNewsEvent` uses `MutableSharedFlow` with `replay = 1` intentionally to support push notification flows where `onNewIntent` triggers `onCreate` and new collectors need to receive the last emitted navigation event.
oungsi2000
left a comment
oungsi2000
left a comment
There was a problem hiding this comment.
고생하셨습니다 타마 !
키보드 위치에 따라 검색창이 위로 올라가는게 예쁜 것 같아요.
검색창이 깜빡이는 문제 때문에 한 번 더 검토해주시면 감사하겠습니다.
| modifier: Modifier = Modifier, | ||
| ) { | ||
| Scaffold( | ||
| containerColor = Color.White, |
There was a problem hiding this comment.
modifier 인자를 전달하지 않고있어요!
compose lint에 걸릴겁니다.
| modifier = Modifier.padding(WindowInsets.statusBars.asPaddingValues()), | ||
| ) | ||
| }, |
There was a problem hiding this comment.
Modifier.statusBarsPadding() 도 있는데 asPaddingValues를 사용하신 이유가 있으실까요?
There was a problem hiding this comment.
오 Modifier.statusBarsPadding() 를 사용하도록 수정했습니다
| viewModel.sideEffect.collect { effect -> | ||
| when (effect) { | ||
| is ExploreSideEffect.NavigateToMain -> { | ||
| latestKeyboardController?.hide() | ||
| latestNavigateToMain(effect.searchResult) | ||
| } | ||
| } |
There was a problem hiding this comment.
lifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) 없이 호출되어 백그라운드에 있어도 플로우를 수집할 수 있을 것 같아요.
ObserveAsEvent()컴포저블 확장 함수를 만들었는데 사용해주시면 감사하겠습니다.
| Box( | ||
| modifier = | ||
| Modifier | ||
| .fillMaxSize() | ||
| .padding(innerPadding) | ||
| .padding(20.dp), | ||
| ) { | ||
| ExploreSearchContent( |
There was a problem hiding this comment.
Box로 한번 더 감싸신 이유가 있으신가요?
There was a problem hiding this comment.
XXContent에는 외부 레이아웃과는 관계없이 보여주고 싶은 컨텐츠만 가지도록 했습니다.
따라서 내부 구현에는 영향이 없도록 하기 위해 XXScreen에서 Box를 사용해 레이아웃을 설정!했습니다
| navigateToMainScreen() | ||
| exploreRepository.saveFestivalId(university.festivalId) | ||
| viewModelScope.launch { | ||
| _sideEffect.tryEmit(ExploreSideEffect.NavigateToMain(university)) |
There was a problem hiding this comment.
기왕 코루틴으로 감쌌는데 emit을 쓰는건 어떠신가요?
tryEmit은 emit과 달리 collector가 없다면 지연시키지 않고 바로 실패 처리를 합니다!
There was a problem hiding this comment.
기존 Release 앱에는 상단에 '축제 검색' 이라는 문구가 있었는데 현재는 사라졌어요!
혹시 의도하신 걸까요?
There was a problem hiding this comment.
놓쳐버렸네요 😅
바로 수정했습니다.
| val isSearchResultEmpty = | ||
| searchState is SearchUiState.Success && searchState.universitiesFound.isEmpty() | ||
| val isSearchError = searchState is SearchUiState.Error |
There was a problem hiding this comment.
SearchUiState의 확장 프로퍼티로 만들면 더 깔끔하게 관리할 수 있을 것 같아요!
| focusedContainerColor = Color.White, | ||
| unfocusedContainerColor = Color.White, |
There was a problem hiding this comment.
아마 이 부분 때문에 SearchBar가 깜빡거리는 것 같은데요.
disableContainerColor, errorContainerColor도 white로 설정하거나, focusedContainerColor, unfocusedContainerColor를 Color.Transparent 로 설정하면 해결될 것 같습니다.
parkjiminnnn
left a comment
parkjiminnnn
left a comment
There was a problem hiding this comment.
고생하셨습니다 타마!
전체적으로 contentDescription 한번만 확인해주세요!
별거 아닌거라 미루지 않고 미리 해두면 좋을 것 같아서요! 다음 요청은 approve 바로 누르겠습니다~
app/src/main/java/com/daedan/festabook/presentation/explore/component/ExploreBackHeader.kt
Outdated
Show resolved
Hide resolved
| val isSearchResultEmpty = | ||
| searchState is SearchUiState.Success && searchState.universitiesFound.isEmpty() | ||
| val isSearchError = searchState is SearchUiState.Error | ||
| val isError = isSearchResultEmpty || isSearchError |
There was a problem hiding this comment.
sealed interface SearchUiState {
val isSearchResultEmpty get() = false
val isSearchError get() = isSearchResultEmpty || this is Error
data object Idle : SearchUiState
data object Loading : SearchUiState
data class Success(
val universitiesFound: List<SearchResultUiModel> = emptyList(),
) : SearchUiState {
override val isSearchResultEmpty: Boolean get() = universitiesFound.isEmpty()
}
data class Error(
val throwable: Throwable,
) : SearchUiState
}
검색 결과의 여부는 SearchUiState가 가지고 있는게 더 자연스러울 거 같은데 어떠신가용?
There was a problem hiding this comment.
해당 로직을 상태에서 관리할 수 있다고 생각하지 못했네요!
참고해서 바로 반영했어요!~
| val isSearchError = searchState is SearchUiState.Error | ||
| val isError = isSearchResultEmpty || isSearchError | ||
|
|
||
| val isSearchMode = query.isNotBlank() |
There was a problem hiding this comment.
val isSearchMode = query.isNotBlank()이것도 ExploreUiState가 가지고 있게 하는게 좋아 보여요 !
There was a problem hiding this comment.
해당 조건은 상태라고 보기보다는 화면 분기 판단 로직인 것 같아서, UI 단에 있어도 괜찮을 것 같아요!
어떻게 생각하시나요!?
There was a problem hiding this comment.
음 정답은 없다고 생각해서 편하신대로 해도 좋을 것 같아용.
검색어에 따른 화면의 상태라고 보면 이것도 uiState가 아닌가 ? 하는 생각이 들었거든요!
| Scaffold( | ||
| containerColor = Color.White, | ||
| ) { innerPadding -> | ||
| BoxWithConstraints( |
There was a problem hiding this comment.
BoxWithConstraints를 저도 과제 테스트하다가 알게 되었는데 성능 개맛있게 잡아먹는 흑마법이래요.. ㅋㅋ
There was a problem hiding this comment.
헉 그렇군요!
weight로 비율을 계산하도록 개선해보았습니다
| initialContentExit = fadeOut(tween(200)), | ||
| ) | ||
| }, | ||
| label = "LandingSearchTransition", |
|
|
||
| Image( | ||
| painter = painterResource(id = R.drawable.logo_title), | ||
| contentDescription = "FestaBook Logo", |
| private fun checkFestivalId() { | ||
| val festivalId = exploreRepository.getFestivalId() | ||
| Timber.d("festival ID : $festivalId") | ||
| if (festivalId != null) { | ||
| _uiState.update { it.copy(hasFestivalId = true) } | ||
| } | ||
| } |
- `onSearch` 람다 호출 시 `query`를 전달하도록 수정했습니다. - 하드코딩된 "뒤로가기"와 로고의 `contentDescription`을 스트링 리소스로 대체했습니다.
- `BoxWithConstraints`를 `Box`와 `weight`로 변경하여 랜딩 화면의 레이아웃을 보다 명확하게 개선했습니다.
- `SearchUiState`를 sealed interface로 변경하고, UI 상태를 더 명확하게 표현하기 위해 `isEmptyResult`, `isFailure`, `shouldShowErrorUi` 프로퍼티를 추가했습니다. - `ExploreSearchContent`와 `ExploreScreen`에서 `shouldShowErrorUi` 프로퍼티를 사용하여 에러 상태 UI를 결정하도록 로직을 단순화했습니다.
- `ExploreSearchBar`의 컨테이너 색상을 투명(`Transparent`)으로 변경했습니다. - 검색창의 커서 및 비활성화 상태의 테두리 색상 값을 추가했습니다.
oungsi2000
left a comment
oungsi2000
left a comment
There was a problem hiding this comment.
고생하셨습니다 !
ExploreBackHeader에 '축제 검색' 이라는 문구가 반영이 안된 것 같은데 확인 한번 부탁드려요
3 participants
#️⃣ 이슈 번호
🛠️ 작업 내용
sideEffect라는 이름으로 분리했습니다. 한 번만 발생해야하는 이벤트이기 때문에 sideEffect 객체와SharedFlow(replay = 0, extraBufferCapacity = 1)를 사용해 작동하도록 구현했습니다.🙇🏻 중점 리뷰 요청
📸 이미지 첨부 (Optional)
Screen_recording_20260131_021301.mp4
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.