Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
package `in`.koreatech.koin.feature.callvan.ui.create.component

import androidx.compose.animation.AnimatedVisibility
import androidx.compose.animation.expandVertically
import androidx.compose.animation.shrinkVertically
import androidx.compose.foundation.background
import androidx.compose.foundation.border
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.ExperimentalLayoutApi
import androidx.compose.foundation.layout.FlowRow
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.imePadding
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.foundation.text.BasicTextField
import androidx.compose.material3.ButtonDefaults
import androidx.compose.material3.HorizontalDivider
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.derivedStateOf
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import `in`.koreatech.koin.core.designsystem.component.button.FilledButton
import `in`.koreatech.koin.core.designsystem.theme.RebrandKoinTheme
import `in`.koreatech.koin.feature.callvan.R
import `in`.koreatech.koin.feature.callvan.model.CallvanLocationOption
import `in`.koreatech.koin.feature.callvan.ui.component.CallvanBottomSheet
import `in`.koreatech.koin.feature.callvan.ui.displayNameRes

@Composable
fun CallvanLocationPickerBottomSheet(

Check failure on line 44 in feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/create/component/CallvanLocationPickerBottomSheet.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 20 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=BCSDLab_KOIN_ANDROID&issues=AZzhHi_An1Kc99plzDpu&open=AZzhHi_An1Kc99plzDpu&pullRequest=1323
isDeparture: Boolean,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] isDeparture: Boolean 대신 열거형/sealed class 사용 권장

Boolean 파라미터는 호출부에서 의미를 파악하기 어렵고, 향후 "왕복" 같은 모드가 추가될 경우 확장이 불가능합니다.

// 호출부에서 의미가 불분명함
CallvanLocationPickerBottomSheet(isDeparture = true, ...)

// 개선안 1: sealed class 사용
sealed class LocationPickerMode {
    object Departure : LocationPickerMode()
    object Arrival : LocationPickerMode()
}

// 개선안 2 (간단하게): enum 사용
enum class LocationPickerMode { DEPARTURE, ARRIVAL }

// 파라미터 변경
fun CallvanLocationPickerBottomSheet(
    mode: LocationPickerMode,
    ...
)

호출부에서 isDeparture = true보다 mode = LocationPickerMode.DEPARTURE가 의도를 명확히 전달합니다.

modifier: Modifier = Modifier,
initialSelection: CallvanLocationOption? = null,
initialCustomText: String? = null,
onLocationSelected: (CallvanLocationOption, String?) -> Unit = { _, _ -> },
onDismiss: () -> Unit = {}
Comment on lines +49 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] 콜백 파라미터 기본값 — 미연결 시 무음 실패 가능

onLocationSelectedonDismiss에 빈 람다를 기본값으로 두면 호출 측에서 실수로 연결하지 않아도 컴파일 오류 없이 동작이 사라집니다. Preview용 편의가 목적이라면 @PreviewParameter 또는 별도 Preview 전용 래퍼를 쓰는 것이 더 안전합니다.

기본값을 제거해서 의도적 명시를 강제하는 것을 권장합니다:

