[Feature] 콜밴 리스트 아이템 ui 구현#1361
[Feature] 콜밴 리스트 아이템 ui 구현#1361TTRR1007 wants to merge 7 commits intofeature/#1283-callvan-list-filter-and-searchfrom
Conversation
WalkthroughCallvan 기능에 enum, UI 구성요소(참여자 수, 리스트 아이템, 상태별 버튼), UI 상태 모델 및 콜백 클래스, 벡터 드로어블과 문자열 리소스를 추가하고 CallvanDetailScreen에서 참여자 수 UI를 재사용 컴포넌트로 교체했습니다. Changes
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 Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@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: 2
🧹 Nitpick comments (4)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/component/CallvanParticipantCount.kt (1)
49-65: Preview 함수명 용어를Participant로 통일하면 가독성이 더 좋아집니다.현재
CallvanPersonCount...는 본 컴포넌트명(CallvanParticipantCount)과 용어가 달라 검색성과 일관성이 조금 떨어집니다.🤖 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/component/CallvanParticipantCount.kt` around lines 49 - 65, Rename the two preview Composable functions to use "Participant" to match the component name: change CallvanPersonCountPreview -> CallvanParticipantCountPreview and CallvanPersonCountFullPreview -> CallvanParticipantCountFullPreview; update any local references/usages to these function names, keeping the `@Preview` and `@Composable` annotations and the existing CallvanParticipantCount(...) calls intact so searchability and naming consistency with the CallvanParticipantCount component are restored.feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/enums/CallvanRouteState.kt (1)
3-9: 중앙화된 상태 매핑 팩토리를 추가하여 분산된 변환 로직을 통합하세요.현재 enum 정의만 있고, 도메인 모델의
status/isJoined/isAuthor속성에서 이 상태로 변환하는 로직이 보이지 않습니다. 리스트 화면 구현이 완성되면 다음과 같이 팩토리 메서드를 통해 상태 변환을 중앙화하는 것을 권장합니다.권장 패턴: enum 내부 팩토리로 매핑 중앙화
enum class CallvanRouteState { DEFAULT, JOINED, CLOSED, OWNER_ACTIVE, - OWNER_CLOSED + OWNER_CLOSED; + + companion object { + fun from(status: String, isJoined: Boolean, isAuthor: Boolean): CallvanRouteState = when { + isAuthor && status.equals("CLOSED", ignoreCase = true) -> OWNER_CLOSED + isAuthor -> OWNER_ACTIVE + status.equals("CLOSED", ignoreCase = true) -> CLOSED + isJoined -> JOINED + else -> DEFAULT + } + } }리스트 화면 구현 시 CallvanPost 데이터를 UI에 전달할 때 이 팩토리를 사용하여
CallvanRouteState.from(post.status, post.isJoined, post.isAuthor)로 변환하면 화면 분기 불일치를 방지할 수 있습니다.🤖 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/enums/CallvanRouteState.kt` around lines 3 - 9, 현재 CallvanRouteState enum에 도메인 속성(status/isJoined/isAuthor)에서 상태로 변환하는 중앙화된 로직이 없어 분산된 변환으로 일관성 문제가 발생할 수 있습니다; 수정 방법은 CallvanRouteState 내부에 정적 팩토리 메서드 from(status: String, isJoined: Boolean, isAuthor: Boolean) 또는 from(post: CallvanPost)를 추가하여 입력 값에 따른 분기(예: isAuthor -> OWNER_ACTIVE/OWNER_CLOSED, isJoined -> JOINED, status 기반 DEFAULT/CLOSED 등)를 하나의 장소에서 처리하도록 하고, UI 리스트 코드에서는 직접 조건을 검사하지 말고 항상 CallvanRouteState.from(...)을 호출하도록 변경하세요.feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.kt (1)
25-29: 필수 파라미터 순서를 가이드에 맞춰 주세요.
clickListener가 필수값인데modifier뒤에 있어 이 컴포넌트만 호출부 형태가 달라집니다.필수 파라미터 → modifier → 기본값 파라미터순서로 맞추는 편이 좋겠습니다.As per coding guidelines, "@composable 함수의 파라미터는 기본값이 없는 필수 파라미터, modifier, 기본값이 있는 파라미터 순서로 선언, 기본값이 없는 lambda 는 최하단에 선언해도 통과".♻️ 제안 수정안
fun CallvanListItem( uiState: CallvanListUiState, state: CallvanRouteState, - modifier: Modifier = Modifier, - clickListener: CallvanListItemClickListener + clickListener: CallvanListItemClickListener, + modifier: Modifier = Modifier, ) {🤖 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/CallvanListItem.kt` around lines 25 - 29, The CallvanListItem parameter order is wrong: move the required clickListener parameter before the modifier so parameters follow the rule (non-default required params, modifier, defaulted params); update the CallvanListItem signature (reference: CallvanListItem(uiState: CallvanListUiState, state: CallvanRouteState, clickListener: CallvanListItemClickListener, modifier: Modifier = Modifier, ...)) and ensure any other defaulted parameters remain after modifier—note that a non-default lambda may remain at the bottom per guideline.feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItemButtons.kt (1)
44-119: 상태별 UI 분기를 한 곳으로 모아두는 편이 안전합니다.같은
state를 상단 아이콘, 보조 버튼, 메인 버튼에서 각각 다시 분기하고else경로도 따로 두고 있어서, 상태 규칙이 바뀌면 조합이 쉽게 어긋나거나 새 상태가 조용히 빈 UI로 내려갈 수 있습니다. 상태별 표시 모델을 먼저 만들고 렌더링을 한 번만 분기하도록 정리하면 유지보수가 훨씬 안전해집니다.🤖 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/CallvanListItemButtons.kt` around lines 44 - 119, The UI branches for CallvanRouteState are duplicated across the icon block, the secondary CallvanOutlinedButton block, and the primary CallvanFilledButton block; centralize by computing a single per-state presentation model (e.g., a small data class or sealed type) keyed on CallvanRouteState that contains: icon type and click action (used by the IconButton), optional secondary button config (text, onClick, enabled/content/border options) for CallvanOutlinedButton, and optional primary button config (text, onClick, contentPadding) for CallvanFilledButton; replace the three separate when(state) blocks with one when(state) that produces the model and then render IconButton, CallvanOutlinedButton, and CallvanFilledButton based solely on that model, wiring clickListener methods (onChat, onCall, onCancelJoin, onClose, onReRecruit, onJoin, onComplete) into the model so behavior stays identical.
🤖 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/CallvanListItem.kt`:
- Around line 31-66: Reorder the CallvanListItem composable parameters so
required params come first (uiState, state, clickListener) and modifier is last
to follow Compose parameter conventions (update the function signature
referencing CallvanListItem, uiState, state, clickListener, modifier). To
prevent layout overflow, give the left Column a weight and end spacing by
applying modifier.weight(1f).padding(end = 16.dp) (update the Column call that
contains CallvanRouteInfo and CallvanParticipantCount), and add the import
androidx.compose.foundation.layout.weight; verify long route text behavior after
the change.
In `@feature/callvan/src/main/res/values/strings.xml`:
- Line 91: 문구가 붙어 있어 어색하므로 string 리소스 callvan_btn_cancel_join의 값을 "참여취소"에서 "참여
취소"로 수정하세요; 구체적으로 문자열 리소스 이름 callvan_btn_cancel_join를 찾아 값(inner text)을 "참여 취소"로
변경하면 됩니다.
---
Nitpick comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/enums/CallvanRouteState.kt`:
- Around line 3-9: 현재 CallvanRouteState enum에 도메인 속성(status/isJoined/isAuthor)에서
상태로 변환하는 중앙화된 로직이 없어 분산된 변환으로 일관성 문제가 발생할 수 있습니다; 수정 방법은 CallvanRouteState 내부에
정적 팩토리 메서드 from(status: String, isJoined: Boolean, isAuthor: Boolean) 또는
from(post: CallvanPost)를 추가하여 입력 값에 따른 분기(예: isAuthor ->
OWNER_ACTIVE/OWNER_CLOSED, isJoined -> JOINED, status 기반 DEFAULT/CLOSED 등)를 하나의
장소에서 처리하도록 하고, UI 리스트 코드에서는 직접 조건을 검사하지 말고 항상 CallvanRouteState.from(...)을 호출하도록
변경하세요.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/component/CallvanParticipantCount.kt`:
- Around line 49-65: Rename the two preview Composable functions to use
"Participant" to match the component name: change CallvanPersonCountPreview ->
CallvanParticipantCountPreview and CallvanPersonCountFullPreview ->
CallvanParticipantCountFullPreview; update any local references/usages to these
function names, keeping the `@Preview` and `@Composable` annotations and the
existing CallvanParticipantCount(...) calls intact so searchability and naming
consistency with the CallvanParticipantCount component are restored.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.kt`:
- Around line 25-29: The CallvanListItem parameter order is wrong: move the
required clickListener parameter before the modifier so parameters follow the
rule (non-default required params, modifier, defaulted params); update the
CallvanListItem signature (reference: CallvanListItem(uiState:
CallvanListUiState, state: CallvanRouteState, clickListener:
CallvanListItemClickListener, modifier: Modifier = Modifier, ...)) and ensure
any other defaulted parameters remain after modifier—note that a non-default
lambda may remain at the bottom per guideline.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItemButtons.kt`:
- Around line 44-119: The UI branches for CallvanRouteState are duplicated
across the icon block, the secondary CallvanOutlinedButton block, and the
primary CallvanFilledButton block; centralize by computing a single per-state
presentation model (e.g., a small data class or sealed type) keyed on
CallvanRouteState that contains: icon type and click action (used by the
IconButton), optional secondary button config (text, onClick,
enabled/content/border options) for CallvanOutlinedButton, and optional primary
button config (text, onClick, contentPadding) for CallvanFilledButton; replace
the three separate when(state) blocks with one when(state) that produces the
model and then render IconButton, CallvanOutlinedButton, and CallvanFilledButton
based solely on that model, wiring clickListener methods (onChat, onCall,
onCancelJoin, onClose, onReRecruit, onJoin, onComplete) into the model so
behavior stays identical.
🪄 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: 75e8251b-17f9-44d5-9bba-e8e6bfc3771e
📒 Files selected for processing (10)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/enums/CallvanRouteState.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/component/CallvanParticipantCount.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/detail/CallvanDetailScreen.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItemButtons.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/model/CallvanListItemClickListener.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/model/CallvanListUiState.ktfeature/callvan/src/main/res/drawable/ic_callvan_call.xmlfeature/callvan/src/main/res/drawable/ic_callvan_chat.xmlfeature/callvan/src/main/res/values/strings.xml
...callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.kt
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.kt (1)
3-9:⚠️ Potential issue | 🔴 Critical
Modifier.weight사용에 필요한 import가 누락되어 컴파일 오류가 발생할 수 있습니다.
Modifier.weight(1f)를 사용 중인데androidx.compose.foundation.layout.weightimport가 없어 unresolved reference가 날 수 있습니다.🔧 제안 수정안
import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.weight#!/bin/bash set -euo pipefail FILE="feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.kt" echo "[1] weight 사용 위치 확인" rg -n '\.weight\s*\(' "$FILE" || true echo echo "[2] weight import 존재 여부 확인" rg -n 'import androidx\.compose\.foundation\.layout\.weight' "$FILE" || true echo echo "예상 결과:" echo "- [1]에서 .weight(...) 호출이 1개 이상 확인됨" echo "- [2]가 비어 있으면 import 누락 상태"Also applies to: 44-46
🤖 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/CallvanListItem.kt` around lines 3 - 9, The file is missing the import required for Modifier.weight which causes unresolved reference errors; locate the composable (e.g., CallvanListItem or the place using Modifier.weight) and add the import androidx.compose.foundation.layout.weight so Modifier.weight(1f) compiles properly and remove any duplicate/unused imports if necessary.
🧹 Nitpick comments (1)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.kt (1)
73-166: Preview 샘플 리터럴 중복은 상수/공용 fixture로 추출하는 편이 좋습니다.동일한 날짜/목적지 문자열이 여러 Preview에 반복되어 변경 비용이 큽니다. Preview 전용 상수로 빼면 Sonar 경고도 같이 정리됩니다.
🤖 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/CallvanListItem.kt` around lines 73 - 166, Multiple Preview composables (CallvanListItemDefaultPreview, CallvanListItemJoinedPreview, CallvanListItemClosedPreview, CallvanListItemOwnerActivePreview, CallvanListItemOwnerClosedPreview) repeat the same literals for date/time/destination/currentCount/maxCount; extract these into Preview fixtures or constants (e.g., private val PREVIEW_DATE, PREVIEW_TIME, PREVIEW_DESTINATION and/or a private function or val SAMPLE_CALLVAN_UI_STATE: CallvanListUiState) and use those in each CallvanListItem(...) call instead of duplicated literals to simplify updates and remove Sonar duplicate-literal warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.kt`:
- Around line 3-9: The file is missing the import required for Modifier.weight
which causes unresolved reference errors; locate the composable (e.g.,
CallvanListItem or the place using Modifier.weight) and add the import
androidx.compose.foundation.layout.weight so Modifier.weight(1f) compiles
properly and remove any duplicate/unused imports if necessary.
---
Nitpick comments:
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.kt`:
- Around line 73-166: Multiple Preview composables
(CallvanListItemDefaultPreview, CallvanListItemJoinedPreview,
CallvanListItemClosedPreview, CallvanListItemOwnerActivePreview,
CallvanListItemOwnerClosedPreview) repeat the same literals for
date/time/destination/currentCount/maxCount; extract these into Preview fixtures
or constants (e.g., private val PREVIEW_DATE, PREVIEW_TIME, PREVIEW_DESTINATION
and/or a private function or val SAMPLE_CALLVAN_UI_STATE: CallvanListUiState)
and use those in each CallvanListItem(...) call instead of duplicated literals
to simplify updates and remove Sonar duplicate-literal warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5546eff7-98d1-4387-86b4-aa6ceb65bd98
📒 Files selected for processing (3)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/component/CallvanParticipantCount.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.ktfeature/callvan/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/component/CallvanParticipantCount.kt
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| else -> {} | ||
| } | ||
|
|
||
| if (state == CallvanRouteState.DEFAULT || state == CallvanRouteState.OWNER_CLOSED) { |
There was a problem hiding this comment.
[Major] 상태별 분기 로직이 분산되어 유지보수성이 낮음
현재 when(state) 블록이 두 곳으로 분산되어 있고, 추가로 if (state == DEFAULT || state == OWNER_CLOSED) 조건까지 3단 분기가 이루어집니다. 같은 state에 대한 UI 구성 요소들이 뿔뿔이 흩어져 있어, 새로운 상태 추가 시 누락이 발생하기 쉽습니다.
또한 onClick 람다 내부에서 state를 다시 확인하는 패턴은 함수형 UI에서 피해야 하는 안티패턴입니다. 외부 if에서 이미 OWNER_CLOSED 상태를 확인했음에도 람다 내부에서 DEFAULT인지 재확인합니다.
개선 방법: when(state)를 단일 블록으로 통합하세요:
Column(
modifier = modifier,
horizontalAlignment = Alignment.End,
verticalArrangement = Arrangement.spacedBy(16.dp)
) {
when (state) {
CallvanRouteState.DEFAULT -> {
Spacer(modifier = Modifier.size(24.dp))
Row(horizontalArrangement = Arrangement.spacedBy(4.dp)) {
CallvanFilledButton(
text = stringResource(R.string.callvan_btn_join),
onClick = { clickListener.onJoin() }
)
}
}
CallvanRouteState.JOINED -> {
IconButton(onClick = { clickListener.onChat() }, modifier = Modifier.size(24.dp)) { /* ... */ }
Row(horizontalArrangement = Arrangement.spacedBy(4.dp)) {
CallvanOutlinedButton(
text = stringResource(R.string.callvan_btn_cancel_join),
onClick = { clickListener.onCancelJoin() }
)
}
}
CallvanRouteState.OWNER_CLOSED -> {
IconButton(onClick = { clickListener.onCall() }, modifier = Modifier.size(24.dp)) { /* ... */ }
Row(horizontalArrangement = Arrangement.spacedBy(4.dp)) {
CallvanOutlinedButton(
text = stringResource(R.string.callvan_btn_re_recruit),
onClick = { clickListener.onReRecruit() },
contentPadding = PaddingValues(horizontal = 8.dp, vertical = 4.dp)
)
CallvanFilledButton(
text = stringResource(R.string.callvan_btn_complete),
onClick = { clickListener.onComplete() },
contentPadding = PaddingValues(horizontal = 8.dp, vertical = 4.dp)
)
}
}
// ... 나머지 상태
}
}각 상태에 대한 UI 전체를 한 곳에서 파악할 수 있어 가독성과 유지보수성이 크게 향상됩니다.
| fun CallvanListItemButtons( | ||
| state: CallvanRouteState, | ||
| modifier: Modifier = Modifier, | ||
| clickListener: CallvanListItemClickListener = object : CallvanListItemClickListener {} |
There was a problem hiding this comment.
[Minor] 기본 파라미터로 익명 객체(anonymous object) 생성
object : CallvanListItemClickListener {} 형태의 기본값은 이 컴포저블이 호출될 때마다 새 인스턴스를 생성합니다. CallvanListItemClickListener는 인터페이스라 Compose 컴파일러가 안정성(stability)을 보장할 수 없어, 의도치 않은 리컴포지션을 유발할 수 있습니다.
실제로 외부에서 clickListener를 항상 명시적으로 전달하고 있다면 (CallvanListItem.kt 참고), 이 기본값은 사용되지 않습니다. 기본값이 필요하다면 상위 호출부에서 remember { object : CallvanListItemClickListener {} } 형태로 관리하는 것이 더 안전합니다.
// 기본값 제거 후, 필요 시 호출부에서 관리
fun CallvanListItemButtons(
state: CallvanRouteState,
clickListener: CallvanListItemClickListener,
modifier: Modifier = Modifier,
)
| horizontalArrangement = Arrangement.spacedBy(8.dp) | ||
| ) { | ||
| Text( | ||
| text = formatDateTime(date, time), |
There was a problem hiding this comment.
[Minor] formatDateTime 호출에 remember 래핑 권고
formatDateTime(date, time)이 컴포지션 중 직접 호출되고 있습니다. 이 함수가 문자열 파싱이나 포맷팅 같은 비용이 드는 연산을 수행한다면, date 또는 time이 변경되지 않아도 리컴포지션 시마다 재실행됩니다.
// 개선안
val formattedDateTime = remember(date, time) { formatDateTime(date, time) }
Text(
text = formattedDateTime,
style = KoinTheme.typography.regular12,
color = KoinTheme.colors.neutral600
)formatDateTime이 단순 문자열 결합이라면 큰 문제는 아니지만, 향후 로직이 복잡해질 경우를 대비해 remember로 래핑하는 습관을 권장합니다.
| .padding(end = 12.dp), | ||
| verticalArrangement = Arrangement.spacedBy(4.dp) | ||
| ) { | ||
| with(uiState) { |
There was a problem hiding this comment.
[Info] with(uiState) 스코프 사용 가독성
with(uiState) 블록 내에서 departure, destination, date, time 등의 프로퍼티가 수신자 없이 사용됩니다. 이 패턴은 간결하지만, Compose 컴포저블 내에서는 어떤 객체의 프로퍼티인지 명확하지 않아 가독성을 떨어뜨릴 수 있습니다.
// 현재: 어디서 온 값인지 불명확
with(uiState) {
CallvanRouteInfo(departure = departure, destination = destination)
}
// 개선안: 명시적으로 전달
CallvanRouteInfo(departure = uiState.departure, destination = uiState.destination)
Text(text = formatDateTime(uiState.date, uiState.time), ...)
CallvanParticipantCount(currentCount = uiState.currentCount, maxCount = uiState.maxCount)팀 컨벤션에 따라 판단하시되, Compose 코드에서는 명시적 참조가 코드 리뷰 및 디버깅에 유리합니다.
| } | ||
| } | ||
|
|
||
| @Preview(showBackground = true) |
There was a problem hiding this comment.
[Minor] Preview에 테마 래퍼 누락
다른 컴포넌트의 Preview(CallvanListItem, CallvanListItemButtons 등)는 RebrandKoinTheme으로 감싸져 있는데, 이 컴포넌트는 테마 없이 렌더링되어 색상·타이포그래피가 올바르게 반영되지 않을 수 있습니다.
| @Preview(showBackground = true) | |
| @Preview(showBackground = true) | |
| @Composable | |
| private fun CallvanParticipantCountPreview() { | |
| RebrandKoinTheme { | |
| CallvanParticipantCount( | |
| currentCount = 1, | |
| maxCount = 8 | |
| ) | |
| } | |
| } |
CallvanParticipantCountFullPreview도 동일하게 수정이 필요합니다.
| <string name="callvan_btn_cancel_join">참여 취소</string> | ||
| <string name="callvan_btn_closed">모집마감</string> | ||
| <string name="callvan_btn_close">마감하기</string> | ||
| <string name="callvan_btn_re_recruit">재모집</string> |
There was a problem hiding this comment.
[Trivial] 문자열 리소스 이름 컨벤션 불일치
ic_callvan_chat, ic_callvan_call은 drawable 리소스 이름처럼 보여 혼동을 줍니다. contentDescription용 문자열은 의도를 명확히 드러내는 이름을 사용하는 것이 좋습니다.
| <string name="callvan_btn_re_recruit">재모집</string> | |
| <string name="callvan_content_description_chat">채팅</string> | |
| <string name="callvan_content_description_call">전화하기</string> |
코드에서도 동일하게 R.string.callvan_content_description_chat, R.string.callvan_content_description_call로 참조를 변경해야 합니다.
| } | ||
| } | ||
| CallvanRouteState.OWNER_ACTIVE -> { | ||
| IconButton(onClick = { callbacks.onCall() }, modifier = Modifier.size(24.dp)) { |
There was a problem hiding this comment.
[Major] 접근성 위반: 최소 터치 타겟 크기 미달 (OWNER_ACTIVE)
58번 줄과 동일한 문제입니다. Modifier.size(24.dp)를 IconButton에 적용하지 말고, 내부 Icon에만 적용하세요.
| IconButton(onClick = { callbacks.onCall() }, modifier = Modifier.size(24.dp)) { | |
| IconButton(onClick = { callbacks.onCall() }) { | |
| Icon( | |
| imageVector = ImageVector.vectorResource(R.drawable.ic_callvan_call), | |
| contentDescription = stringResource(R.string.ic_callvan_call), | |
| tint = Color.Unspecified, | |
| modifier = Modifier.size(24.dp) | |
| ) |
| } | ||
| } | ||
| CallvanRouteState.OWNER_CLOSED -> { | ||
| IconButton(onClick = { callbacks.onCall() }, modifier = Modifier.size(24.dp)) { |
There was a problem hiding this comment.
[Major] 접근성 위반: 최소 터치 타겟 크기 미달 (OWNER_CLOSED)
58번, 92번 줄과 동일한 문제입니다. IconButton의 크기는 기본값(48dp)을 유지하고, Icon에만 Modifier.size(24.dp)를 적용하세요.
| IconButton(onClick = { callbacks.onCall() }, modifier = Modifier.size(24.dp)) { | |
| IconButton(onClick = { callbacks.onCall() }) { | |
| Icon( | |
| imageVector = ImageVector.vectorResource(R.drawable.ic_callvan_call), | |
| contentDescription = stringResource(R.string.ic_callvan_call), | |
| tint = Color.Unspecified, | |
| modifier = Modifier.size(24.dp) | |
| ) |
...callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.kt
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
| CallvanRouteState.JOINED -> { | ||
| IconButton(onClick = { callbacks.onChat() }, modifier = Modifier.size(24.dp)) { |
There was a problem hiding this comment.
[Major] 터치 타겟 크기 미달 — 접근성 위반
Modifier.size(24.dp)는 Material Design 및 Android 접근성 가이드라인에서 요구하는 최소 터치 타겟 크기(48×48dp)를 크게 밑돕니다. 손가락이 굵은 사용자나 운동 장애가 있는 사용자는 이 버튼을 누르기 어려울 수 있습니다.
IconButton은 기본적으로 48dp 터치 영역을 제공하면서 아이콘 자체는 원하는 크기로 표시할 수 있습니다:
| IconButton(onClick = { callbacks.onChat() }, modifier = Modifier.size(24.dp)) { | |
| IconButton(onClick = { callbacks.onChat() }) { | |
| Icon( | |
| imageVector = ImageVector.vectorResource(R.drawable.ic_callvan_chat), | |
| contentDescription = stringResource(R.string.ic_callvan_chat), | |
| tint = Color.Unspecified, | |
| modifier = Modifier.size(24.dp) | |
| ) |
OWNER_ACTIVE(line 92)와 OWNER_CLOSED(line 110)의 IconButton에도 동일하게 적용이 필요합니다.
.../callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/model/CallvanListUiState.kt
Show resolved
Hide resolved
| ) { | ||
| CallvanFilledButton( | ||
| text = stringResource(R.string.callvan_btn_join), | ||
| onClick = { callbacks.onJoin() } |
There was a problem hiding this comment.
[Minor] 불필요한 람다 래핑
{ callbacks.onJoin() }처럼 단순히 다른 함수를 호출하기만 하는 람다는 함수 참조로 교체하면 불필요한 클로저 객체 생성을 피할 수 있습니다. 동일한 패턴이 onCancelJoin, onClose, onReRecruit, onComplete, onCall에도 있습니다.
| onClick = { callbacks.onJoin() } | |
| onClick = callbacks.onJoin |
| ) { | ||
| when (state) { | ||
| CallvanRouteState.DEFAULT -> { | ||
| Spacer(modifier = Modifier.size(24.dp)) |
There was a problem hiding this comment.
[Minor] 레이아웃 정렬을 위한 Spacer 사용 — 암묵적 결합
DEFAULT와 CLOSED 상태에서 Spacer(24.dp)를 삽입해 JOINED/OWNER_* 상태의 IconButton(24.dp) 높이를 맞추는 방식은 두 UI 요소가 암묵적으로 결합된 것입니다. 아이콘 크기가 변경되면 Spacer 크기도 수동으로 같이 바꿔야 하는 유지보수 부담이 생깁니다.
Column 대신 Box나 height(IntrinsicSize.Max) + 고정 높이 전략을 사용하거나, 아이콘 영역을 항상 렌더링하되 Visibility만 제어하는 방법을 고려해보세요:
// 예시: 항상 공간을 확보하되 상태에 따라 표시 여부 조절
Icon(
...,
modifier = Modifier
.size(24.dp)
.alpha(if (hasIcon) 1f else 0f)
)
| shape = KoinTheme.shapes.extraSmall, | ||
| contentPadding = contentPadding, | ||
| colors = ButtonDefaults.outlinedButtonColors( | ||
| contentColor = contentColor, |
There was a problem hiding this comment.
[Minor] disabled 상태에서 테두리 색상이 변경되지 않음
disabledContentColor = contentColor로 텍스트 색상은 파라미터로 제어하고 있지만, border = BorderStroke(borderWidth, borderColor)는 enabled 상태에 관계없이 항상 동일한 색상으로 렌더링됩니다. CLOSED 상태에서는 borderColor = KoinTheme.colors.neutral300을 명시적으로 전달하므로 의도대로 동작하지만, 이 컴포넌트를 재사용하는 다른 곳에서 enabled = false로 쓸 경우 테두리가 활성 색상(primary500)으로 남아 시각적으로 어색할 수 있습니다.
disabledBorderColor 파라미터를 추가하거나, enabled 값에 따라 내부에서 색상을 계산하도록 개선을 권장합니다:
border = BorderStroke(
borderWidth,
if (enabled) borderColor else disabledBorderColor
)
| onClick = onClick, | ||
| shape = KoinTheme.shapes.extraSmall, | ||
| contentPadding = contentPadding, | ||
| colors = ButtonDefaults.buttonColors( |
There was a problem hiding this comment.
[Minor] 버튼 텍스트 색상 미지정 — 의도치 않은 색상 적용 가능성
ButtonDefaults.buttonColors(containerColor = RebrandKoinTheme.colors.primary500)에서 contentColor를 지정하지 않으면, Material3는 contentColorFor(containerColor)를 통해 색상을 추론합니다. 그런데 primary500이 Material 색상 스킴에 등록된 색상이 아닐 경우 폴백으로 Color.Black이 적용될 수 있어, 흰 텍스트를 기대한 디자인과 달리 검은 텍스트가 표시될 위험이 있습니다.
| colors = ButtonDefaults.buttonColors( | |
| colors = ButtonDefaults.buttonColors( | |
| containerColor = RebrandKoinTheme.colors.primary500, | |
| contentColor = Color.White | |
| ) |
| .padding(end = 12.dp), | ||
| verticalArrangement = Arrangement.spacedBy(4.dp) | ||
| ) { | ||
| val formattedDateTime = remember(uiState.date, uiState.time) { |
There was a problem hiding this comment.
[Minor] remember 키를 uiState 전체로 단순화 가능
CallvanListUiState가 @Immutable로 선언되어 있으므로, Compose 컴파일러는 uiState가 바뀌지 않는 한 recomposition을 스킵합니다. 따라서 remember 키로 개별 필드 대신 uiState 전체를 사용하는 것이 더 명확하고, 향후 uiState에 필드가 추가될 때 키 업데이트를 빠뜨리는 실수를 방지합니다.
| val formattedDateTime = remember(uiState.date, uiState.time) { | |
| val formattedDateTime = remember(uiState) { | |
| formatDateTime(uiState.date, uiState.time) | |
| } |
더 나아가, 이 포매팅 로직을 ViewModel에서 처리하거나 CallvanListUiState의 프로퍼티로 노출하면 Composable에서 완전히 제거할 수 있습니다.
...callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.kt
Outdated
Show resolved
Hide resolved
| ) { | ||
| Icon( | ||
| imageVector = ImageVector.vectorResource(R.drawable.ic_participants), | ||
| contentDescription = null, |
There was a problem hiding this comment.
[Minor] 접근성 — contentDescription null
아이콘의 contentDescription = null로 설정하면 TalkBack 등 스크린리더가 이 아이콘을 완전히 무시합니다. 옆 Text가 숫자 정보를 제공하더라도, 아이콘의 의미("참여 인원")를 설명하는 content description을 추가하면 시각 장애 사용자 경험이 개선됩니다.
| contentDescription = null, | |
| contentDescription = stringResource(R.string.callvan_participants_icon_description), |
strings.xml에 "참여 인원 아이콘" 같은 항목을 추가해 주세요.
| data class CallvanListUiState( | ||
| val departure: String, | ||
| val destination: String, | ||
| val date: String, |
There was a problem hiding this comment.
[Minor] Primitive Obsession — 날짜/시간을 String으로 관리
date: String과 time: String을 별도로 보유하면 다음 문제가 발생할 수 있습니다:
- 형식 검증이 없어
"2025-13-45"같은 잘못된 값이 런타임까지 잡히지 않음 CallvanListItem에서formatDateTime(uiState.date, uiState.time)처럼 매번 두 필드를 함께 전달해야 하는 번거로움
kotlinx-datetime의 LocalDateTime이나 java.time.LocalDateTime으로 통합하거나, 적어도 ViewModel 레이어에서 포매팅된 단일 String으로 변환해 UiState에 담는 것을 권장합니다:
@Immutable
data class CallvanListUiState(
val departure: String,
val destination: String,
val formattedDateTime: String, // ViewModel에서 포매팅 후 전달
val currentCount: Int,
val maxCount: Int
)
...an/src/main/java/in/koreatech/koin/feature/callvan/ui/list/model/CallvanListItemCallbacks.kt
Show resolved
Hide resolved
.../callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/model/CallvanListUiState.kt
Show resolved
Hide resolved
...lvan/src/main/java/in/koreatech/koin/feature/callvan/ui/component/CallvanParticipantCount.kt
Show resolved
Hide resolved
|
|
@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/component/CallvanListItemButtons.kt (1)
160-187: 파라미터 수가 많은 함수 - 리팩토링 고려SonarCloud에서 10개 파라미터가 권장 수(7개)를 초과한다고 경고합니다. 스타일 관련 파라미터들을 데이터 클래스로 그룹화하면 가독성과 유지보수성이 향상될 수 있습니다.
♻️ 선택적 리팩토링 제안
// 스타일 파라미터를 그룹화하는 데이터 클래스 private data class CallvanOutlinedButtonStyle( val contentColor: Color = RebrandKoinTheme.colors.primary500, val disabledContentColor: Color = contentColor, val borderColor: Color = RebrandKoinTheme.colors.primary500, val disabledBorderColor: Color = borderColor, val borderWidth: Dp = 0.5.dp ) `@Composable` private fun CallvanOutlinedButton( text: String, onClick: () -> Unit, modifier: Modifier = Modifier, enabled: Boolean = true, contentPadding: PaddingValues = PaddingValues(horizontal = 12.dp, vertical = 4.dp), style: CallvanOutlinedButtonStyle = CallvanOutlinedButtonStyle() ) { ... }🤖 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/CallvanListItemButtons.kt` around lines 160 - 187, The CallvanOutlinedButton function has too many parameters (SonarCloud warns >7); refactor by grouping style-related params into a single data class (e.g., CallvanOutlinedButtonStyle) and replace contentColor, disabledContentColor, borderColor, disabledBorderColor and borderWidth with a single style parameter; keep core params (text, onClick, modifier, enabled, contentPadding) on the composable and provide a default CallvanOutlinedButtonStyle to preserve existing behavior, then update usages of CallvanOutlinedButton to either pass the style object or rely on defaults.feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.kt (1)
89-182: 프리뷰 함수에서 중복된 리터럴 상수화 고려SonarCloud에서
"2025-02-05"가 5회,"천안 시외터미널"이 4회 중복 사용된다고 경고합니다. 프리뷰 전용 코드이지만, 상수로 추출하면 유지보수성이 향상됩니다.♻️ 선택적 리팩토링 제안
+private object PreviewData { + const val SAMPLE_DATE = "2025-02-05" + const val SAMPLE_TIME = "14:00" + const val SAMPLE_DESTINATION_TERMINAL = "천안 시외터미널" +} + `@Preview`(showBackground = true) `@Composable` private fun CallvanListItemDefaultPreview() { RebrandKoinTheme { CallvanListItem( uiState = CallvanListUiState( departure = "테니스장", - destination = "천안 시외터미널", - date = "2025-02-05", - time = "14:00", + destination = PreviewData.SAMPLE_DESTINATION_TERMINAL, + date = PreviewData.SAMPLE_DATE, + time = PreviewData.SAMPLE_TIME, currentCount = 1, maxCount = 8 ), state = CallvanRouteState.DEFAULT, callbacks = CallvanListItemCallbacks() ) } }🤖 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/CallvanListItem.kt` around lines 89 - 182, Multiple preview functions (CallvanListItemDefaultPreview, CallvanListItemJoinedPreview, CallvanListItemClosedPreview, CallvanListItemOwnerActivePreview, CallvanListItemOwnerClosedPreview) reuse the same string literals ("2025-02-05" and "천안 시외터미널"); extract these repeated literals into private constants (e.g., private const val PREVIEW_DATE = "2025-02-05" and private const val PREVIEW_DESTINATION = "천안 시외터미널") declared near the preview functions and replace the inlined literals in each CallvanListItem(...) invocation to reduce duplication and satisfy the SonarCloud warning.
🤖 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/CallvanListItem.kt`:
- Around line 89-182: Multiple preview functions (CallvanListItemDefaultPreview,
CallvanListItemJoinedPreview, CallvanListItemClosedPreview,
CallvanListItemOwnerActivePreview, CallvanListItemOwnerClosedPreview) reuse the
same string literals ("2025-02-05" and "천안 시외터미널"); extract these repeated
literals into private constants (e.g., private const val PREVIEW_DATE =
"2025-02-05" and private const val PREVIEW_DESTINATION = "천안 시외터미널") declared
near the preview functions and replace the inlined literals in each
CallvanListItem(...) invocation to reduce duplication and satisfy the SonarCloud
warning.
In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItemButtons.kt`:
- Around line 160-187: The CallvanOutlinedButton function has too many
parameters (SonarCloud warns >7); refactor by grouping style-related params into
a single data class (e.g., CallvanOutlinedButtonStyle) and replace contentColor,
disabledContentColor, borderColor, disabledBorderColor and borderWidth with a
single style parameter; keep core params (text, onClick, modifier, enabled,
contentPadding) on the composable and provide a default
CallvanOutlinedButtonStyle to preserve existing behavior, then update usages of
CallvanOutlinedButton to either pass the style object or rely on defaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8f6fd54b-20fd-40e6-a92e-db3b807bc56a
📒 Files selected for processing (5)
feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/component/CallvanParticipantCount.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItem.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/component/CallvanListItemButtons.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/model/CallvanListItemCallbacks.ktfeature/callvan/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/component/CallvanParticipantCount.kt
3 participants
PR 개요
이슈 번호:
PR 체크리스트
작업사항
작업사항의 상세한 설명
콜밴 리스트 아이템의 ui와 CallvanParticipantCount를 추가하였습니다
CallvanParticipantCount를 공용 컴포넌트로 만들고 기존에 이를 사용하던 부분은 CallvanParticipantCount을 사용하도록 리펙토링 하였습니다
논의 사항
스크린샷
추가내용
Summary by CodeRabbit
릴리스 노트