[SPM-223] 검색 기능 구현#19
Conversation
33Auto-Bot
requested review from
CHOOSLA,
Lee-Jong-Jin,
taemin3,
vivivim and
yangjiseonn
Walkthrough부품 검색(Paging) 기능과 관련 API/DTO/매퍼/페이징소스·저장소·유스케이스·뷰모델·UI(검색바·페이징 리스트·하단 시트) 통합, 네비게이션·스타일·리소스·의존성(paging) 추가 및 일부 PullToRefresh 동작 조정이 포함됩니다. (≤50단어) Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant UI as PartScreen
participant VM as PartViewModel
participant UseCase as SearchPartsUseCase
participant Pager as Pager / PartPagingSource
participant API as PartApi
User->>UI: 키워드 입력
UI->>VM: PartUiEvent.Search(keyword)
VM->>VM: debounce 500ms, distinctUntilChanged
VM->>UseCase: invoke(keyword)
UseCase->>Pager: Pager 생성 (Factory -> PartPagingSource)
rect rgb(230,245,255)
Note over Pager,API: 페이징 로드 (비동기)
Pager->>API: searchParts(keyword, page, size)
API-->>Pager: SearchDataDto
Pager->>Pager: DTO.toModel() -> List<SearchResult>
Pager-->>VM: PagingData<SearchResult>
end
VM-->>UI: Flow<PagingData> (collectAsLazyPagingItems)
UI->>User: 페이징된 검색 결과 렌더링
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 6
🧹 Nitpick comments (12)
app/src/main/res/values/strings.xml (1)
50-51: 문구 중복/역할 정리 제안
part_placeholder_search(부품명으로 검색)와part_search_description(부품명으로 검색어를 입력하세요) 의미가 겹칩니다. 하나를 재사용하거나 위치별(placeholder vs empty-state) 역할을 명확히 구분해 네이밍을 정리해 주세요. 예:part_search_empty_description.app/src/main/java/com/sampoom/android/feature/part/domain/model/SearchResult.kt (1)
1-7: 단순 DTO 정의 좋습니다
Part와 소속 카테고리/그룹명을 함께 담는 구조가 명확합니다. 화면 간 전달이 필요해지면Parcelable적용이나 최소 표현(아이디 기반)으로의 전환만 추후 고려해 주세요.app/src/main/java/com/sampoom/android/feature/part/data/repository/PartRepositoryImpl.kt (1)
38-43: PagingConfig 최적화 제안(캐싱은 이미 구현됨)
.cachedIn(viewModelScope)은 이미 PartViewModel에서 적용되어 있습니다(확인됨).다만 API 기본 size 파라미터(10)와 PagingConfig의 pageSize(20)가 일치하지 않습니다. 네트워크 기반 리스트이므로 다음과 같이 추가 설정을 권장합니다:
- return Pager( - config = PagingConfig(pageSize = 20), - pagingSourceFactory = { PartPagingSource(api, keyword) } - ).flow + return Pager( + config = PagingConfig( + pageSize = 20, + initialLoadSize = 20, + prefetchDistance = 20, + enablePlaceholders = false + ), + pagingSourceFactory = { PartPagingSource(api, keyword) } + ).flow또한 API의 searchParts() 메서드 기본 파라미터를
size: Int = 20으로 변경하여 API 기본값과 PagingConfig를 정렬할 수 있습니다.app/build.gradle.kts (1)
88-90: Paging 의존성 추가 LGTM. 호환성 확인 완료 및 테스트 의존성 권장
- 호환성 확인 완료: Compose BOM 2025.10.01과 Paging 3.3.6은 호환됩니다. Compose BOM은 Compose 라이브러리만 관리하며, Paging은 독립적인 AndroidX 아티팩트로 안전하게 사용됩니다.
- 유닛테스트에서 PagingSource를 검증하려면
paging-testing또는paging-common을 테스트 의존성으로 추가하는 것을 권장합니다.dependencies { // Paging implementation(libs.androidx.paging.runtime) implementation(libs.androidx.paging.compose) + testImplementation(libs.androidx.paging.testing) // 또는 paging-common }먼저
gradle/libs.versions.toml에 다음을 추가하세요:[libraries] androidx-paging-testing = { group = "androidx.paging", name = "paging-testing", version.ref = "paging" }app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt (3)
77-81: NavHost 배경 적용 시 전체 영역 보장 필요NavHost에 배경만 지정하면 일부 레이아웃에서 빈 영역이 생길 수 있습니다. fillMaxSize를 함께 적용해 전체 화면을 확실히 채우세요.
적용 diff:
@@ -import androidx.compose.foundation.background +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.fillMaxSize @@ NavHost( navController = navController, - startDestination = if (isLoggedIn) ROUTE_HOME else ROUTE_LOGIN, - modifier = Modifier.background(backgroundColor()) + startDestination = if (isLoggedIn) ROUTE_HOME else ROUTE_LOGIN, + modifier = Modifier + .fillMaxSize() + .background(backgroundColor()) ) {
111-116: savedStateHandle 전달 패턴 확인현재 화면의 savedStateHandle에 "groupName"을 set 후 navigate하는 패턴은, 목적지에서 previousBackStackEntry로 접근해야 합니다. PartListScreen에서 아래처럼 읽는지 확인해 주세요.
예시:
val groupName = navController.previousBackStackEntry ?.savedStateHandle ?.get<String>("groupName")키 오타 방지를 위해 상수로 추출하는 것도 권장합니다.
54-54: 미사용 상수 제거 또는 구현 필요검증 결과
ROUTE_SEARCH는 정의되었으나 전체 코드베이스에서 사용되지 않고 있습니다. 다음 중 하나를 선택하세요:
- 현재 사용할 계획이 없다면 선언을 제거
- 검색 화면 라우팅에 실제로 연결할 계획이라면 네비게이션 로직에 통합
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (5)
129-141: 하드코딩된 접근성 문자열 제거contentDescription에 하드코딩된 "검색어 지우기" 대신 stringResource를 사용해 i18n/접근성 품질을 보장하세요.
적용 diff (strings.xml에 R.string.search_clear 추가 필요):
- Icon( - imageVector = Icons.Default.Clear, - contentDescription = "검색어 지우기" - ) + Icon( + imageVector = Icons.Default.Clear, + contentDescription = stringResource(R.string.search_clear) + )
132-134: TextFieldValue 초기화 방식 개선copy("")는 이전 selection/composition을 유지해 예기치 않은 selection 상태가 남을 수 있습니다. 완전한 리셋이 필요하면 새 인스턴스를 사용하세요.
적용 diff:
- textFieldState = textFieldState.copy("") + textFieldState = TextFieldValue("")
175-212: Paging 부하 상태(append) 처리 보완refresh만 처리하고 append 상태(추가 로딩/에러) UI는 없습니다. 리스트 하단 로더/재시도 아이템을 추가하면 UX가 개선됩니다.
예시(요약):
when (val append = searchResultsPaged.loadState.append) { is LoadState.Loading -> item { CircularProgressIndicator(Modifier.fillMaxWidth()) } is LoadState.Error -> item { ErrorContent(onRetry = { searchResultsPaged.retry() }, modifier = Modifier.fillMaxWidth()) } else -> Unit }
221-221: 고정 Spacer(100.dp) 마진 — 추후 레이아웃 깨짐 위험SearchBar 높이에 의존하는 고정값은 기기/폰트/밀도에 따라 오차가 생길 수 있습니다. Scaffold의 topBar로 올리거나, WindowInsets/measure 기반으로 계산하는 방식으로 전환을 권장합니다.
48-49: 불필요한 import 정리kotlin.collections.forEach는 확장 함수로 별도 import가 필요 없습니다. 정리하여 빌드 로그를 깨끗하게 유지하세요.
적용 diff:
-import kotlin.collections.forEach
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
app/build.gradle.kts(1 hunks)app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt(8 hunks)app/src/main/java/com/sampoom/android/core/ui/component/EmptyContent.kt(2 hunks)app/src/main/java/com/sampoom/android/feature/cart/ui/CartListScreen.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/order/ui/OrderDetailScreen.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/order/ui/OrderListScreen.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/outbound/ui/OutboundListScreen.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/part/data/mapper/PartMappers.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/part/data/paging/PartPagingSource.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/part/data/remote/api/PartApi.kt(2 hunks)app/src/main/java/com/sampoom/android/feature/part/data/remote/dto/SearchDataDto.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/part/data/repository/PartRepositoryImpl.kt(2 hunks)app/src/main/java/com/sampoom/android/feature/part/domain/model/SearchResult.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/part/domain/repository/PartRepository.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/part/domain/usecase/SearchPartsUseCase.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/part/ui/PartListScreen.kt(8 hunks)app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt(8 hunks)app/src/main/java/com/sampoom/android/feature/part/ui/PartUiEvent.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/part/ui/PartUiState.kt(2 hunks)app/src/main/java/com/sampoom/android/feature/part/ui/PartViewModel.kt(2 hunks)app/src/main/res/values/strings.xml(1 hunks)gradle/libs.versions.toml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
app/src/main/java/com/sampoom/android/core/ui/component/EmptyContent.kt (1)
app/src/main/java/com/sampoom/android/core/ui/theme/Color.kt (1)
textSecondaryColor(259-260)
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (4)
app/src/main/java/com/sampoom/android/core/ui/theme/Color.kt (5)
backgroundColor(250-251)backgroundCardColor(253-254)textSecondaryColor(259-260)textColor(256-257)disableColor(262-263)app/src/main/java/com/sampoom/android/core/ui/component/ErrorContent.kt (1)
ErrorContent(18-37)app/src/main/java/com/sampoom/android/core/ui/component/EmptyContent.kt (1)
EmptyContent(11-26)app/src/main/java/com/sampoom/android/feature/part/ui/PartDetailBottomSheet.kt (1)
PartDetailBottomSheet(45-272)
app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt (1)
app/src/main/java/com/sampoom/android/core/ui/theme/Color.kt (1)
backgroundColor(250-251)
app/src/main/java/com/sampoom/android/feature/part/ui/PartListScreen.kt (1)
app/src/main/java/com/sampoom/android/core/ui/theme/Color.kt (1)
disableColor(262-263)
🔇 Additional comments (19)
app/src/main/java/com/sampoom/android/core/ui/component/EmptyContent.kt (1)
4-4: 테마 기반 스타일링으로의 리팩토링 승인Material Design 모범 사례에 따라 테마 기반 색상과 타이포그래피를 적용한 개선입니다.
textSecondaryColor()를 통해 다크/라이트 모드를 자동으로 지원하며,MaterialTheme.typography.bodyMedium을 사용하여 일관된 타이포그래피를 유지합니다. 앱 전반의 테마 업데이트와도 일관성이 있습니다.Also applies to: 9-9, 20-24
app/src/main/java/com/sampoom/android/feature/outbound/ui/OutboundListScreen.kt (1)
82-96: PullToRefreshBox의 isRefreshing이 항상 false로 설정되어 있습니다.
isRefreshing을false로 고정하면 PullToRefreshBox 자체의 리프레시 애니메이션이 비활성화됩니다. 내부 Indicator는 여전히uiState.outboundLoading을 사용하므로 로딩 표시는 작동하지만, 사용자가 pull-to-refresh 제스처를 수행할 때 박스 레벨의 피드백이 없습니다.이 변경사항이 의도적인지 확인해 주세요. 동일한 패턴이 CartListScreen, OrderListScreen, OrderDetailScreen에도 적용되어 있어 일관된 설계 결정으로 보이지만, 사용자 경험에 영향을 줄 수 있습니다.
만약 이것이 의도하지 않은 변경이라면, 다음과 같이 수정하세요:
PullToRefreshBox( - isRefreshing = false, + isRefreshing = uiState.outboundLoading, onRefresh = { viewModel.onEvent(OutboundListUiEvent.LoadOutboundList) },app/src/main/java/com/sampoom/android/feature/cart/ui/CartListScreen.kt (1)
76-90: PullToRefreshBox의 isRefreshing이 항상 false로 설정되어 있습니다.OutboundListScreen과 동일한 패턴이 적용되었습니다.
isRefreshing = false로 설정하면 PullToRefreshBox의 리프레시 상태 표시가 비활성화되지만, 내부 Indicator는 여전히uiState.cartLoading을 사용합니다.이 변경이 의도적인 UX 개선인지 확인이 필요합니다.
app/src/main/java/com/sampoom/android/feature/order/ui/OrderDetailScreen.kt (1)
84-98: PullToRefreshBox의 isRefreshing이 항상 false로 설정되어 있습니다.다른 화면들과 동일한 패턴입니다.
isRefreshing을false로 고정하면 PullToRefreshBox 레벨의 리프레시 애니메이션이 비활성화됩니다.이 변경이 전체 앱의 일관된 UX 전략인지 확인해 주세요.
app/src/main/java/com/sampoom/android/feature/order/ui/OrderListScreen.kt (1)
66-80: PullToRefreshBox의 isRefreshing이 항상 false로 설정되어 있습니다.이 파일을 포함하여 총 4개의 화면(OutboundListScreen, CartListScreen, OrderDetailScreen, OrderListScreen)에서 동일한 패턴이 적용되었습니다. 이는 일관된 설계 변경으로 보이지만, pull-to-refresh 제스처 시 박스 레벨의 시각적 피드백이 제거되는 영향이 있습니다.
전체 앱에서 이러한 접근 방식을 의도적으로 채택한 것인지 확인해 주세요.
app/src/main/java/com/sampoom/android/feature/part/ui/PartUiEvent.kt (1)
10-11: 이벤트 분리 방향 LGTM. 사용 시멘틱만 명확히
SetKeyword(입력 변화)와Search(검색 실행) 용도가 분리된 점 좋습니다.Search가 실제 네트워크 트리거임을 코드 주석/핸들러에서 명확히 해 두세요(디바운스/중복호출 방지 포함).app/src/main/java/com/sampoom/android/feature/part/domain/repository/PartRepository.kt (1)
3-14: LGTM!검색 기능을 위한 인터페이스 확장이 깔끔합니다. Paging3의
Flow<PagingData<T>>패턴을 올바르게 사용하고 있습니다.app/src/main/java/com/sampoom/android/feature/part/domain/usecase/SearchPartsUseCase.kt (1)
9-15: LGTM!Use Case 레이어가 올바르게 구현되었습니다. 단순한 위임 패턴으로 repository를 래핑하여 clean architecture를 따르고 있습니다.
app/src/main/java/com/sampoom/android/feature/part/ui/PartListScreen.kt (2)
236-236: LGTM!chevron 아이콘의 색상을
disableColor()로 변경하여 UI 계층 구조를 더 명확하게 표현하고 있습니다.
62-65: 네비게이션 구조와 상태 전달이 안전하게 구현됨검증 결과,
PartListScreen으로의 네비게이션은PartScreen에서만 시작되며(AppNavHost.kt:113-114),groupName은 네비게이션 전에currentBackStackEntry에 설정된 후 전달됩니다. 따라서previousBackStackEntry는 정상 네비게이션 흐름에서 항상 존재하며, 해당 데이터를 포함합니다. 심화 링크나 다른 진입점은 없으며, 현재 구현된 null 안전 처리(?.getStateFlow()?.collectAsState() ?: remember { mutableStateOf(null) })는 앱 재시작 등의 엣지 케이스를 충분히 커버합니다.app/src/main/java/com/sampoom/android/feature/part/ui/PartViewModel.kt (2)
55-68: 검색 Flow 구현이 우수합니다검색 Flow 구현이 잘 되어 있습니다:
debounce(300): 불필요한 API 호출 방지distinctUntilChanged(): 중복 검색 방지flatMapLatest: 이전 검색 취소cachedIn(viewModelScope): 구성 변경 시 데이터 유지빈 키워드에 대해 빈 PagingData를 반환하는 것도 적절합니다.
76-82: LGTM!검색 이벤트 처리가 적절하게 구현되어 있습니다.
_searchKeywordFlow를 업데이트하고 UI 상태를 동기화합니다.app/src/main/java/com/sampoom/android/feature/part/data/remote/dto/SearchDataDto.kt (1)
3-20: LGTM!검색 응답을 위한 DTO 구조가 잘 설계되었습니다. 계층적 데이터(Category → Group → Part)와 페이지네이션 정보를 명확하게 모델링하고 있습니다.
gradle/libs.versions.toml (1)
20-20: LGTM!Paging 3.3.6 버전 추가가 적절합니다.
paging-runtime과paging-compose의존성을 함께 추가하여 Compose에서 Paging을 완전히 지원할 수 있습니다.Also applies to: 48-49
app/src/main/java/com/sampoom/android/feature/part/data/paging/PartPagingSource.kt (2)
15-20: LGTM!
getRefreshKey구현이 표준 Paging 패턴을 올바르게 따르고 있습니다. anchor position 기준으로 적절한 키를 반환합니다.
22-37: Now let me search for the PartApi definition to understand the API contract:The web search didn't directly find the PartApi definition. Let me search within the codebase directly:Based on my analysis of the Paging library documentation and code patterns:
params.loadSize가 API 호출 간 일관성 있게 작동하지 않을 수 있다는 우려는 타당합니다.
PagingConfig.pageSize가 20이고 initialLoadSize를 명시하지 않으면, LoadParams.loadSize는 초기 로드에서는 initialLoadSize(기본값 3 × pageSize = 60)를 반환하고, 이후 로드에서만 pageSize(20)를 반환합니다. 스크립트 결과에서 확인한 PagingConfig 설정이 바로 이 경우입니다.
현재 구현의 실제 위험성:
코드가
params.loadSize를 직접 API에 전달하면서 동시에 page 번호로 페이지 계산을 하는 경우, 다음과 같은 문제가 발생할 수 있습니다:
- 초기 로드: size=60으로 요청
- 다음 로드: size=20으로 요청
- API가 요청한 size를 기준으로 totalPages를 계산한다면, 이 값의 일관성이 깨질 수 있습니다.
권장 수정:
일관성을 보장하기 위해 고정 크기를 사용하세요:
- val response = api.searchParts(keyword, page, params.loadSize) + val response = api.searchParts(keyword, page, 20)또는 PagingConfig에서 초기 로드 크기를 명시적으로 설정하세요:
config = PagingConfig( pageSize = 20, + initialLoadSize = 20 )app/src/main/java/com/sampoom/android/feature/part/data/mapper/PartMappers.kt (1)
17-33: LGTM!매퍼 구현이 우수합니다. 계층적 구조(Category → Group → Part)를
flatMap을 사용하여 평탄화된SearchResult리스트로 올바르게 변환하고 있습니다. 각 부품에 대해 카테고리명과 그룹명을 함께 유지하는 것이 검색 결과 표시에 적합합니다.app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt (1)
204-208: FAB 아이콘 틴트 적용 LGTM명확한 대비를 위해 White 틴트 적용이 적절합니다.
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (1)
380-396: 바텀시트 상태 처리 LGTM선택된 파트 존재 시에만 시트를 그리도록 한 조건부 렌더링과 해제 시 ViewModel 이벤트 정리가 깔끔합니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
app/src/main/java/com/sampoom/android/feature/part/data/paging/PartPagingSource.kt (2)
37-40: nextKey 계산의 서버 계약 의존성 — 확인 또는 방어적 처리 권장
totalPages가 요청 시 전달한size = params.loadSize기준으로 계산된다는 서버 보장이 필요합니다. 불일치 시 마지막 페이지 판단이 어긋나 중복/누락 로드가 발생할 수 있습니다. 백엔드 계약을 확인하거나, 방어적으로 빈 데이터 기반으로 종료를 고려하세요.가능한 대안 예시:
- nextKey = if (page < response.data.totalPages - 1) page + 1 else null + // 계약 확인 전 임시 안전책: 응답이 비면 종료 + nextKey = if (flatParts.isEmpty()) null else page + 1
29-35: 검색어 정규화로 불필요한 호출/캐시 미스 최소화공백만 다른 쿼리로 별도 페이지가 생성될 수 있습니다. 트리밍 후 호출을 권장합니다.
- val response = api.searchParts(keyword, page, params.loadSize) + val safeKeyword = keyword.trim() + val response = api.searchParts(safeKeyword, page, params.loadSize)app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (3)
68-69: 검색 텍스트 상태 Saveable 권장(회전/프로세스 재생성 대비)
TextFieldValue를rememberSaveable로 보존하면 회전 등 구성 변경 시 검색어가 유지됩니다.- var textFieldState by remember { mutableStateOf(TextFieldValue("")) } + var textFieldState by rememberSaveable(stateSaver = TextFieldValue.Saver) { + mutableStateOf(TextFieldValue("")) + }
66-68: ViewModel 분리(검색 결과 vs. 바텀시트 선택) — 결합도 완화 제안검색 결과는
viewModel, 선택/시트 상태는searchViewModel이 담당합니다. 교차 의존 이벤트가 늘면 상태 동기화 비용이 생길 수 있습니다. 단일 VM로 통합하거나, 명확한 경계(예: SearchFeatureViewModel)로 분리하는 방안을 검토하세요.현재 구조가 의도된 분할인지 확인 부탁드립니다.
485-510: Paging 리스트: contentType 지정으로 재구성 비용 절감 가능항목의 유형이 단일하더라도
contentType을 지정하면 측정/배치 캐시가 향상됩니다.- items( + items( count = searchResults.itemCount, key = searchResults.itemKey { it.part.partId } - ) { index -> + , contentType = { "part_search_item" } + ) { index ->
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/com/sampoom/android/feature/part/data/paging/PartPagingSource.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/part/data/remote/api/PartApi.kt(2 hunks)app/src/main/java/com/sampoom/android/feature/part/data/repository/PartRepositoryImpl.kt(2 hunks)app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt(8 hunks)app/src/main/java/com/sampoom/android/feature/part/ui/PartUiState.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/sampoom/android/feature/part/data/remote/api/PartApi.kt
- app/src/main/java/com/sampoom/android/feature/part/ui/PartUiState.kt
- app/src/main/java/com/sampoom/android/feature/part/data/repository/PartRepositoryImpl.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (4)
app/src/main/java/com/sampoom/android/core/ui/theme/Color.kt (5)
backgroundColor(250-251)backgroundCardColor(253-254)textSecondaryColor(259-260)textColor(256-257)disableColor(262-263)app/src/main/java/com/sampoom/android/core/ui/component/ErrorContent.kt (1)
ErrorContent(18-37)app/src/main/java/com/sampoom/android/core/ui/component/EmptyContent.kt (1)
EmptyContent(11-26)app/src/main/java/com/sampoom/android/feature/part/ui/PartDetailBottomSheet.kt (1)
PartDetailBottomSheet(45-272)
🔇 Additional comments (4)
app/src/main/java/com/sampoom/android/feature/part/data/paging/PartPagingSource.kt (2)
12-20: AssistedInject 적용 적절 — 런타임 파라미터 처리 정석
@AssistedInject+@AssistedFactory로keyword런타임 파라미터를 올바르게 처리했습니다. Hilt 관례에 부합합니다.
22-27: getRefreshKey 구현 적절표준 구현으로 문제 없습니다.
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (2)
71-76: 디바운스 스코프 관리 개선 LGTM
rememberCoroutineScope()로 스코프를 고정하고searchJob으로 이전 작업을 취소하는 패턴이 적절합니다.
339-357: 레이아웃 확장 이슈 해결 LGTM
height(200.dp).fillMaxWidth()로 수정되어 의도치 않은 전체 높이 확장을 방지합니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (1)
486-490: 가시성 수정 권장: 재사용하지 않는다면 private으로 변경하세요.
SearchResultsList가 이 파일 내에서만 사용되고 다른 곳에서 재사용할 계획이 없다면,private키워드를 추가하여 가시성을 제한하는 것이 좋습니다.적용 diff:
@Composable -fun SearchResultsList( +private fun SearchResultsList( searchResults: LazyPagingItems<SearchResult>, onPartClick: (Part) -> Unit, ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (4)
app/src/main/java/com/sampoom/android/core/ui/theme/Color.kt (5)
backgroundColor(250-251)backgroundCardColor(253-254)textSecondaryColor(259-260)textColor(256-257)disableColor(262-263)app/src/main/java/com/sampoom/android/core/ui/component/ErrorContent.kt (1)
ErrorContent(18-37)app/src/main/java/com/sampoom/android/core/ui/component/EmptyContent.kt (1)
EmptyContent(11-26)app/src/main/java/com/sampoom/android/feature/part/ui/PartDetailBottomSheet.kt (1)
PartDetailBottomSheet(45-272)
🔇 Additional comments (2)
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (2)
73-73: 이전 리뷰 피드백이 모두 적절히 반영되었습니다!이전 리뷰에서 지적된 주요 문제들이 올바르게 수정되었습니다:
rememberCoroutineScope()를 사용하여 코루틴 스코프 누수 해결- 즉시 검색 및 Clear 버튼 클릭 시 대기 중인 디바운스 작업 취소
Also applies to: 96-96, 108-108, 135-135
218-382: 카테고리/그룹 선택 및 하단 시트 통합이 잘 구현되었습니다.코드가 일관된 패턴을 따르고 있으며, 로딩/에러/빈 상태 처리가 적절히 구현되어 있습니다. 하단 시트 상태 관리도 올바르게 처리되고 있습니다.
Also applies to: 384-402, 514-570
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/src/main/java/com/sampoom/android/feature/part/data/paging/PartPagingSource.kt (1)
29-44: 페이지 크기를 params.loadSize로 변경하는 것을 권장합니다.Line 32에서 페이지 크기가 20으로 하드코딩되어 있어, Paging 설정에서 지정한
loadSize가 무시됩니다. 일관성을 위해params.loadSize를 사용하세요.다음과 같이 수정하세요:
- val response = api.searchParts(keyword, page, 20) + val response = api.searchParts(keyword, page, params.loadSize)app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (1)
515-542: 페이징 append 로드 상태 처리가 추가되었습니다.이전 리뷰에서 지적된 append 로드 상태 처리가 구현되었습니다. 다만, append 에러 상태에서 재시도 버튼이 없어 사용자가 추가 페이지 로드 실패 시 대응하기 어렵습니다.
선택적으로 append 에러 상태에도 재시도 기능을 추가할 수 있습니다:
is LoadState.Error -> { - Box( - modifier = Modifier - .fillMaxWidth() - .padding(16.dp), - contentAlignment = Alignment.Center - ) { - Text( - text = stringResource(R.string.common_error), - color = FailRed - ) - } + Column( + modifier = Modifier + .fillMaxWidth() + .padding(16.dp), + horizontalAlignment = Alignment.CenterHorizontally + ) { + Text( + text = stringResource(R.string.common_error), + color = FailRed + ) + Spacer(modifier = Modifier.height(8.dp)) + TextButton(onClick = { searchResults.retry() }) { + Text(stringResource(R.string.common_retry)) + } + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/sampoom/android/feature/part/data/paging/PartPagingSource.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt(8 hunks)app/src/main/res/values/strings.xml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (4)
app/src/main/java/com/sampoom/android/core/ui/theme/Color.kt (5)
backgroundColor(250-251)backgroundCardColor(253-254)textSecondaryColor(259-260)textColor(256-257)disableColor(262-263)app/src/main/java/com/sampoom/android/core/ui/component/ErrorContent.kt (1)
ErrorContent(18-37)app/src/main/java/com/sampoom/android/core/ui/component/EmptyContent.kt (1)
EmptyContent(11-26)app/src/main/java/com/sampoom/android/feature/part/ui/PartDetailBottomSheet.kt (1)
PartDetailBottomSheet(45-272)
🔇 Additional comments (10)
app/src/main/res/values/strings.xml (1)
50-52: LGTM! 검색 UI를 위한 문자열 리소스가 적절히 추가되었습니다.새로 추가된 문자열 리소스들이 명명 규칙을 잘 따르고 있으며, 검색 기능 UI에서 필요한 모든 텍스트를 제공합니다.
app/src/main/java/com/sampoom/android/feature/part/data/paging/PartPagingSource.kt (2)
12-20: LGTM! AssistedInject 패턴이 올바르게 구현되었습니다.이전 리뷰에서 지적된
@Inject대신@AssistedInject사용 문제가 해결되었습니다. 런타임 파라미터인keyword가@Assisted로 적절히 표시되었고, Factory 인터페이스도 올바르게 정의되었습니다.
22-27: LGTM! getRefreshKey 구현이 표준 패턴을 따릅니다.Android Paging 라이브러리의 권장 사항에 따라 올바르게 구현되었습니다.
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (7)
69-71: LGTM! 코루틴 스코프 누수 문제가 해결되었습니다.이전 리뷰에서 지적된
CoroutineScope()생성 문제가rememberCoroutineScope()를 사용하여 올바르게 수정되었습니다.
81-161: LGTM! SearchBar 구현이 이전 리뷰 피드백을 모두 반영했습니다.
- 디바운스 로직에서 중복 검색 방지를 위한 job 취소가 올바르게 구현되었습니다.
- contentDescription이
stringResource로 현지화되었습니다.- Clear 버튼에서도 대기 중인 검색 job을 적절히 취소합니다.
162-215: LGTM! 검색 결과 로드 상태가 포괄적으로 처리되었습니다.로딩, 에러, 빈 결과 상태가 모두 적절히 처리되고 있으며, 에러 상태에서 재시도 기능도 제공합니다.
335-364: LGTM! 이전 리뷰에서 지적된 fillMaxSize 문제가 수정되었습니다.그룹 목록 섹션의 모든 상태(로딩, 에러, 빈 상태)에서
height(200.dp).fillMaxWidth()수정자를 일관되게 사용하고 있습니다.
388-406: LGTM! ModalBottomSheet 구현이 적절합니다.Bottom sheet의 상태 관리와 dismiss 처리가 올바르게 구현되어 있습니다.
546-604: LGTM! SearchPartItem 구성 요소가 잘 구현되었습니다.검색 결과 아이템이 기존 UI 패턴과 일관되게 구현되었으며, 카테고리/그룹 경로, 부품 정보, 재고량을 명확하게 표시합니다.
54-55: 두 ViewModel의 역할이 명확히 분리되어 있어 현재 구조는 적절합니다.PartViewModel은 카테고리/그룹 네비게이션과 검색을 담당하고, PartListViewModel은 선택된 그룹의 부품 목록과 바텀시트를 관리합니다. 각각 다른 상태(categoryList, groupList vs partList), 다른 유스케이스, 그리고 PartListViewModel은 SavedStateHandle을 통해 네비게이션 인자를 받으므로 두 ViewModel으로의 분리는 단일 책임 원칙을 적절히 따르고 있습니다.
5 participants
📝 Summary
Android SearchBar + Paging 라이브러리 사용하여 검색 기능을 구현했습니다
🙏 Question & PR point
📬 Reference
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
스타일
Chores