Suggested change
onLocationSelected: (CallvanLocationOption, String?) -> Unit = { _, _ -> },
onDismiss: () -> Unit = {}
onLocationSelected: (CallvanLocationOption, String?) -> Unit,
onDismiss: () -> Unit

) {
Comment on lines +44 to +51
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

이 시그니처 때문에 CI가 막히고 있습니다.

현재 Detekt가 LongParameterList로 실패하고 있어서 이 상태로는 머지할 수 없습니다. initialSelection/initialCustomText를 하나의 초기 상태 모델로 묶어서 파라미터 수를 줄이거나, 정말 예외가 필요하면 suppression 근거를 같이 남겨 주세요.

🧰 Tools
🪛 GitHub Actions: CI

[error] 43-43: Detekt: The function CallvanLocationPickerBottomSheet(isDeparture: Boolean, modifier: Modifier, initialSelection: CallvanLocationOption?, initialCustomText: String?, onLocationSelected: (CallvanLocationOption, String?) -> Unit, onDismiss: () -> Unit) has too many parameters. The current threshold is set to 6. [LongParameterList]

🤖 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/create/component/CallvanLocationPickerBottomSheet.kt`
around lines 43 - 50, The function CallvanLocationPickerBottomSheet currently
fails Detekt due to LongParameterList; refactor by introducing a single initial
state model (e.g., a data class like CallvanLocationInitialState containing
initialSelection: CallvanLocationOption? and initialCustomText: String?) and
replace the two parameters initialSelection and initialCustomText with one
parameter of that model in the CallvanLocationPickerBottomSheet signature and
update all call sites to pass the model; alternatively, if you intentionally
must keep the parameters, add a targeted suppression (e.g.,
`@Suppress`("LongParameterList")) directly on CallvanLocationPickerBottomSheet and
include a short comment explaining why the exception is justified.

var selectedLocation by remember(initialSelection) { mutableStateOf(initialSelection) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Info - 잘 작성된 코드] 👍

remember(initialSelection)을 키로 사용하여 부모에서 초기값이 변경될 때 내부 상태를 올바르게 리셋하는 패턴이 잘 구현되어 있습니다. customTextremember(initialSelection, initialCustomText)로 키를 두 개 지정하여 CUSTOM 선택 시 초기 텍스트를 올바르게 복원하는 로직도 깔끔합니다.

var customText by remember(initialSelection, initialCustomText) {
mutableStateOf(
if (initialSelection == CallvanLocationOption.CUSTOM) initialCustomText.orEmpty() else ""
)
}
val isCustomSelected by remember { derivedStateOf { selectedLocation == CallvanLocationOption.CUSTOM } }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Trivial] 단순 동등 비교에 derivedStateOf — 약간 과도한 사용

derivedStateOf는 여러 state를 복잡하게 조합하거나, 계산 비용이 크고 결과가 자주 바뀌지 않을 때 recomposition을 줄이기 위해 사용합니다. 여기서는 단순 == 비교 한 번이라 오버헤드 대비 이득이 거의 없습니다. 물론 기능적으로는 완전히 올바릅니다.

isCustomSelected를 읽는 composable들이 selectedLocation이 non-CUSTOM 값들 사이에서 바뀔 때 불필요한 recomposition을 피하는 효과는 있지만, 이 화면의 규모에서는 실질적인 차이가 없습니다. 팀 컨벤션에 따라 아래처럼 단순화할 수도 있습니다.

// 심플한 대안 (기능 동일)
val isCustomSelected = selectedLocation == CallvanLocationOption.CUSTOM

derivedStateOf를 유지하는 것도 완전히 괜찮습니다.


CallvanBottomSheet(
title = stringResource(
if (isDeparture) {
R.string.callvan_create_departure_picker_question
} else {
R.string.callvan_create_arrival_picker_question
}
),
onDismiss = onDismiss,
showCloseButton = true
) {
Column(
modifier = modifier
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] modifier 파라미터가 루트 Composable이 아닌 내부 Column에 적용됨

Compose 컨벤션상 modifier 파라미터는 컴포넌트의 최상위 레이아웃 요소에 적용해야 합니다. 현재는 CallvanBottomSheet 내부의 Column에 전달되고 있어, 호출자가 예상하는 동작(예: Modifier.testTag(), Modifier.padding() 등이 시트 전체에 적용)과 다를 수 있습니다.

실제로 바텀시트 컴포넌트 특성상 외부 modifier를 받지 않는 설계도 유효하므로, 의도에 따라 다음 두 가지 중 하나를 선택하세요:

Option A: modifier를 제거하고 내부에 직접 하드코딩 (bottom sheet는 보통 외부 modifier를 받지 않음)

fun CallvanLocationPickerBottomSheet(
    isDeparture: Boolean,
    // modifier 파라미터 제거
    initialSelection: CallvanLocationOption? = null,
    ...

Option B: modifierCallvanBottomSheet에 전달하도록 수정 (API가 지원하는 경우)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] modifier가 외부 컨테이너 대신 내부 Column에 적용됨

CallvanBottomSheetmodifier를 받지 않는다면 이해는 가능하지만, 현재 구조에서는 외부에서 전달된 modifier가 내부 레이아웃 컬럼에 적용되어 의도치 않은 동작을 유발할 수 있습니다. (예: 외부에서 fillMaxWidth()나 특정 padding을 전달하면 내부 패딩과 충돌)

CallvanBottomSheetmodifier를 지원한다면 그쪽에 전달하거나, 아니면 내부 Column에는 Modifier(기본값)만 쓰고 외부 API에서 modifier를 제거하는 것을 고려해주세요.

.fillMaxWidth()
.imePadding()
.padding(horizontal = 32.dp, vertical = 12.dp),
verticalArrangement = Arrangement.spacedBy(24.dp)
) {
Comment on lines +71 to +77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] 키보드 표시 시 레이아웃 가림 — imePadding 누락

기타 옵션 선택 후 키보드가 올라오면 FilledButton이 키보드 뒤에 가려질 수 있습니다. CallvanBottomSheet 내부에서 처리하고 있다면 무시해도 됩니다만, 그렇지 않다면 Column modifier에 imePadding()을 추가하세요.

Suggested change
Column(
modifier = modifier
.fillMaxWidth()
.padding(horizontal = 32.dp, vertical = 12.dp),
verticalArrangement = Arrangement.spacedBy(24.dp)
) {
Column(
modifier = modifier
.fillMaxWidth()
.imePadding()
.padding(horizontal = 32.dp, vertical = 12.dp),
verticalArrangement = Arrangement.spacedBy(24.dp)
) {

Column(
verticalArrangement = Arrangement.spacedBy(12.dp)
) {
CallvanLocationChipGroup(
selectedLocation = selectedLocation,
onLocationClick = { location ->
selectedLocation = location
if (location != CallvanLocationOption.CUSTOM) customText = ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] 기타 재선택 시 커스텀 텍스트 유실

CUSTOM이 아닌 다른 항목을 클릭할 때 customText = ""로 초기화됩니다. 사용자가 직접 입력한 뒤 실수로 다른 칩을 눌렀다가 기타로 돌아오면 입력 내용이 사라집니다. UX 측면에서 CUSTOM이 다시 선택될 때까지 텍스트를 보존하는 방향을 고려해 보세요.

Suggested change
if (location != CallvanLocationOption.CUSTOM) customText = ""
selectedLocation = location

customText는 확인 버튼 클릭 시 isCustomSelectedfalsenull로 전달되므로, 굳이 중간에 지울 필요가 없습니다.

}
)
CallvanCustomLocationInput(
visible = isCustomSelected,
value = customText,
onValueChange = { customText = it }
)
HorizontalDivider(
color = RebrandKoinTheme.colors.neutral300,
thickness = 0.5.dp
)
}
FilledButton(
text = stringResource(
if (isCustomSelected) {
R.string.callvan_create_location_picker_confirm_other
} else {
R.string.callvan_create_location_picker_select
}
),
onClick = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Trivial] 중복 null 체크 — ?.let 불필요

enabled = selectedLocation != null && ... 조건이 이미 selectedLocation이 null일 때 버튼 클릭을 막아줍니다. onClick 내부의 ?.let null 체크는 이론적으로 실행될 수 없는 방어 코드입니다. 코드를 단순화할 수 있습니다.

Suggested change
onClick = {
onClick = {
onLocationSelected(
selectedLocation!!,
if (isCustomSelected) customText.trim() else null
)
},

!!이 불편하다면, selectedLocation ?: return@FilledButton 패턴도 허용되지만, enabled 가드가 있기 때문에 !!이 의도를 더 명확하게 드러냅니다.

selectedLocation?.let { loc ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Trivial] ?.let null 체크 중복

바로 위 109번 라인에서 enabled = selectedLocation != null && ... 조건이 있으므로, 버튼이 활성화된 상태에서만 onClick이 호출됩니다. 따라서 selectedLocation?.let의 null 체크는 실질적으로 도달 불가능한 방어 코드입니다.

onClick = {
    onLocationSelected(
        selectedLocation!!,
        if (isOtherSelected) customText.trim() else null
    )
},

혹은 안전하게 non-null 보장이 필요하다면 주석으로 의도를 명시해 주세요.

onLocationSelected(loc, if (isCustomSelected) customText.trim() else null)
}
},
enabled = selectedLocation != null && (!isCustomSelected || customText.isNotBlank()),
modifier = Modifier.fillMaxWidth(),
shape = RebrandKoinTheme.shapes.medium,
textStyle = RebrandKoinTheme.typography.bold16,
contentPadding = PaddingValues(vertical = 10.dp),
colors = ButtonDefaults.buttonColors(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] ButtonDefaults.buttonColors(...)remember로 감싸야 합니다.

ButtonDefaults.buttonColors(...)는 매 리컴포지션마다 새로운 ButtonColors 객체를 생성합니다. 이 컴포저블이 자주 리컴포지션된다면(예: customText 입력 중) 불필요한 객체 할당이 반복됩니다.

val buttonColors = remember {
    ButtonDefaults.buttonColors(
        containerColor = RebrandKoinTheme.colors.primary500,
        disabledContainerColor = RebrandKoinTheme.colors.neutral400,
        contentColor = RebrandKoinTheme.colors.neutral0,
        disabledContentColor = RebrandKoinTheme.colors.neutral0
    )
}

FilledButton(
    ...
    colors = buttonColors
)

다만 RebrandKoinTheme.colorsCompositionLocal로 제공된다면 테마 변경 시 재생성이 필요하므로, 테마 관련 키를 remember에 추가하는 것도 고려할 수 있습니다.

containerColor = RebrandKoinTheme.colors.primary500,
disabledContainerColor = RebrandKoinTheme.colors.neutral400,
contentColor = RebrandKoinTheme.colors.neutral0,
disabledContentColor = RebrandKoinTheme.colors.neutral0
)
Comment on lines +116 to +121
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] ButtonDefaults.buttonColors() recomposition마다 새 객체 생성

ButtonDefaults.buttonColors()@Composable 함수이지만, FilledButton이 매번 recompose될 때마다 새 ButtonColors 인스턴스를 생성합니다. 버튼이 자주 recompose된다면 불필요한 할당이 발생합니다.

remember로 캐싱하거나, 이 색상 조합이 앱 전체에서 반복 사용된다면 FilledButton의 기본값으로 통합하는 것도 좋습니다.

Suggested change
colors = ButtonDefaults.buttonColors(
containerColor = RebrandKoinTheme.colors.primary500,
disabledContainerColor = RebrandKoinTheme.colors.neutral400,
contentColor = RebrandKoinTheme.colors.neutral0,
disabledContentColor = RebrandKoinTheme.colors.neutral0
)
colors = remember {
ButtonDefaults.buttonColors(
containerColor = RebrandKoinTheme.colors.primary500,
disabledContainerColor = RebrandKoinTheme.colors.neutral400,
contentColor = RebrandKoinTheme.colors.neutral0,
disabledContentColor = RebrandKoinTheme.colors.neutral0
)
},

)
}
}
}

@OptIn(ExperimentalLayoutApi::class)
@Composable
private fun CallvanLocationChipGroup(
selectedLocation: CallvanLocationOption?,
onLocationClick: (CallvanLocationOption) -> Unit = {}
) {
FlowRow(
horizontalArrangement = Arrangement.spacedBy(12.dp),
verticalArrangement = Arrangement.spacedBy(8.dp)
) {
CallvanLocationOption.entries.forEach { location ->
val isSelected = selectedLocation == location
Box(
modifier = Modifier
.height(34.dp)
.border(
width = 1.dp,
color = if (isSelected) {
RebrandKoinTheme.colors.primary500
} else {
RebrandKoinTheme.colors.neutral300
},
shape = RoundedCornerShape(24.dp)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Trivial] 하드코딩된 RoundedCornerShape(24.dp) vs 테마 shape 불일치

같은 파일 내에서 BasicTextFieldRebrandKoinTheme.shapes.medium을 사용하는 반면, 칩은 RoundedCornerShape(24.dp)를 하드코딩합니다. 칩의 완전 둥근(pill) 모양이 디자인 의도라면 테마에 shapes.pill 또는 shapes.chip으로 정의해서 사용하거나, 사용 의도를 주석으로 명시하면 좋겠습니다.

// 의도를 명확히: pill 형태의 chip shape
shape = RoundedCornerShape(percent = 50)  // 또는 테마 값 사용

)
.clip(RoundedCornerShape(24.dp))
Comment on lines +149 to +151
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Trivial] 하드코딩된 RoundedCornerShape(24.dp) 중복

borderclip 모두에 RoundedCornerShape(24.dp)가 하드코딩되어 있습니다. 디자인 시스템에 적절한 shape 토큰이 있다면 그것을 사용하고, 없다면 지역 변수로 추출하여 일관성을 유지하세요.

val chipShape = RoundedCornerShape(24.dp)
Box(
    modifier = Modifier
        .height(34.dp)
        .border(width = 1.dp, color = ..., shape = chipShape)
        .clip(chipShape)
        ...
)

.clickable { onLocationClick(location) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] 접근성(Accessibility) 시맨틱 누락

.clickable 만으로는 TalkBack 등 접근성 도구가 이 요소가 "버튼"임을 인식하지 못하고, 선택 상태도 전달되지 않습니다. Modifier.semantics를 추가하여 역할과 선택 상태를 명시해 주세요.

.semantics {
    role = Role.Button
    selected = isSelected
    contentDescription = /* stringResource(location.displayNameRes()) — 이미 Text로 표시되므로 필요 시 추가 */
}
.clickable(
    onClickLabel = stringResource(location.displayNameRes())
) { onLocationClick(location) }

.padding(horizontal = 12.dp),
contentAlignment = Alignment.Center
) {
Text(
text = stringResource(location.displayNameRes()),
style = RebrandKoinTheme.typography.medium14,
color = if (isSelected) {
RebrandKoinTheme.colors.primary500
} else {
RebrandKoinTheme.colors.neutral500
}
)
}
}
}
}

@Composable
private fun CallvanCustomLocationInput(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] BasicTextField placeholder 미구현 — PR 설명과 불일치

PR 설명에서 "출발지/도착지 모드에 따라 제목과 플레이스홀더 자동 변경"을 언급하고 있지만, CallvanCustomLocationInputplaceholder 파라미터 자체가 없고, BasicTextFielddecorationBox도 없어 텍스트가 비어 있을 때 아무런 힌트가 표시되지 않습니다.

BasicTextFieldplaceholder를 기본 제공하지 않으므로 decorationBox를 통해 직접 구현해야 합니다.

Suggested change
private fun CallvanCustomLocationInput(
@Composable
private fun CallvanCustomLocationInput(
visible: Boolean,
value: String,
placeholder: String = "",
onValueChange: (String) -> Unit = {}
) {

그리고 BasicTextField 안에 decorationBox를 추가하세요:

BasicTextField(
    value = value,
    onValueChange = onValueChange,
    singleLine = true,
    textStyle = RebrandKoinTheme.typography.regular14.copy(
        color = RebrandKoinTheme.colors.neutral600
    ),
    decorationBox = { innerTextField ->
        Box(modifier = Modifier.fillMaxWidth()) {
            if (value.isEmpty()) {
                Text(
                    text = placeholder,
                    style = RebrandKoinTheme.typography.regular14,
                    color = RebrandKoinTheme.colors.neutral400
                )
            }
            innerTextField()
        }
    },
    modifier = Modifier...
)

호출 측(CallvanLocationPickerBottomSheet)에서도 isDeparture 값을 기반으로 적절한 placeholder string resource를 넘겨주어야 합니다.

visible: Boolean,
value: String,
onValueChange: (String) -> Unit = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] 커스텀 입력 최대 길이 제한 없음

서버 API 또는 UI 레이아웃에 따라 허용 가능한 최대 글자 수가 있을 텐데, 현재는 onValueChange에 아무런 제한이 없습니다. VisualTransformation 대신 onValueChange 레벨에서 간단히 제한할 수 있습니다.

onValueChange = { newText ->
    if (newText.length <= MAX_CUSTOM_LOCATION_LENGTH) onValueChange(newText)
},

MAX_CUSTOM_LOCATION_LENGTH를 companion object 또는 상수로 정의하고, 서버 spec과 맞추는 것을 권장합니다.

) {
AnimatedVisibility(
visible = visible,
enter = expandVertically(),
exit = shrinkVertically()
) {
BasicTextField(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] BasicTextField에 placeholder(힌트 텍스트) 없음 - UX 문제

BasicTextFieldTextField/OutlinedTextField와 달리 placeholder를 자동으로 처리하지 않습니다. 현재 코드는 입력 필드가 비어있을 때 사용자에게 무엇을 입력해야 하는지 힌트를 전혀 제공하지 않습니다.

decorationBox를 사용해 placeholder를 추가해야 합니다:

BasicTextField(
    value = value,
    onValueChange = onValueChange,
    singleLine = true,
    textStyle = RebrandKoinTheme.typography.regular14.copy(
        color = RebrandKoinTheme.colors.neutral600
    ),
    decorationBox = { innerTextField ->
        Box {
            if (value.isEmpty()) {
                Text(
                    text = stringResource(R.string.callvan_create_location_custom_placeholder),
                    style = RebrandKoinTheme.typography.regular14,
                    color = RebrandKoinTheme.colors.neutral400
                )
            }
            innerTextField()
        }
    },
    modifier = Modifier
        .fillMaxWidth()
        .padding(top = 4.dp)
        .border(1.dp, RebrandKoinTheme.colors.neutral300, RebrandKoinTheme.shapes.medium)
        .background(RebrandKoinTheme.colors.neutral0, RebrandKoinTheme.shapes.medium)
        .padding(horizontal = 20.dp, vertical = 13.dp)
)

placeholder 문자열 리소스도 함께 추가해주세요.

value = value,
onValueChange = onValueChange,
singleLine = true,
textStyle = RebrandKoinTheme.typography.regular14.copy(
color = RebrandKoinTheme.colors.neutral600
),
modifier = Modifier
.fillMaxWidth()
.padding(top = 4.dp)
.clip(RebrandKoinTheme.shapes.medium)
.border(1.dp, RebrandKoinTheme.colors.neutral300, RebrandKoinTheme.shapes.medium)
.background(RebrandKoinTheme.colors.neutral0, RebrandKoinTheme.shapes.medium)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] borderbackground 순서, clip 누락

수식어 체인에서 .border(...).background(...) 순서는 시각적으로 문제없지만, 텍스트 컨텐츠가 둥근 모서리를 넘어 그려지는 것을 방지하려면 .clip()이 필요합니다. 특히 포커스 시 텍스트가 스크롤될 때 모서리 밖으로 삐져나올 수 있습니다.

modifier = Modifier
    .fillMaxWidth()
    .padding(top = 4.dp)
    .clip(RebrandKoinTheme.shapes.medium)          // 컨텐츠 클리핑 추가
    .border(1.dp, RebrandKoinTheme.colors.neutral300, RebrandKoinTheme.shapes.medium)
    .background(RebrandKoinTheme.colors.neutral0, RebrandKoinTheme.shapes.medium)
    .padding(horizontal = 20.dp, vertical = 13.dp)

참고로 CallvanLocationChipGroup의 칩은 .clip()이 적용되어 있습니다.

.padding(horizontal = 20.dp, vertical = 13.dp)
)
Comment on lines +181 to +195
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] BasicTextField — 플레이스홀더(힌트) 없음

현재 BasicTextField에는 decorationBox가 없어서 필드가 비어 있을 때 사용자에게 무엇을 입력해야 하는지 전달되지 않습니다. 특히 기타 옵션은 자유 입력이라 안내 텍스트가 필수입니다.

decorationBox를 추가하여 플레이스홀더를 구현하세요:

BasicTextField(
    value = value,
    onValueChange = onValueChange,
    singleLine = true,
    textStyle = RebrandKoinTheme.typography.regular14.copy(
        color = RebrandKoinTheme.colors.neutral600
    ),
    modifier = Modifier
        .fillMaxWidth()
        .padding(top = 4.dp)
        .clip(RebrandKoinTheme.shapes.medium)
        .border(1.dp, RebrandKoinTheme.colors.neutral300, RebrandKoinTheme.shapes.medium)
        .background(RebrandKoinTheme.colors.neutral0, RebrandKoinTheme.shapes.medium)
        .padding(horizontal = 20.dp, vertical = 13.dp),
    decorationBox = { innerTextField ->
        Box(modifier = Modifier.fillMaxWidth()) {
            if (value.isEmpty()) {
                Text(
                    text = stringResource(R.string.callvan_create_custom_location_hint),
                    style = RebrandKoinTheme.typography.regular14,
                    color = RebrandKoinTheme.colors.neutral400
                )
            }
            innerTextField()
        }
    }
)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

실제로 textField 에 추가 그래픽이 없는 것 같습니다.
한번 확인해보시고 디자인과 일치시켜 주세요

Comment on lines +181 to +195
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] BasicTextFieldkeyboardOptions / keyboardActions 미설정

현재 소프트 키보드에 IME 액션이 설정되어 있지 않아, 사용자가 키보드의 완료/확인 버튼으로 입력을 마칠 수 없습니다. ImeAction.Done 과 함께 확인 버튼과 동일한 동작을 연결하는 것이 좋습니다.

import androidx.compose.foundation.text.KeyboardActions
import androidx.compose.foundation.text.KeyboardOptions
import androidx.compose.ui.text.input.ImeAction

BasicTextField(
    value = value,
    onValueChange = onValueChange,
    singleLine = true,
    keyboardOptions = KeyboardOptions(imeAction = ImeAction.Done),
    keyboardActions = KeyboardActions(
        onDone = { onImeAction() }  // 상위 컴포저블에서 콜백 주입
    ),
    textStyle = RebrandKoinTheme.typography.regular14.copy(
        color = RebrandKoinTheme.colors.neutral600
    ),
    ...
)

}
}

@Preview(showBackground = true)
@Composable
private fun CallvanLocationPickerBottomSheetPreview() {
RebrandKoinTheme {
CallvanLocationPickerBottomSheet(
isDeparture = true,
initialSelection = CallvanLocationOption.FRONT_GATE,
initialCustomText = null,
onLocationSelected = { _, _ -> },
onDismiss = {}
)
}
}

@Preview(showBackground = true)
@Composable
private fun CallvanLocationPickerBottomSheetCustomPreview() {
RebrandKoinTheme {
CallvanLocationPickerBottomSheet(
isDeparture = true,
initialSelection = CallvanLocationOption.CUSTOM,
initialCustomText = "test test test",
onLocationSelected = { _, _ -> },
onDismiss = {}
)
}
}
Loading