Skip to content
Closed
Show file tree
Hide file tree
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
Expand Up @@ -197,23 +197,7 @@ private fun CallvanDatePickerCard(
)
}
HorizontalDivider(color = RebrandKoinTheme.colors.neutral200)
Row(
modifier = Modifier.fillMaxWidth(),
horizontalArrangement = Arrangement.spacedBy(16.dp, Alignment.End)
) {
Text(
text = stringResource(R.string.callvan_create_picker_reset),
style = RebrandKoinTheme.typography.medium14,
color = RebrandKoinTheme.colors.primary500,
modifier = Modifier.clickable(onClick = onReset)
)
Text(
text = stringResource(R.string.callvan_create_picker_confirm),
style = RebrandKoinTheme.typography.medium14,
color = RebrandKoinTheme.colors.primary500,
modifier = Modifier.clickable(onClick = onConfirm)
)
}
CallvanPickerFooter(onReset = onReset, onConfirm = onConfirm)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package `in`.koreatech.koin.feature.callvan.ui.create.component

import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.unit.dp
import `in`.koreatech.koin.core.designsystem.theme.RebrandKoinTheme
import `in`.koreatech.koin.feature.callvan.R

@Composable
fun CallvanPickerFooter(
onReset: () -> Unit,
onConfirm: () -> Unit
) {
Row(
Copy link
Contributor

Choose a reason for hiding this comment

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

[Minor] Row 자체에도 수직 패딩 부재

Row에 수직 패딩이 없어, Card 내부에서 HorizontalDivider 바로 아래에 버튼들이 붙어 보일 수 있습니다. 시각적 여백을 위해 Row에 패딩을 추가하는 것을 권장합니다.

Suggested change
Row(
Row(
modifier = Modifier
.fillMaxWidth()
.padding(vertical = 4.dp),
horizontalArrangement = Arrangement.spacedBy(16.dp, Alignment.End)
) {

modifier = Modifier.fillMaxWidth(),
horizontalArrangement = Arrangement.spacedBy(16.dp, Alignment.End)
) {
Text(
text = stringResource(R.string.callvan_create_picker_reset),
style = RebrandKoinTheme.typography.medium14,
color = RebrandKoinTheme.colors.primary500,
modifier = Modifier.clickable(onClick = onReset)
)
Text(
text = stringResource(R.string.callvan_create_picker_confirm),
style = RebrandKoinTheme.typography.medium14,
color = RebrandKoinTheme.colors.primary500,
modifier = Modifier.clickable(onClick = onConfirm)
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
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.border
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material3.Card
import androidx.compose.material3.CardDefaults
import androidx.compose.material3.HorizontalDivider
import androidx.compose.material3.Text
import androidx.compose.material3.VerticalDivider
import androidx.compose.runtime.Composable
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import `in`.koreatech.koin.core.designsystem.theme.RebrandKoinTheme
import `in`.koreatech.koin.feature.callvan.R
import java.util.Locale
import kotlinx.collections.immutable.toImmutableList

@Composable
fun CallvanTimeField(

Check warning on line 34 in feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/create/component/CallvanTimeField.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function has 11 parameters, which is greater than the 7 authorized.

See more on https://sonarcloud.io/project/issues?id=BCSDLab_KOIN_ANDROID&issues=AZziAdZqn1Kc99pl8Jv7&open=AZziAdZqn1Kc99pl8Jv7&pullRequest=1325
formattedTime: String,
isPickerVisible: Boolean,
isAm: Boolean,
selectedHour: Int,
selectedMinute: Int,
onFieldClick: () -> Unit = {},
onAmPmIndexChange: (Int) -> Unit = {},
onHourIndexChange: (Int) -> Unit = {},
onMinuteIndexChange: (Int) -> Unit = {},
onReset: () -> Unit = {},
onConfirm: () -> Unit = {}
) {
val amPmText = stringResource(if (isAm) R.string.callvan_am else R.string.callvan_pm)

Column(modifier = Modifier.fillMaxWidth()) {
Column(
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 24.dp, vertical = 12.dp),
verticalArrangement = Arrangement.spacedBy(4.dp)
) {
CallvanSectionHeader(
label = stringResource(R.string.callvan_create_time_label),
hint = stringResource(R.string.callvan_create_time_hint)
)
Row(
modifier = Modifier
.fillMaxWidth()
.border(1.dp, RebrandKoinTheme.colors.neutral400, RoundedCornerShape(4.dp))
.clickable(onClick = onFieldClick),
verticalAlignment = Alignment.CenterVertically
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

[Major] 접근성 - 클릭 가능한 Row에 시맨틱 정보 누락

현재 clickable modifier만 적용되어 있어 TalkBack 사용자가 이 Row를 탭하면 "선택되지 않음, 더블탭으로 활성화" 정도만 읽힙니다. 시각장애인 사용자는 이 요소가 시간 선택 필드임을 알 수 없습니다.

role = Role.ButtoncontentDescription을 추가해야 합니다:

Suggested change
) {
) {
Row(
    modifier = Modifier
        .fillMaxWidth()
        .border(1.dp, RebrandKoinTheme.colors.neutral400, RoundedCornerShape(4.dp))
        .semantics {
            role = Role.Button
            contentDescription = "$amPmText $formattedTime, 시간 선택"
        }
        .clickable(onClick = onFieldClick),
    verticalAlignment = Alignment.CenterVertically
) {

import androidx.compose.ui.semantics.Role
import androidx.compose.ui.semantics.role
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.semantics.contentDescription

Text(
text = amPmText,
style = RebrandKoinTheme.typography.regular14,
color = RebrandKoinTheme.colors.neutral800,
modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp)
)
VerticalDivider(
modifier = Modifier.height(38.dp),
Copy link
Contributor

Choose a reason for hiding this comment

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

[Minor] VerticalDivider 높이 하드코딩 - 접근성 설정 시 레이아웃 깨짐 가능성

38.dp는 양쪽 Text의 padding(vertical = 8.dp) × 2 + 예상 폰트 높이를 합산한 값으로 보입니다. 그러나 시스템 폰트 크기를 키운 경우(접근성 설정), Text 높이가 커지면 VerticalDivider가 Row보다 짧아져 시각적으로 어색해집니다.

IntrinsicSize.Min을 활용하면 부모 Row의 실제 높이에 맞출 수 있습니다:

Row(
    modifier = Modifier
        .fillMaxWidth()
        .height(IntrinsicSize.Min)  // Row 높이를 자식 최대 높이로
        .border(...)
        .clickable(onClick = onFieldClick),
    verticalAlignment = Alignment.CenterVertically
) {
    // ...
    VerticalDivider(
        modifier = Modifier.fillMaxHeight(),  // 38.dp 대신
        color = RebrandKoinTheme.colors.neutral400
    )
    // ...
}

import androidx.compose.foundation.layout.fillMaxHeight
import androidx.compose.foundation.layout.IntrinsicSize

color = RebrandKoinTheme.colors.neutral400
)
Text(
text = formattedTime,
Comment on lines +77 to +78
Copy link
Contributor

Choose a reason for hiding this comment

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

[Minor] 빈 시간 문자열에 대한 placeholder 미처리

formattedTime이 빈 문자열일 경우 Text가 비어 보입니다. 날짜 선택 필드(CallvanDateField)와 일관성을 맞추기 위해 hint 텍스트를 표시하는 것을 권장합니다.

Suggested change
Text(
text = formattedTime,
Text(
text = formattedTime.ifEmpty { stringResource(R.string.callvan_create_time_hint) },
style = RebrandKoinTheme.typography.regular14,
color = if (formattedTime.isEmpty()) RebrandKoinTheme.colors.neutral400
else RebrandKoinTheme.colors.neutral800,
modifier = Modifier
.weight(1f)
.padding(horizontal = 16.dp, vertical = 8.dp)
)

style = RebrandKoinTheme.typography.regular14,
color = RebrandKoinTheme.colors.neutral800,
modifier = Modifier
.weight(1f)
.padding(horizontal = 16.dp, vertical = 8.dp)
)
}
}
AnimatedVisibility(
visible = isPickerVisible,
enter = expandVertically(),
exit = shrinkVertically()
) {
CallvanTimePickerCard(
isAm = isAm,
selectedHour = selectedHour,
selectedMinute = selectedMinute,
onAmPmIndexChange = onAmPmIndexChange,
onHourIndexChange = onHourIndexChange,
onMinuteIndexChange = onMinuteIndexChange,
onReset = onReset,
onConfirm = onConfirm
)
}
}
}
Comment on lines +33 to +104
Copy link
Contributor

Choose a reason for hiding this comment

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

[Info] 잘 작성된 코드 — 칭찬

전반적인 설계가 훌륭합니다:

  • 완벽한 State Hoisting: CallvanTimeField는 완전한 Stateless Composable로 설계되어 있어 테스트 가능성과 재사용성이 높습니다.
  • remember + toImmutableList() 활용: AM/PM, 시간, 분 리스트를 remember와 불변 리스트로 감싸 불필요한 recomposition을 방지한 점이 좋습니다.
  • AnimatedVisibility 적절 사용: 픽커 카드 표시/숨김 시 expandVertically/shrinkVertically 애니메이션을 적용해 자연스러운 UX를 구현했습니다.
  • @Suppress("LongParameterList") 명시: 파라미터 수가 많은 이유가 명확한 경우 lint 억제를 적절히 사용했습니다.
  • private 내부 컴포저블 분리: CallvanTimePickerCardprivate으로 숨겨 모듈 외부 노출을 최소화한 점이 좋습니다.


@Suppress("LongParameterList")
@Composable
private fun CallvanTimePickerCard(

Check warning on line 108 in feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/create/component/CallvanTimeField.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function has 8 parameters, which is greater than the 7 authorized.

See more on https://sonarcloud.io/project/issues?id=BCSDLab_KOIN_ANDROID&issues=AZziAdZqn1Kc99pl8Jv8&open=AZziAdZqn1Kc99pl8Jv8&pullRequest=1325
isAm: Boolean,
selectedHour: Int,
selectedMinute: Int,
onAmPmIndexChange: (Int) -> Unit,
onHourIndexChange: (Int) -> Unit,
onMinuteIndexChange: (Int) -> Unit,
onReset: () -> Unit,
onConfirm: () -> Unit
) {
val amLabel = stringResource(R.string.callvan_am)
val pmLabel = stringResource(R.string.callvan_pm)
val amPmItems = remember(amLabel, pmLabel) {
listOf(amLabel, pmLabel).toImmutableList()
}
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 key 활용

val amPmItems = remember(amLabel, pmLabel) {
    listOf(amLabel, pmLabel).toImmutableList()
}

런타임 언어 변경(locale change) 시 amLabel/pmLabel이 변경되면 캐시가 자동으로 무효화되도록 key를 올바르게 설정한 점이 좋습니다. ImmutableList를 사용해 불필요한 recomposition을 방지한 것도 좋은 패턴입니다.

val hourItems = remember {
(1..12).map { it.toString() }.toImmutableList()
}
val minuteItems = remember {
(0..59).map { String.format(Locale.ROOT, "%02d", it) }.toImmutableList()
}

val amPmIndex = if (isAm) 0 else 1
val hourIndex = (selectedHour - 1).coerceIn(0, 11)
val minuteIndex = selectedMinute.coerceIn(0, 59)
Copy link
Contributor

Choose a reason for hiding this comment

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

[Minor] selectedHour = 0 엣지 케이스 - 피커 표시와 필드 표시 불일치 가능성

val hourIndex = (selectedHour - 1).coerceIn(0, 11)

selectedHour가 12시간제 기준 1..12 범위여야 하는데, 초기 상태가 0으로 설정될 경우:

  • (0 - 1).coerceIn(0, 11)hourIndex = 0 → 피커는 "1" 표시
  • 부모 ViewModel의 formattedTime"00:00" 또는 초기값 표시

피커와 필드 헤더의 값이 불일치하는 상황이 발생합니다. 초기 상태를 1로 강제하거나, ViewModel에서 항상 유효한 값(1..12)을 보장하는 것을 권장합니다. requireinit 블록에서 검증하는 것도 방법입니다.

// ViewModel 또는 UseCase에서:
val selectedHour: Int = savedHour.coerceIn(1, 12)


Card(
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 24.dp),
shape = RoundedCornerShape(8.dp),
colors = CardDefaults.cardColors(containerColor = RebrandKoinTheme.colors.neutral100),
elevation = CardDefaults.cardElevation(defaultElevation = 4.dp)
) {
Column(
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 24.dp, vertical = 12.dp),
verticalArrangement = Arrangement.spacedBy(8.dp)
) {
Row(
modifier = Modifier.fillMaxWidth()
) {
CallvanScrollPicker(
items = amPmItems,
selectedIndex = amPmIndex,
onIndexChange = onAmPmIndexChange,
modifier = Modifier.weight(1f)
)
CallvanScrollPicker(
items = hourItems,
selectedIndex = hourIndex,
onIndexChange = onHourIndexChange,
modifier = Modifier.weight(1f),
textAlign = TextAlign.End
)
CallvanScrollPicker(
items = minuteItems,
selectedIndex = minuteIndex,
onIndexChange = onMinuteIndexChange,
modifier = Modifier.weight(1f),
textAlign = TextAlign.End
)
}
HorizontalDivider(color = RebrandKoinTheme.colors.neutral200)
CallvanPickerFooter(onReset = onReset, onConfirm = onConfirm)
}
}
}

@Preview(showBackground = true)
@Composable
private fun CallvanTimeFieldPreview() {
CallvanTimeField(
formattedTime = "09:30",
isPickerVisible = false,
isAm = true,
selectedHour = 9,
selectedMinute = 30,
onFieldClick = {},
onAmPmIndexChange = {},
onHourIndexChange = {},
onMinuteIndexChange = {},
onReset = {},
onConfirm = {}
)
}

@Preview(showBackground = true)
@Composable
private fun CallvanTimeFieldPickerVisiblePreview() {
CallvanTimeField(
formattedTime = "02:45",
isPickerVisible = true,
isAm = false,
selectedHour = 2,
selectedMinute = 45,
onFieldClick = {},
onAmPmIndexChange = {},
onHourIndexChange = {},
onMinuteIndexChange = {},
onReset = {},
onConfirm = {}
)
}
5 changes: 4 additions & 1 deletion feature/callvan/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,7 @@
<string name="callvan_report_error_already_pending">이미 신고가 접수된 사용자입니다.</string>
<string name="callvan_report_error_only_participant">같은 콜밴 참여자만 신고할 수 있습니다.</string>
<string name="callvan_report_error_failed">신고에 실패했습니다. 다시 시도해주세요.</string>
</resources>

<string name="callvan_am">오전</string>
<string name="callvan_pm">오후</string>
</resources>
Loading