Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough콜밴(CallVan) 기능에 필터 타입(sealed Parcelable), 바텀시트 기반 필터 UI(칩·아이템·다중선택·리셋·적용), 검색 텍스트필드 및 관련 상수, 벡터 드로어블·문자열 리소스가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant UI as FilterBottomSheet (Composable)
participant Actions as FilterBottomSheetActions
participant Host as 화면/뷰모델
User->>UI: 필터 열기 / 항목 선택 / 검색어 입력
UI->>Actions: onSortTypeChange / onStatusesTypeChange / onDeparturesTypeChange / onArrivalsTypeChange
Actions->>Host: 콜백으로 선택값 전달
User->>UI: Reset 클릭
UI->>Actions: onReset()
Actions->>Host: 초기 상태 요청/반영
User->>UI: Apply 클릭
UI->>Actions: onApplyClick()
Actions->>Host: 선택값 전달
Host->>UI: onDismissRequest 호출
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Member
|
@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: 4
🧹 Nitpick comments (5)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt (2)
72-75: 콜백 타입을CallvanFilterType로 넓히지 않는 편이 안전합니다.지금은 하위 섹션에서 공통 타입으로 넘긴 값을 여기서 다시
as SortType/as StatusesType/as ...FilterType로 좁히고 있어서, 연결이 한 번만 잘못돼도 런타임ClassCastException으로 끝납니다.FilterSection/FilterDuplicateSection을 제네릭으로 바꾸고FilterBottomSheetContent콜백도 구체 타입을 유지하면 이 리스크를 컴파일 타임에 막을 수 있습니다.예시 리팩터링
fun FilterBottomSheetContent( selectedSortType: SortType, selectedStatusesType: StatusesType, selectedDeparturesType: ImmutableList<DeparturesFilterType>, selectedArrivalsType: ImmutableList<ArrivalsFilterType>, - onSortTypeChange: (CallvanFilterType) -> Unit, - onStatusesTypeChange: (CallvanFilterType) -> Unit, - onDeparturesTypeChange: (ImmutableList<CallvanFilterType>) -> Unit, - onArrivalsTypeChange: (ImmutableList<CallvanFilterType>) -> Unit, + onSortTypeChange: (SortType) -> Unit, + onStatusesTypeChange: (StatusesType) -> Unit, + onDeparturesTypeChange: (ImmutableList<DeparturesFilterType>) -> Unit, + onArrivalsTypeChange: (ImmutableList<ArrivalsFilterType>) -> Unit, onReset: () -> Unit, onApplyClick: () -> Unit ) -fun FilterSection( +fun <T : CallvanFilterType> FilterSection( title: String, - items: ImmutableList<CallvanFilterType>, - selectedItem: CallvanFilterType, - onItemSelected: (CallvanFilterType) -> Unit + items: ImmutableList<T>, + selectedItem: T, + onItemSelected: (T) -> Unit ) -fun FilterDuplicateSection( +fun <T : CallvanFilterType> FilterDuplicateSection( title: String, - items: ImmutableList<CallvanFilterType>, - selectedItems: ImmutableList<CallvanFilterType>, - onItemSelected: (ImmutableList<CallvanFilterType>) -> Unit + items: ImmutableList<T>, + selectedItems: ImmutableList<T>, + onItemSelected: (ImmutableList<T>) -> Unit )Also applies to: 87-88, 125-128, 236-240, 266-270
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt` around lines 72 - 75, Change the unsafe downcasts by making the filter section components and callbacks generic over the specific CallvanFilterType subtype instead of using the common CallvanFilterType and casting; i.e., convert FilterSection and FilterDuplicateSection to generics like FilterSection<T : CallvanFilterType> and update FilterBottomSheetContent’s callback signatures (onSortTypeChange, onStatusesTypeChange, onArrivalsTypeChange, etc.) to accept the concrete generic T so callers receive SortType / StatusesType / ArrivalsFilterType directly; then propagate the generics through the calling site so assignments like selectedSortType = it no longer require as-casts and the compiler enforces type safety.
49-55: 필터 선택값은@Immutable상태 객체로 묶는 편이 낫습니다.지금 API는 관련 값이 흩어져 있어서 호출부가 길고,
selectedArrivalsType/selectedDeparturesType처럼 의미가 비슷한 값은 순서 실수도 나기 쉽습니다. 상태 객체 하나와 액션 콜백으로 정리하면 Sonar 경고도 같이 사라지고, 이후 필터가 늘어나도 API가 덜 흔들립니다. As per coding guidelines, "Compose 최적화를 위해 State 클래스에@Immutable및 ImmutableList 사용 필수".예시 구조
+import androidx.compose.runtime.Immutable + +@Immutable +data class FilterBottomSheetUiState( + val selectedSortType: SortType, + val selectedStatusesType: StatusesType, + val selectedDeparturesType: ImmutableList<DeparturesFilterType>, + val selectedArrivalsType: ImmutableList<ArrivalsFilterType>, +) + `@Composable` fun FilterBottomSheet( onDismissRequest: () -> Unit, - selectedSortType: SortType, - selectedStatusesType: StatusesType, - selectedArrivalsType: ImmutableList<ArrivalsFilterType>, - selectedDeparturesType: ImmutableList<DeparturesFilterType>, - onApply: (SortType, StatusesType, ImmutableList<DeparturesFilterType>, ImmutableList<ArrivalsFilterType>) -> Unit + state: FilterBottomSheetUiState, + onApply: (FilterBottomSheetUiState) -> Unit ) `@Composable` fun FilterBottomSheetContent( - selectedSortType: SortType, - selectedStatusesType: StatusesType, - selectedDeparturesType: ImmutableList<DeparturesFilterType>, - selectedArrivalsType: ImmutableList<ArrivalsFilterType>, + state: FilterBottomSheetUiState, onSortTypeChange: (SortType) -> Unit, onStatusesTypeChange: (StatusesType) -> Unit, onDeparturesTypeChange: (ImmutableList<DeparturesFilterType>) -> Unit, onArrivalsTypeChange: (ImmutableList<ArrivalsFilterType>) -> Unit, onReset: () -> Unit, onApplyClick: () -> Unit )Also applies to: 120-130
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt` around lines 49 - 55, 현재 FilterBottomSheet의 여러 개의 분산된 파라미터(selectedSortType, selectedStatusesType, selectedArrivalsType, selectedDeparturesType, onApply)를 하나의 불변 상태 객체로 묶어 전달하도록 변경하세요: 정의할 `@Immutable` 데이터 클래스(e.g., FilterState)에 SortType, StatusesType, ImmutableList<ArrivalsFilterType>, ImmutableList<DeparturesFilterType>를 포함시키고 FilterBottomSheet 시그니처는 해당 상태 객체와 액션 콜백(onFilterChange 또는 onApplyFilter 같은 단일 콜백)을 받도록 바꿉니다; 기존 selected* 파라미터와 onApply 사용처(호출부)를 모두 해당 상태 객체와 단일 콜백으로 교체하여 호출부가 간결해지고 인자 순서 실수와 Sonar 경고를 제거하세요.feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/Constant.kt (1)
3-3: 공개 상수 타입을 명시해 주세요.
publictop-level 상수라Int를 명시해 두는 편이 안전합니다.제안된 수정
-const val TEXT_FIELD_MAX_LENGTH = 255 +const val TEXT_FIELD_MAX_LENGTH: Int = 255As per coding guidelines "MUST use explicit types for public APIs; type inference is preferred for private/local variables".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/Constant.kt` at line 3, The public top-level constant TEXT_FIELD_MAX_LENGTH in Constant.kt lacks an explicit type; update its declaration to specify the Int type (e.g., change TEXT_FIELD_MAX_LENGTH = 255 to TEXT_FIELD_MAX_LENGTH: Int = 255) so the public API has an explicit type annotation.feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanFilterChip.kt (1)
26-29: 클릭 람다는 직접 전달해 주세요.불필요한 래핑이라 재구성 때마다 새 람다가 만들어집니다.
제안된 수정
- onClick = { - onClick() - }, + onClick = onClick,As per coding guidelines "람다가 불필요하게 재생성되지 않도록, 내부에 선언된 외부 변수는 직접 참조하여 접근하는 방식으로 수정".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanFilterChip.kt` around lines 26 - 29, The Surface onClick currently wraps the passed-in onClick by creating a new lambda (Surface(onClick = { onClick() })), causing unnecessary allocations; change it to pass the callback directly (Surface(onClick = onClick)) and ensure the CallvanFilterChip composable's onClick parameter is of type () -> Unit so no outer variables are captured inside a new lambda.feature/callvan/src/main/res/values/strings.xml (1)
59-59:filter_list_adapt는 의미가 맞지 않는 리소스명 같습니다.표시 문자열이
적용하기라서adapt보다apply가 더 자연스럽습니다. 지금 이름으로 굳어지면 이후 참조부 검색성과 가독성이 떨어집니다.✏️ 제안
- <string name="filter_list_adapt">적용하기</string> + <string name="filter_list_apply">적용하기</string>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/res/values/strings.xml` at line 59, The resource key filter_list_adapt is semantically misleading for the displayed string "적용하기"; rename the resource to filter_list_apply and update all references: replace <string name="filter_list_adapt"> with <string name="filter_list_apply"> in strings.xml, then update every XML reference (`@string/filter_list_adapt`) and Java/Kotlin usages (R.string.filter_list_adapt or getString(R.string.filter_list_adapt)) to use filter_list_apply; keep the string value unchanged and run a project-wide search/replace and rebuild to ensure no unresolved references remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/enums/CallvanFilterType.kt`:
- Around line 9-12: The API expects omitted filters as null, but
CallvanFilterType.All is encoded as "ALL", causing queries like ?departures=ALL;
update the code that builds request parameters (where FilterBottomSheet.onApply
passes selected filters) to drop any CallvanFilterType.All entries
(CallvanFilterType.All) and convert empty lists to null before calling the API,
or alternatively treat a selection that contains only All as null; ensure this
logic is applied for departures, arrivals and statuses collections so no "ALL"
string is sent.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt`:
- Around line 132-233: The top-level Column in the FilterBottomSheet composable
currently lacks scrolling so the bottom action Row (Reset/Apply) can be cut off
on small screens; update the modifier on the outer Column (the one wrapping
FilterSection / FilterDuplicateSection and the Row) to include
verticalScroll(rememberScrollState()) to enable scrolling while keeping the
current layout, or alternatively replace the outer Column with a LazyColumn if
you prefer lazy rendering; ensure you import rememberScrollState/verticalScroll
and keep the existing children (FilterSection, FilterDuplicateSection, the
action Row with onReset and onApplyClick) unchanged.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheetItem.kt`:
- Around line 3-12: The composable that renders the filter chip (e.g.,
FilterBottomSheetItem) only uses visual cues (border and text color) and lacks
accessibility state; update the Composable to set semantic selection state (use
Modifier.semantics { selected = isSelected } or semantics(mergeDescendants =
true) { selected = isSelected }) on the clickable/Surface/Text container so
TalkBack receives the selected/unselected state; place the semantics modifier
alongside the existing Modifier chain (before clickable if present) and ensure
the boolean property (e.g., isSelected or selected) provided to the Composable
is used for the semantics selected value.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt`:
- Around line 43-70: Wrap the BasicTextField modifier with semantics that expose
a stable accessibility label and current value (e.g.,
Modifier.weight(1f).semantics(mergeDescendants = true) { contentDescription =
listOf(hint); text = AnnotatedString(value) }) so TalkBack always announces this
as the search field both before and after input, and make the trailing Icon
decorative by setting its contentDescription to null; update
ItemSearchTextField’s BasicTextField modifier and the Icon invocation
accordingly.
---
Nitpick comments:
In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/Constant.kt`:
- Line 3: The public top-level constant TEXT_FIELD_MAX_LENGTH in Constant.kt
lacks an explicit type; update its declaration to specify the Int type (e.g.,
change TEXT_FIELD_MAX_LENGTH = 255 to TEXT_FIELD_MAX_LENGTH: Int = 255) so the
public API has an explicit type annotation.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanFilterChip.kt`:
- Around line 26-29: The Surface onClick currently wraps the passed-in onClick
by creating a new lambda (Surface(onClick = { onClick() })), causing unnecessary
allocations; change it to pass the callback directly (Surface(onClick =
onClick)) and ensure the CallvanFilterChip composable's onClick parameter is of
type () -> Unit so no outer variables are captured inside a new lambda.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt`:
- Around line 72-75: Change the unsafe downcasts by making the filter section
components and callbacks generic over the specific CallvanFilterType subtype
instead of using the common CallvanFilterType and casting; i.e., convert
FilterSection and FilterDuplicateSection to generics like FilterSection<T :
CallvanFilterType> and update FilterBottomSheetContent’s callback signatures
(onSortTypeChange, onStatusesTypeChange, onArrivalsTypeChange, etc.) to accept
the concrete generic T so callers receive SortType / StatusesType /
ArrivalsFilterType directly; then propagate the generics through the calling
site so assignments like selectedSortType = it no longer require as-casts and
the compiler enforces type safety.
- Around line 49-55: 현재 FilterBottomSheet의 여러 개의 분산된 파라미터(selectedSortType,
selectedStatusesType, selectedArrivalsType, selectedDeparturesType, onApply)를
하나의 불변 상태 객체로 묶어 전달하도록 변경하세요: 정의할 `@Immutable` 데이터 클래스(e.g., FilterState)에
SortType, StatusesType, ImmutableList<ArrivalsFilterType>,
ImmutableList<DeparturesFilterType>를 포함시키고 FilterBottomSheet 시그니처는 해당 상태 객체와 액션
콜백(onFilterChange 또는 onApplyFilter 같은 단일 콜백)을 받도록 바꿉니다; 기존 selected* 파라미터와
onApply 사용처(호출부)를 모두 해당 상태 객체와 단일 콜백으로 교체하여 호출부가 간결해지고 인자 순서 실수와 Sonar 경고를
제거하세요.
In `@feature/callvan/src/main/res/values/strings.xml`:
- Line 59: The resource key filter_list_adapt is semantically misleading for the
displayed string "적용하기"; rename the resource to filter_list_apply and update all
references: replace <string name="filter_list_adapt"> with <string
name="filter_list_apply"> in strings.xml, then update every XML reference
(`@string/filter_list_adapt`) and Java/Kotlin usages (R.string.filter_list_adapt
or getString(R.string.filter_list_adapt)) to use filter_list_apply; keep the
string value unchanged and run a project-wide search/replace and rebuild to
ensure no unresolved references remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fa304fdd-cfee-4808-b717-ace773482954
📒 Files selected for processing (10)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/Constant.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/enums/CallvanFilterType.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanFilterChip.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheetItem.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.ktfeature/callvan/src/main/res/drawable/ic_list_filter.xmlfeature/callvan/src/main/res/drawable/ic_process.xmlfeature/callvan/src/main/res/drawable/ic_search_vector.xmlfeature/callvan/src/main/res/values/strings.xml
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/enums/CallvanFilterType.kt
Show resolved
Hide resolved
...llvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt
Show resolved
Hide resolved
...n/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheetItem.kt
Show resolved
Hide resolved
Comment on lines
+43
to
+70
| BasicTextField( | ||
| modifier = Modifier.weight(1f), | ||
| value = value, | ||
| textStyle = textStyle, | ||
| singleLine = true, | ||
| onValueChange = { | ||
| if (it.length < maxLength) { | ||
| onValueChange(it) | ||
| } else { | ||
| onValueChange(it.take(maxLength)) | ||
| } | ||
| }, | ||
| decorationBox = { innerTextField -> | ||
| Box { | ||
| if (value.isEmpty()) { | ||
| Text( | ||
| text = hint, | ||
| color = KoinTheme.colors.neutral600 | ||
| ) | ||
| } | ||
| innerTextField() | ||
| } | ||
| } | ||
| ) | ||
| Icon( | ||
| imageVector = ImageVector.vectorResource(R.drawable.ic_search_vector), | ||
| contentDescription = "" | ||
| ) |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
find . -path "*/feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt" -type fRepository: BCSDLab/KOIN_ANDROID
Repository: BCSDLab/KOIN_ANDROID
Exit code: 0
stdout:
./feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt
Script executed:
fd "ItemSearchTextField.kt" --type fRepository: BCSDLab/KOIN_ANDROID
Repository: BCSDLab/KOIN_ANDROID
Exit code: 0
stdout:
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt
feature/lostandfound/src/main/java/in/koreatech/koin/feature/lostandfound/ui/list/component/ItemSearchTextField.kt
Script executed:
cat -n "feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt"Repository: BCSDLab/KOIN_ANDROID
Repository: BCSDLab/KOIN_ANDROID
Exit code: 0
stdout:
1 package `in`.koreatech.koin.feature.callvan.ui.list.component
2
3 import androidx.compose.foundation.background
4 import androidx.compose.foundation.layout.Box
5 import androidx.compose.foundation.layout.Row
6 import androidx.compose.foundation.layout.fillMaxWidth
7 import androidx.compose.foundation.layout.padding
8 import androidx.compose.foundation.text.BasicTextField
9 import androidx.compose.material3.Icon
10 import androidx.compose.material3.Text
11 import androidx.compose.runtime.Composable
12 import androidx.compose.ui.Alignment
13 import androidx.compose.ui.Modifier
14 import androidx.compose.ui.graphics.vector.ImageVector
15 import androidx.compose.ui.res.stringResource
16 import androidx.compose.ui.res.vectorResource
17 import androidx.compose.ui.text.TextStyle
18 import androidx.compose.ui.tooling.preview.Preview
19 import androidx.compose.ui.unit.dp
20 import `in`.koreatech.koin.core.designsystem.theme.KoinTheme
21 import `in`.koreatech.koin.feature.callvan.R
22 import `in`.koreatech.koin.feature.callvan.TEXT_FIELD_MAX_LENGTH
23
24 `@Composable`
25 fun ItemSearchTextField(
26 value: String,
27 modifier: Modifier = Modifier,
28 maxLength: Int = TEXT_FIELD_MAX_LENGTH,
29 hint: String = stringResource(R.string.callvan_search_hint),
30 textStyle: TextStyle = KoinTheme.typography.regular14.copy(color = KoinTheme.colors.neutral600),
31 onValueChange: (String) -> Unit = {}
32 ) {
33 Row(
34 modifier = modifier
35 .fillMaxWidth()
36 .background(
37 color = KoinTheme.colors.neutral100,
38 shape = KoinTheme.shapes.extraSmall
39 )
40 .padding(vertical = 8.dp, horizontal = 16.dp),
41 verticalAlignment = Alignment.CenterVertically
42 ) {
43 BasicTextField(
44 modifier = Modifier.weight(1f),
45 value = value,
46 textStyle = textStyle,
47 singleLine = true,
48 onValueChange = {
49 if (it.length < maxLength) {
50 onValueChange(it)
51 } else {
52 onValueChange(it.take(maxLength))
53 }
54 },
55 decorationBox = { innerTextField ->
56 Box {
57 if (value.isEmpty()) {
58 Text(
59 text = hint,
60 color = KoinTheme.colors.neutral600
61 )
62 }
63 innerTextField()
64 }
65 }
66 )
67 Icon(
68 imageVector = ImageVector.vectorResource(R.drawable.ic_search_vector),
69 contentDescription = ""
70 )
71 }
72 }
73
74 `@Preview`(showBackground = true, backgroundColor = 0xFFFFFFFF)
75 `@Composable`
76 private fun ItemSearchTextFieldPreview() {
77 ItemSearchTextField(
78 value = "",
79 hint = "검색어를 입력해주세요.",
80 onValueChange = {}
81 )
82 }
검색 입력창에 접근성 라벨이 없습니다.
placeholder Text는 BasicTextField의 접근성 라벨이 아니어서, 값이 입력된 뒤에는 TalkBack이 이 필드가 검색창이라는 정보를 잃습니다. trailing icon도 장식용이면 contentDescription = null로 처리하는 편이 맞습니다. 필드 자체에 라벨/힌트 semantics를 추가해 주세요. 입력 전/후 둘 다 TalkBack이 검색 용도를 읽는지도 같이 확인해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt`
around lines 43 - 70, Wrap the BasicTextField modifier with semantics that
expose a stable accessibility label and current value (e.g.,
Modifier.weight(1f).semantics(mergeDescendants = true) { contentDescription =
listOf(hint); text = AnnotatedString(value) }) so TalkBack always announces this
as the search field both before and after input, and make the trailing Icon
decorative by setting its contentDescription to null; update
ItemSearchTextField’s BasicTextField modifier and the Icon invocation
accordingly.
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt (1)
122-132:FilterBottomSheetContent파라미터 수를 줄여 호출 안정성을 높여주세요.현재 상태/이벤트를 개별 인자로 10개 전달하고 있어, 호출부(Line 69 이후)에서 인자 누락/순서 실수 위험이 큽니다.
state+actions객체로 묶는 쪽이 유지보수에 유리합니다.♻️ 제안 리팩터링
+@Immutable +data class FilterBottomSheetContentState( + val selectedSortType: SortType, + val selectedStatusesType: StatusesType, + val selectedDeparturesType: ImmutableList<DeparturesFilterType>, + val selectedArrivalsType: ImmutableList<ArrivalsFilterType>, +) + +data class FilterBottomSheetContentActions( + val onSortTypeChange: (CallvanFilterType) -> Unit, + val onStatusesTypeChange: (CallvanFilterType) -> Unit, + val onDeparturesTypeChange: (ImmutableList<CallvanFilterType>) -> Unit, + val onArrivalsTypeChange: (ImmutableList<CallvanFilterType>) -> Unit, + val onReset: () -> Unit, + val onApplyClick: () -> Unit, +) + `@Composable` fun FilterBottomSheetContent( - selectedSortType: SortType, - selectedStatusesType: StatusesType, - selectedDeparturesType: ImmutableList<DeparturesFilterType>, - selectedArrivalsType: ImmutableList<ArrivalsFilterType>, - onSortTypeChange: (CallvanFilterType) -> Unit, - onStatusesTypeChange: (CallvanFilterType) -> Unit, - onDeparturesTypeChange: (ImmutableList<CallvanFilterType>) -> Unit, - onArrivalsTypeChange: (ImmutableList<CallvanFilterType>) -> Unit, - onReset: () -> Unit, - onApplyClick: () -> Unit + state: FilterBottomSheetContentState, + actions: FilterBottomSheetContentActions, ) {Also applies to: 69-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt` around lines 122 - 132, The FilterBottomSheetContent signature passes many separate parameters causing fragile calls; refactor it to accept a single immutable state data class (e.g., FilterBottomSheetState containing selectedSortType, selectedStatusesType, selectedDeparturesType, selectedArrivalsType) and a single actions interface/data class (e.g., FilterBottomSheetActions with onSortTypeChange, onStatusesTypeChange, onDeparturesTypeChange, onArrivalsTypeChange, onReset, onApplyClick) and update the FilterBottomSheetContent function to use these two objects; then update all call sites to build and pass a FilterBottomSheetState and FilterBottomSheetActions instead of the 10 separate parameters (ensure names match the existing symbols: SortType, StatusesType, DeparturesFilterType, ArrivalsFilterType and the onX callbacks) so invocation becomes stable and less error-prone.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt`:
- Around line 122-132: The FilterBottomSheetContent signature passes many
separate parameters causing fragile calls; refactor it to accept a single
immutable state data class (e.g., FilterBottomSheetState containing
selectedSortType, selectedStatusesType, selectedDeparturesType,
selectedArrivalsType) and a single actions interface/data class (e.g.,
FilterBottomSheetActions with onSortTypeChange, onStatusesTypeChange,
onDeparturesTypeChange, onArrivalsTypeChange, onReset, onApplyClick) and update
the FilterBottomSheetContent function to use these two objects; then update all
call sites to build and pass a FilterBottomSheetState and
FilterBottomSheetActions instead of the 10 separate parameters (ensure names
match the existing symbols: SortType, StatusesType, DeparturesFilterType,
ArrivalsFilterType and the onX callbacks) so invocation becomes stable and less
error-prone.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4ef9dea1-0a98-4443-98a9-9f9558030a2b
📒 Files selected for processing (5)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/enums/CallvanFilterType.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheetItem.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.ktfeature/callvan/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (3)
- feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheetItem.kt
- feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt
- feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/enums/CallvanFilterType.kt
…r-and-search' into feature/#1283-callvan-list-filter-and-search
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt (1)
61-64: 파라미터와 동일한 이름의 로컬 변수 사용 - 가독성 저하파라미터 이름(
selectedSortType,selectedStatusesType등)과 동일한 이름의 로컬var변수를 선언하면 shadowing이 발생하여 코드 가독성이 떨어지고 혼란을 줄 수 있습니다. 구분을 위해 다른 네이밍을 권장합니다.♻️ 제안된 수정
- var selectedSortType by remember { mutableStateOf(selectedSortType) } - var selectedStatusesType by remember { mutableStateOf(selectedStatusesType) } - var selectedArrivalsType by remember { mutableStateOf(selectedArrivalsType) } - var selectedDeparturesType by remember { mutableStateOf(selectedDeparturesType) } + var currentSortType by remember { mutableStateOf(selectedSortType) } + var currentStatusesType by remember { mutableStateOf(selectedStatusesType) } + var currentArrivalsType by remember { mutableStateOf(selectedArrivalsType) } + var currentDeparturesType by remember { mutableStateOf(selectedDeparturesType) }이후 해당 변수들을 사용하는 모든 곳(lines 72-119)에서도 변수명을 업데이트해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt` around lines 61 - 64, The local vars in FilterBottomSheet are shadowing the function parameters (selectedSortType, selectedStatusesType, selectedArrivalsType, selectedDeparturesType); rename the remembered state variables (e.g., selectedSortTypeState, selectedStatusesState, selectedArrivalsState, selectedDeparturesState) and update every usage within the composable (uses around lines 72-119) so references use the new names; ensure you keep the initial values from the parameters when calling remember { mutableStateOf(...) } and adjust any reads/writes and callbacks that refer to these symbols.feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetActions.kt (1)
6-13: 콜백 파라미터 타입을 더 구체적인 타입으로 변경 권장
onSortTypeChange와onStatusesTypeChange가CallvanFilterType을 받지만, 실제 사용처(FilterBottomSheet.kt 79-80줄)에서it as SortType,it as StatusesType으로 unsafe cast를 수행합니다. 타입 안전성을 위해 구체적인 타입 사용을 권장합니다.♻️ 제안된 수정
data class FilterBottomSheetActions( - val onSortTypeChange: (CallvanFilterType) -> Unit, - val onStatusesTypeChange: (CallvanFilterType) -> Unit, + val onSortTypeChange: (CallvanFilterType.SortType) -> Unit, + val onStatusesTypeChange: (CallvanFilterType.StatusesType) -> Unit, val onDeparturesTypeChange: (ImmutableList<CallvanFilterType>) -> Unit, val onArrivalsTypeChange: (ImmutableList<CallvanFilterType>) -> Unit, val onReset: () -> Unit, val onApplyClick: () -> Unit )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetActions.kt` around lines 6 - 13, The callbacks onSortTypeChange and onStatusesTypeChange in the FilterBottomSheetActions data class use the broad CallvanFilterType causing unsafe casts; change their parameter types to the concrete types (SortType) and (StatusesType) respectively in the FilterBottomSheetActions declaration, then update all places that construct or invoke these callbacks (e.g., where FilterBottomSheetActions is instantiated and where the callbacks are called) to accept and pass the concrete types without casting, and add any necessary imports for SortType and StatusesType so compilation and type-safety are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetState.kt`:
- Around line 9-14: Annotate the FilterBottomSheetState data class with
`@Immutable` and provide default values for all properties: set selectedSortType
to a sensible default like SortType.values().first(), selectedStatusesType to
StatusesType.values().first(), and set selectedDeparturesType and
selectedArrivalsType to empty immutable lists (e.g., persistentListOf()) so they
are ImmutableList defaults; import androidx.compose.runtime.Immutable and
kotlinx.collections.immutable.persistentListOf (or the project’s ImmutableList
factory) and update the FilterBottomSheetState declaration accordingly.
---
Nitpick comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt`:
- Around line 61-64: The local vars in FilterBottomSheet are shadowing the
function parameters (selectedSortType, selectedStatusesType,
selectedArrivalsType, selectedDeparturesType); rename the remembered state
variables (e.g., selectedSortTypeState, selectedStatusesState,
selectedArrivalsState, selectedDeparturesState) and update every usage within
the composable (uses around lines 72-119) so references use the new names;
ensure you keep the initial values from the parameters when calling remember {
mutableStateOf(...) } and adjust any reads/writes and callbacks that refer to
these symbols.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetActions.kt`:
- Around line 6-13: The callbacks onSortTypeChange and onStatusesTypeChange in
the FilterBottomSheetActions data class use the broad CallvanFilterType causing
unsafe casts; change their parameter types to the concrete types (SortType) and
(StatusesType) respectively in the FilterBottomSheetActions declaration, then
update all places that construct or invoke these callbacks (e.g., where
FilterBottomSheetActions is instantiated and where the callbacks are called) to
accept and pass the concrete types without casting, and add any necessary
imports for SortType and StatusesType so compilation and type-safety are
preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 93da22ea-a3cc-48ac-ac60-59d4da6d7d21
📒 Files selected for processing (3)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetActions.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetState.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt
...re/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetState.kt
Show resolved
Hide resolved
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt (1)
215-221: Modifier 파라미터 추가 권장재사용성과 유연성을 높이기 위해
modifier: Modifier = Modifier파라미터를 추가하는 것을 권장합니다.♻️ Modifier 파라미터 추가 제안
`@Composable` fun <T : CallvanFilterType> FilterSection( title: String, items: ImmutableList<T>, selectedItem: T, + modifier: Modifier = Modifier, onItemSelected: (T) -> Unit ) { - Column(modifier = Modifier.padding(vertical = 12.dp)) { + Column(modifier = modifier.padding(vertical = 12.dp)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt` around lines 215 - 221, The FilterSection composable lacks a Modifier parameter which limits reuse and layout control; update the function signature for FilterSection to accept a modifier: Modifier = Modifier (and pass it through to the root composable inside the function), adjust any call sites to provide or omit the modifier as needed, and ensure the parameter is used on the top-level container (e.g., Column/Box) so callers can customize spacing, alignment, or click behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt`:
- Around line 276-284: The click handler in FilterBottomSheet.kt incorrectly
adds allItem to existing selections instead of replacing them; update the
onClick logic in the block handling selectedItems/allItem so that when item ==
allItem you set newSelection to a persistent list containing only allItem
(onItemSelected(persistentListOf(allItem))); otherwise preserve the existing
behavior for toggling non-all items but ensure adding a non-all item also
removes allItem (i.e., when adding item remove allItem from selectedItems before
adding). Target the onClick lambda that builds newSelection, and use
selectedItems, allItem and onItemSelected to implement this replacement
behavior.
---
Nitpick comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt`:
- Around line 215-221: The FilterSection composable lacks a Modifier parameter
which limits reuse and layout control; update the function signature for
FilterSection to accept a modifier: Modifier = Modifier (and pass it through to
the root composable inside the function), adjust any call sites to provide or
omit the modifier as needed, and ensure the parameter is used on the top-level
container (e.g., Column/Box) so callers can customize spacing, alignment, or
click behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e12a0a35-4911-4f0b-8092-ad9ddf6873d4
📒 Files selected for processing (2)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetActions.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetActions.kt
...llvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt
Show resolved
Hide resolved
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetActions.kt (2)
6-13: Compose 안정성을 위해@Stable어노테이션 추가를 권장합니다.
data class에 람다 프로퍼티가 포함되어 있으면equals()비교 시 람다가 참조 비교되어 불필요한 리컴포지션이 발생할 수 있습니다.@Stable어노테이션을 추가하면 Compose가 이 클래스를 안정적으로 처리하여 최적화에 도움이 됩니다.♻️ 수정 제안
package `in`.koreatech.koin.feature.callvan.ui.list +import androidx.compose.runtime.Stable import kotlinx.collections.immutable.ImmutableList import `in`.koreatech.koin.feature.callvan.enums.CallvanFilterType +@Stable data class FilterBottomSheetActions( val onSortTypeChange: (CallvanFilterType.SortType) -> Unit, val onStatusesTypeChange: (CallvanFilterType.StatusesType) -> Unit, val onDeparturesTypeChange: (ImmutableList<CallvanFilterType.DeparturesFilterType>) -> Unit, val onArrivalsTypeChange: (ImmutableList<CallvanFilterType.ArrivalsFilterType>) -> Unit, val onReset: () -> Unit, val onApplyClick: () -> Unit )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetActions.kt` around lines 6 - 13, Add the `@Stable` annotation to the FilterBottomSheetActions data class to mark it as stable for Compose; specifically, annotate the class declaration for FilterBottomSheetActions (which contains the lambda properties onSortTypeChange, onStatusesTypeChange, onDeparturesTypeChange, onArrivalsTypeChange, onReset, and onApplyClick) and add the corresponding import for androidx.compose.runtime.Stable.
3-4: import 순서가 가이드라인과 맞지 않습니다.가이드라인에 따르면 import 순서는 Android > libraries > project imports 순이어야 합니다. 현재 프로젝트 import가 라이브러리 import보다 먼저 선언되어 있습니다.
♻️ 수정 제안
package `in`.koreatech.koin.feature.callvan.ui.list -import `in`.koreatech.koin.feature.callvan.enums.CallvanFilterType import kotlinx.collections.immutable.ImmutableList +import `in`.koreatech.koin.feature.callvan.enums.CallvanFilterType🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetActions.kt` around lines 3 - 4, 현재 FilterBottomSheetActions.kt의 import 순서가 가이드라인(Android > libraries > project imports)과 맞지 않습니다: project import (`in`.koreatech.koin.feature.callvan.enums.CallvanFilterType) 이 라이브러리 import(kotlinx.collections.immutable.ImmutableList)보다 먼저 선언되어 있습니다. 해당 파일의 import 순서를 가이드라인대로 재정렬하여 Android 관련 import가 최상단, 그 다음 외부 라이브러리(import kotlinx.collections.immutable.ImmutableList), 마지막으로 프로젝트 내부 import(import `in`.koreatech.koin.feature.callvan.enums.CallvanFilterType)가 오도록 바꾸고, 변경 후 spotless/ktlint 형식 검사를 통과하도록 커밋하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetActions.kt`:
- Around line 6-13: Add the `@Stable` annotation to the FilterBottomSheetActions
data class to mark it as stable for Compose; specifically, annotate the class
declaration for FilterBottomSheetActions (which contains the lambda properties
onSortTypeChange, onStatusesTypeChange, onDeparturesTypeChange,
onArrivalsTypeChange, onReset, and onApplyClick) and add the corresponding
import for androidx.compose.runtime.Stable.
- Around line 3-4: 현재 FilterBottomSheetActions.kt의 import 순서가 가이드라인(Android >
libraries > project imports)과 맞지 않습니다: project import
(`in`.koreatech.koin.feature.callvan.enums.CallvanFilterType) 이 라이브러리
import(kotlinx.collections.immutable.ImmutableList)보다 먼저 선언되어 있습니다. 해당 파일의
import 순서를 가이드라인대로 재정렬하여 Android 관련 import가 최상단, 그 다음 외부 라이브러리(import
kotlinx.collections.immutable.ImmutableList), 마지막으로 프로젝트 내부 import(import
`in`.koreatech.koin.feature.callvan.enums.CallvanFilterType)가 오도록 바꾸고, 변경 후
spotless/ktlint 형식 검사를 통과하도록 커밋하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0427e2a1-883f-4d81-9b76-123d7848167f
📒 Files selected for processing (2)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/FilterBottomSheetActions.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/FilterBottomSheet.kt
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Member
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt (1)
43-64:⚠️ Potential issue | 🟠 Major검색 필드의 접근성 라벨이 여전히 고정적으로 노출되지 않습니다.
Line 44의
BasicTextField는 placeholder 텍스트만으로는 입력 후 검색 필드 의미 전달이 약해질 수 있습니다. 필드 자체에 semantics 라벨을 명시해 주세요. (기존 리뷰와 동일 이슈)🔧 제안 수정안
import androidx.compose.ui.Modifier +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semantics @@ BasicTextField( - modifier = Modifier.weight(1f), + modifier = Modifier + .weight(1f) + .semantics { contentDescription = hint }, value = value,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt` around lines 43 - 64, The search input (BasicTextField in ItemSearchTextField) lacks an explicit semantics label so screen readers may not announce the field meaning after typing; add a semantics modifier to the BasicTextField (e.g., Modifier.weight(1f).semantics(mergeDescendants = false) { contentDescription = hint } or use clearAndSetSemantics/mergeDescendants as appropriate) so the field's contentDescription is set from the hint (or a dedicated label string) ensuring accessibility tools announce the input as a search field even when it contains text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt`:
- Around line 28-49: ItemSearchTextField can throw IllegalArgumentException when
maxLength is negative because of take(maxLength) inside the BasicTextField
onValueChange; validate or sanitize maxLength at the start of
ItemSearchTextField (e.g., val safeMax = maxLength.coerceAtLeast(0)) and use
that safeMax in the onValueChange call instead of raw maxLength so negative
inputs are clamped to 0 and runtime exceptions are prevented; update any
relevant usages/comments to reflect the sanitized value and keep the public API
intact.
---
Duplicate comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt`:
- Around line 43-64: The search input (BasicTextField in ItemSearchTextField)
lacks an explicit semantics label so screen readers may not announce the field
meaning after typing; add a semantics modifier to the BasicTextField (e.g.,
Modifier.weight(1f).semantics(mergeDescendants = false) { contentDescription =
hint } or use clearAndSetSemantics/mergeDescendants as appropriate) so the
field's contentDescription is set from the hint (or a dedicated label string)
ensuring accessibility tools announce the input as a search field even when it
contains text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 29efc62c-aa77-4122-b46d-29099c7426f6
📒 Files selected for processing (1)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt
Comment on lines
+28
to
+49
| maxLength: Int = TEXT_FIELD_MAX_LENGTH, | ||
| hint: String = stringResource(R.string.callvan_search_hint), | ||
| textStyle: TextStyle = KoinTheme.typography.regular14.copy(color = KoinTheme.colors.neutral600), | ||
| onValueChange: (String) -> Unit = {} | ||
| ) { | ||
| Row( | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .background( | ||
| color = KoinTheme.colors.neutral100, | ||
| shape = KoinTheme.shapes.extraSmall | ||
| ) | ||
| .padding(vertical = 8.dp, horizontal = 16.dp), | ||
| verticalAlignment = Alignment.CenterVertically | ||
| ) { | ||
| BasicTextField( | ||
| modifier = Modifier.weight(1f), | ||
| value = value, | ||
| textStyle = textStyle, | ||
| singleLine = true, | ||
| onValueChange = { onValueChange(it.take(maxLength)) }, | ||
| decorationBox = { innerTextField -> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the complete ItemSearchTextField.kt file
fd -t f "ItemSearchTextField.kt" --exec cat -n {} \;Repository: BCSDLab/KOIN_ANDROID
Length of output: 6318
🏁 Script executed:
# Also search for the TEXT_FIELD_MAX_LENGTH constant definition
rg "TEXT_FIELD_MAX_LENGTH" -t kt -B2 -A2Repository: BCSDLab/KOIN_ANDROID
Length of output: 90
🌐 Web query:
Kotlin String.take() negative value behavior
💡 Result:
String.take(n) (and CharSequence.take(n)) throws IllegalArgumentException when n is negative. If n is 0, it returns ""; if n is greater than the string length, it returns the whole string. [1]
Source: Kotlin stdlib docs for kotlin.text.take. [1]
References:
[1] https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.text/take.html
maxLength이 음수일 때 Line 48에서 런타임 예외가 발생합니다.
take(maxLength)는 음수 값이 전달되면 IllegalArgumentException을 던집니다. 공개 컴포넌트 파라미터이므로 입력값 검증이 필요합니다.
🔧 제안 수정안
`@Composable`
fun ItemSearchTextField(
value: String,
modifier: Modifier = Modifier,
maxLength: Int = TEXT_FIELD_MAX_LENGTH,
hint: String = stringResource(R.string.callvan_search_hint),
textStyle: TextStyle = KoinTheme.typography.regular14.copy(color = KoinTheme.colors.neutral600),
onValueChange: (String) -> Unit = {}
) {
+ val safeMaxLength = maxLength.coerceAtLeast(0)
+
Row(
modifier = modifier
.fillMaxWidth()
.background(
color = KoinTheme.colors.neutral100,
shape = KoinTheme.shapes.extraSmall
)
.padding(vertical = 8.dp, horizontal = 16.dp),
verticalAlignment = Alignment.CenterVertically
) {
BasicTextField(
modifier = Modifier.weight(1f),
value = value,
textStyle = textStyle,
singleLine = true,
- onValueChange = { onValueChange(it.take(maxLength)) },
+ onValueChange = { onValueChange(it.take(safeMaxLength)) },
decorationBox = { innerTextField ->🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/ItemSearchTextField.kt`
around lines 28 - 49, ItemSearchTextField can throw IllegalArgumentException
when maxLength is negative because of take(maxLength) inside the BasicTextField
onValueChange; validate or sanitize maxLength at the start of
ItemSearchTextField (e.g., val safeMax = maxLength.coerceAtLeast(0)) and use
that safeMax in the onValueChange call instead of raw maxLength so negative
inputs are clamped to 0 and runtime exceptions are prevented; update any
relevant usages/comments to reflect the sanitized value and keep the public API
intact.
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
2 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.
PR 개요
이슈 번호:
PR 체크리스트
작업사항
작업사항의 상세한 설명
리스트의 필터와 검색 기능 UI 구현 pr입니다
작업 사항이 이어지는 pr이라 브랜치에서 다시 브랜치를 파는 방식으로 진행했는데, 해당 방식으로 진행하니 conflict이 생기거나 Review skipped가 나타나는 것 같아 pr을 다시 올립니다
논의 사항
스크린샷
추가내용
Summary by CodeRabbit