[Feat] 홈화면 복약컨테이너 UI 수정사항 반영 #156 #158
Conversation
-nextMedicationTime 필드 제거 -doseStatusList 필드 추가
-HomeMedicineContainer UI 개선
Walkthrough홈 화면 관련 DTO에 복용별 상태(DoseStatus)를 추가하고, DTO→UI 매핑과 뷰모델의 기본 next-dose 처리 및 HomeMedicineContainer 컴포저블을 업데이트해 각 약품에 대한 복용 아이콘 행을 렌더하도록 변경했습니다. (50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Screen as HomeScreen
participant VM as HomeViewModel
participant Mapper as HomeUiState.from(dto)
participant Comp as HomeMedicineContainer
participant UI as Composable
Screen->>VM: 약품 데이터 요청
VM->>Mapper: HomeUiState.from(dto)
Mapper->>Mapper: dto.medication...doseStatusList 매핑\n(시간코드 → 문자열, taken 보존)
Mapper-->>VM: MedicineUiState 리스트 반환
VM-->>Screen: UIState 방출
Screen->>Comp: MedicineUiState 전달
Comp->>Comp: renderList 생성 (doseStatusList 기준, null/패딩 보정)
Comp->>UI: dose 상태별 아이콘 행 렌더링 (복용/미복용/미확인)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![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)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt (1)
59-71: 시간 라벨 변환 로직 중복 제거(헬퍼로 추출) + null 처리 일관화
nextTime과dose.time에 대해 "MORNING/LUNCH/DINNER" → 한글 라벨 변환이 중복됩니다. 헬퍼로 뽑아 간결하게 유지하세요. 또한doseStatusList가 DTO에서 non-null이라면 현재의?.map { } ?: emptyList()는冗長합니다(향후 null 가능성을 열어둘 계획이면 유지해도 무방).예시:
private fun String?.toDoseLabel(): String? = when (this) { "MORNING" -> "아침" "LUNCH" -> "점심" "DINNER" -> "저녁" null, "" -> null else -> this } // 사용처 nextDoseTime = it.nextTime.toDoseLabel() ?: "-" doseStatusList = it.doseStatusList ?.map { d -> DoseStatusUiState(time = d.time.toDoseLabel() ?: "-", taken = d.taken) } ?: emptyList()app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (3)
141-166: 아이콘 렌더링 로직은 명확하고 안전합니다(LGTM). 다만 a11y 개선 제안
requiredCount클램프, null 패딩, 상태별 아이콘 선택 모두 적절합니다.접근성 향상을 위해 아이콘별
contentDescription을 구체화하세요.- val iconRes = when (taken) { - true -> R.drawable.ic_pill_taken - false -> R.drawable.ic_pill_untaken - null -> R.drawable.ic_pill_uncheck - } + val (iconRes, desc) = when (taken) { + true -> R.drawable.ic_pill_taken to "복용 완료" + false -> R.drawable.ic_pill_untaken to "미복용" + null -> R.drawable.ic_pill_uncheck to "미확인" + } ... - Image( - painter = painterResource(iconRes), - contentDescription = "복약 상태 아이콘", + Image( + painter = painterResource(iconRes), + contentDescription = desc, modifier = Modifier.size(22.dp), )
45-51: 코너 반경 불일치(그림자 14.dp vs Card 10.dp)
figmaShadow(cornerRadius = 14.dp)와shape = RoundedCornerShape(10.dp)가 달라 미세한 시각 불일치가 생길 수 있습니다. 동일 값으로 통일을 권장합니다.예시:
- shape = RoundedCornerShape(10.dp), + shape = RoundedCornerShape(14.dp),
169-173: 카피 미세 개선 제안
"${taken}/${required}회 복용"은 자연스럽지만 상단"회 하루 복용"과 표현이 조금 엇갈립니다. 상단을"하루 복용 횟수"등으로 정리하면 일관됩니다. 코드 변경은 불필수입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/konkuk/medicarecall/data/dto/response/HomeResponseDto.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt(7 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt (1)
89-95: 내부 코드에서 nextDoseTime 제공 확인됨 - 빌드 문제 없음내부 사용처를 확인한 결과, 모든 MedicineUiState 생성 코드에서 nextDoseTime 파라미터를 명시적으로 제공하고 있습니다:
- HomeUiState.kt:49, HomeViewModel.kt:163, HomeViewModel.kt:211, HomeRepositoryImpl.kt:74 모두 nextDoseTime 지정 완료
- doseStatusList는 기본값이 있어 하위호환성 유지
단, nextDoseTime에 기본값이 없으므로, 이 클래스가 외부 모듈에 노출되는 공개 API인 경우 외부 소비자의 호환성을 검토할 필요가 있습니다.
app/src/main/java/com/konkuk/medicarecall/data/dto/response/HomeResponseDto.kt
Show resolved
Hide resolved
| fun HomeMedicineContainer( | ||
| modifier: Modifier = Modifier, | ||
| medicines: List<MedicineUiState>, | ||
| onClick: () -> Unit | ||
| ) { //TODO: 복약 상태 아이콘 리스트 | ||
| // // 이상치 방어 | ||
| // val safeRequired = remember(todayRequiredCount) { todayRequiredCount.coerceAtLeast(0) } | ||
| // | ||
| // | ||
| // val renderList = remember(doseStatusList, safeRequired) { | ||
| // if (safeRequired == 0) { | ||
| // emptyList() | ||
| // } else { | ||
| // val normalized = doseStatusList.map { | ||
| // when (it.doseStatus) { | ||
| // DoseStatus.TAKEN -> it.copy(doseStatus = DoseStatus.TAKEN) | ||
| // DoseStatus.SKIPPED -> it.copy(doseStatus = DoseStatus.SKIPPED) | ||
| // DoseStatus.NOT_RECORDED -> it.copy(doseStatus = DoseStatus.NOT_RECORDED) | ||
| // } | ||
| // } | ||
| // when { | ||
| // normalized.isEmpty() -> List(safeRequired) { | ||
| // DoseStatusItem(time = "", doseStatus = DoseStatus.NOT_RECORDED) | ||
| // } | ||
| // | ||
| // normalized.size < safeRequired -> normalized + List(safeRequired - normalized.size) { | ||
| // DoseStatusItem(time = "", doseStatus = DoseStatus.NOT_RECORDED) | ||
| // } | ||
| // | ||
| // else -> normalized.take(safeRequired) | ||
| // } | ||
| // } | ||
| // } | ||
| onClick: () -> Unit, | ||
| ) { | ||
|
|
||
| Card( | ||
| modifier = Modifier | ||
| .clickable { onClick() } | ||
| .fillMaxWidth() |
There was a problem hiding this comment.
modifier 매개변수가 무시됩니다 + Card의 onClick API 사용 권장
현재 Card(modifier = Modifier.clickable { ... })로 외부 modifier가 적용되지 않습니다. 또한 M3 Card(onClick = ...)를 쓰면 리플/접근성/포커스가 일관됩니다.
아래처럼 수정:
-Card(
- modifier = Modifier
- .clickable { onClick() }
- .fillMaxWidth()
- .figmaShadow(
- group = LocalMediCareCallShadowProvider.current.shadow03,
- cornerRadius = 14.dp,
- ),
+Card(
+ onClick = onClick,
+ modifier = modifier
+ .fillMaxWidth()
+ .figmaShadow(
+ group = LocalMediCareCallShadowProvider.current.shadow03,
+ cornerRadius = 14.dp,
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun HomeMedicineContainer( | |
| modifier: Modifier = Modifier, | |
| medicines: List<MedicineUiState>, | |
| onClick: () -> Unit | |
| ) { //TODO: 복약 상태 아이콘 리스트 | |
| // // 이상치 방어 | |
| // val safeRequired = remember(todayRequiredCount) { todayRequiredCount.coerceAtLeast(0) } | |
| // | |
| // | |
| // val renderList = remember(doseStatusList, safeRequired) { | |
| // if (safeRequired == 0) { | |
| // emptyList() | |
| // } else { | |
| // val normalized = doseStatusList.map { | |
| // when (it.doseStatus) { | |
| // DoseStatus.TAKEN -> it.copy(doseStatus = DoseStatus.TAKEN) | |
| // DoseStatus.SKIPPED -> it.copy(doseStatus = DoseStatus.SKIPPED) | |
| // DoseStatus.NOT_RECORDED -> it.copy(doseStatus = DoseStatus.NOT_RECORDED) | |
| // } | |
| // } | |
| // when { | |
| // normalized.isEmpty() -> List(safeRequired) { | |
| // DoseStatusItem(time = "", doseStatus = DoseStatus.NOT_RECORDED) | |
| // } | |
| // | |
| // normalized.size < safeRequired -> normalized + List(safeRequired - normalized.size) { | |
| // DoseStatusItem(time = "", doseStatus = DoseStatus.NOT_RECORDED) | |
| // } | |
| // | |
| // else -> normalized.take(safeRequired) | |
| // } | |
| // } | |
| // } | |
| onClick: () -> Unit, | |
| ) { | |
| Card( | |
| modifier = Modifier | |
| .clickable { onClick() } | |
| .fillMaxWidth() | |
| fun HomeMedicineContainer( | |
| modifier: Modifier = Modifier, | |
| medicines: List<MedicineUiState>, | |
| onClick: () -> Unit, | |
| ) { | |
| Card( | |
| onClick = onClick, | |
| modifier = modifier | |
| .fillMaxWidth() | |
| .figmaShadow( | |
| group = LocalMediCareCallShadowProvider.current.shadow03, | |
| cornerRadius = 14.dp, | |
| ), |
🤖 Prompt for AI Agents
In
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt
around lines 33 to 42, the passed-in modifier is ignored because the Card uses a
fresh Modifier with .clickable and .fillMaxWidth(); update the Card to start
with the external modifier (e.g., modifier = modifier.fillMaxWidth()....) and
remove the separate .clickable, instead pass the onClick handler to the Card's
onClick parameter so ripple, focus and accessibility behavior follow Material3
conventions.
- HomeViewModel에 createFallbackHomeUiState 로직 수정
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt (2)
41-52: 선택된 어르신 ID null 시 크래시 가능 + 실패 처리 누락
selectedElderId.value!!는 NPE 위험. 실패 시 무반응도 개선 필요.fun callImmediate( careCallTimeOption: String, ) { viewModelScope.launch { - homeRepository.requestImmediateCareCall( - elderId = selectedElderId.value!!, - careCallOption = careCallTimeOption, - ).onSuccess { - - } - .onFailure { } + val elderId = selectedElderId.value ?: return@launch + homeRepository.requestImmediateCareCall( + elderId = elderId, + careCallOption = careCallTimeOption, + ) + .onSuccess { /* TODO: UI 갱신 or 토스트 */ } + .onFailure { e -> Log.e(TAG, "requestImmediateCareCall failed", e) } } }
156-168: fallback 약 목록의 nextDoseTime이 ‘-’로 고정되어 일관성 깨짐
createFallbackHomeUiState에서는 기본 라벨을 계산하지만, 여기서는"-"로 표기되어 UI에 “다음 복약 : -”가 노출됩니다. 동일 로직으로 통일하세요.- nextDoseTime = "-", // 시간표시 필요없으면 "-" 유지 + // 설정 정보 기반 기본 라벨로 통일 + nextDoseTime = defaultNextDose,예: fallbackMedicines 선언 직전에 아래를 추가
+// 설정 정보에서 첫 복용 시간 → 라벨 +val firstTimeCode = healthInfo?.medications?.keys?.firstOrNull()?.toString()?.uppercase() +val defaultNextDose = when (firstTimeCode) { + "MORNING" -> "아침약" + "LUNCH" -> "점심약" + "DINNER" -> "저녁약" + else -> "-" +}
♻️ Duplicate comments (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (1)
39-46: modifier 무시 + Card의 onClick 미사용 (이전 코멘트 재발)
외부modifier가 적용되지 않고,clickable로 Card 접근성/리플 일관성이 깨집니다. M3Card(onClick=...)와 전달받은modifier를 사용하세요.-Card( - modifier = Modifier - .clickable { onClick() } - .fillMaxWidth() - .figmaShadow( - group = LocalMediCareCallShadowProvider.current.shadow03, - cornerRadius = 14.dp, - ), +Card( + onClick = onClick, + modifier = modifier + .fillMaxWidth() + .figmaShadow( + group = LocalMediCareCallShadowProvider.current.shadow03, + cornerRadius = 14.dp, + ), colors = CardDefaults.cardColors(containerColor = Color.White), shape = RoundedCornerShape(10.dp), )불필요해지는
import androidx.compose.foundation.clickable도 제거 바랍니다.
🧹 Nitpick comments (3)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt (1)
203-217: 첫 복용 시간 선택이 Map 순서 의존적임 → 결정적(아침/점심/저녁) 우선순위로 정렬 권장
keys.firstOrNull()은 구현/런타임에 따라 비결정적일 수 있습니다. MORNING→LUNCH→DINNER 우선순위로 선택하세요.-val firstTimeCode: String? = healthInfo - ?.medications - ?.keys - ?.firstOrNull() - ?.toString() - ?.uppercase() +val firstTimeCode: String? = healthInfo + ?.medications + ?.keys + ?.map { it.toString().uppercase() } + ?.minByOrNull { + when (it) { "MORNING" -> 0; "LUNCH" -> 1; "DINNER" -> 2; else -> Int.MAX_VALUE } + }app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (2)
139-166: 복약 상태 아이콘 접근성 개선(중복 낭독 방지) + 정렬 검증반복 아이콘에 동일
contentDescription을 주면 TalkBack이 과다 낭독합니다. 아이콘별 CD는null로 두고 행 단위 설명을 붙이거나, 전체 상태 텍스트를 별도 노출하세요.-Image( - painter = painterResource(iconRes), - contentDescription = "복약 상태 아이콘", - modifier = Modifier.size(22.dp), -) +Image( + painter = painterResource(iconRes), + contentDescription = null, + modifier = Modifier.size(22.dp), +)또한
doseStatusList가 시간 순서(아침→점심→저녁)로 정렬되어 온다는 보장이 있는지 확인 부탁드립니다. 없으면 정렬/매핑 로직이 필요합니다.
189-205: 프리뷰 데이터 가독성 및 라벨 일관성
- 생성자 위치 인자 대신 명명 인자 사용 시 가독성↑.
nextDoseTime라벨은 본문 로직과 일관(“아침약/점심약/저녁약”)을 유지하세요. 현재"점심","아침"혼재.예:
MedicineUiState( medicineName = "당뇨약", todayTakenCount = 1, todayRequiredCount = 3, nextDoseTime = "점심약", doseStatusList = ... )Also applies to: 213-221
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt(7 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt(6 hunks)
| Text( | ||
| modifier = Modifier.align(Alignment.CenterVertically), | ||
| text = "다음 복약 : ${medicine.nextDoseTime ?: "아침약"}", | ||
| style = MediCareCallTheme.typography.R_14, | ||
| color = MediCareCallTheme.colors.main, | ||
| ) |
There was a problem hiding this comment.
‘다음 복약’ 라벨 fallback 보완 및 문자열 리소스화
서버/VM에서 "-"가 올 수 있어 “다음 복약 : -”가 노출됩니다. 빈 문자열/"-"를 동일하게 처리하세요. 또한 하드코딩 문자열은 리소스로 이동 권장.
-Text(
- modifier = Modifier.align(Alignment.CenterVertically),
- text = "다음 복약 : ${medicine.nextDoseTime ?: "아침약"}",
+val next = medicine.nextDoseTime?.takeUnless { it.isBlank() || it == "-" } ?: "아침약"
+Text(
+ modifier = Modifier.align(Alignment.CenterVertically),
+ text = "다음 복약 : $next",
style = MediCareCallTheme.typography.R_14,
color = MediCareCallTheme.colors.main,
)추가로 "복약", "회 하루 복용" 등도 strings.xml로 이전을 추천드립니다.
🤖 Prompt for AI Agents
In
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt
around lines 122-127, the hardcoded label and fallback allow "-" or empty to be
displayed as "다음 복약 : -" and should be moved to string resources; change the
Text call to use string resources for the label and the fallback (e.g.,
R.string.next_dose_label and R.string.next_dose_fallback), treat both null,
empty, or "-" as equivalent by normalizing medicine.nextDoseTime (if value ==
null || value.isBlank() || value == "-") -> use fallback, and also move other
hardcoded strings like "복약" and "회 하루 복용" into strings.xml and reference them
from code.
| fun <T, R> StateFlow<T>.mapState( | ||
| scope: CoroutineScope = GlobalScope, | ||
| transform: (T) -> R | ||
| transform: (T) -> R, | ||
| ): StateFlow<R> { | ||
| return map(transform).stateIn(scope, SharingStarted.Companion.Eagerly, transform(value)) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
GlobalScope + Eagerly로 인한 생명주기 누수/불필요한 upstream 작업
mapState의 기본 스코프가 GlobalScope, 시작 전략이 Eagerly입니다. 뷰모델 생명주기에 종속되고 구독 시에만 활성화되도록 변경하세요. 호출부도 명시 스코프로 교체 필요.
적용 예시:
- import kotlinx.coroutines.GlobalScope
+ // GlobalScope 제거-fun <T, R> StateFlow<T>.mapState(
- scope: CoroutineScope = GlobalScope,
- transform: (T) -> R,
-): StateFlow<R> {
- return map(transform).stateIn(scope, SharingStarted.Companion.Eagerly, transform(value))
-}
+fun <T, R> StateFlow<T>.mapState(
+ scope: CoroutineScope,
+ transform: (T) -> R,
+): StateFlow<R> =
+ map(transform).stateIn(scope, SharingStarted.WhileSubscribed(5_000), transform(value))호출부 업데이트:
- val elderNameList: StateFlow<List<String>> = _elderInfoList.mapState { list ->
+ val elderNameList: StateFlow<List<String>> = _elderInfoList.mapState(scope = viewModelScope) { list ->
list.map { it.name }
}- private val elderIdByName: StateFlow<Map<String, Int>> =
- _elderInfoList.mapState { list -> list.associate { it.name to it.id } }
+ private val elderIdByName: StateFlow<Map<String, Int>> =
+ _elderInfoList.mapState(scope = viewModelScope) { list -> list.associate { it.name to it.id } }레포 전역 점검 스크립트:
Also applies to: 69-71, 266-268, 15-15
🏁 Script executed:
#!/bin/bash
# mapState 호출부 점검(명시적 scope 파라미터 누락 탐지)
rg -nP '\.mapState\s*\(' -C2Length of output: 568
더 정확한 검색이 필요합니다. 함수 정의가 아닌 실제 호출부를 찾고, 언급된 라인들을 확인하겠습니다.
🏁 Script executed:
#!/bin/bash
# mapState 호출부를 더 정확하게 찾기 (정의가 아닌 호출)
# 호출: ._elderInfoList.mapState, _elderNameList.mapState 등의 패턴
rg -nP '_\w+\s*\.mapState\s*\(' app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
# 특정 라인 범위 확인
echo "=== Line 15 ==="
sed -n '15p' app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
echo "=== Lines 69-71 ==="
sed -n '69,71p' app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
echo "=== Lines 266-268 ==="
sed -n '266,268p' app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
echo "=== Lines 271-276 ==="
sed -n '271,276p' app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.ktLength of output: 591
다른 파일에서의 mapState 사용 여부도 확인하겠습니다.
🏁 Script executed:
#!/bin/bash
# 전체 레포에서 mapState 호출 찾기
rg -nP '\.mapState\s*\(' --type=kt -C2Length of output: 26
🏁 Script executed:
#!/bin/bash
# Kotlin 파일에서 mapState 호출 검색 (타입 지정 없이)
rg -nP '\.mapState\s*\(' --glob='*.kt' -C2Length of output: 568
🏁 Script executed:
#!/bin/bash
# mapState 호출부 검색 (람다 문법 포함)
rg -nP '\.mapState\s*\{' --glob='*.kt' -C2Length of output: 1195
GlobalScope와 Eagerly로 인한 생명주기 누수 문제 수정 필요
검증 결과, mapState 호출부 2곳 모두에서 명시적 scope 파라미터가 누락되어 있습니다. 이로 인해 GlobalScope 기본값이 적용되어 ViewModel 생명주기 이후에도 흐름이 지속되는 누수 발생:
- Line 69 (
elderNameList):.mapState { ... }→ scope 미지정 - Line 267 (
elderIdByName):.mapState { ... }→ scope 미지정 - Line 272:
scope: CoroutineScope = GlobalScope기본값 - Line 276:
SharingStarted.Companion.Eagerly즉시 구독
수정 사항:
- Line 272에서 GlobalScope 기본값 제거 (필수 파라미터로 변경)
- Line 276에서
SharingStarted.WhileSubscribed(5_000)사용 - Line 15의 GlobalScope import 제거 가능
- Line 69, 267의 호출부에서
scope = viewModelScope명시 추가
🤖 Prompt for AI Agents
In
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
around lines 271–276, change the mapState signature to require a CoroutineScope
parameter (remove the default GlobalScope) and replace
SharingStarted.Companion.Eagerly with SharingStarted.WhileSubscribed(5_000);
update the two call sites at lines 69 and 267 to pass scope = viewModelScope
when calling .mapState { ... }; also remove the now-unused GlobalScope import on
line 15. These changes prevent lifecycle leaks by using the ViewModel's scope
and delayed unsubscription.
# Conflicts: # app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt (2)
54-58: null 케이스 기본값 개선 제안
it.nextTime이 null일 때 빈 문자열("")을 반환하는데, UX 관점에서 "시간 미정" 같은 명시적인 문구가 더 나을 수 있습니다.nextDoseTime = when (it.nextTime) { "MORNING" -> "아침약" "LUNCH" -> "점심약" "DINNER" -> "저녁약" - else -> it.nextTime.orEmpty() + else -> it.nextTime ?: "시간 미정" },
59-70: 시간 변환 로직 중복 개선 제안
doseStatusList매핑 로직은 잘 작동하지만, 시간 변환 코드가 위의nextDoseTime(54-57번 줄)과 중복됩니다. 헬퍼 함수로 추출하면 유지보수가 더 쉬워질 것 같네요.예시:
private fun translateMealTime(time: String?): String = when (time) { "MORNING" -> "아침" "LUNCH" -> "점심" "DINNER" -> "저녁" else -> time.orEmpty() }그런 다음
nextDoseTime과doseStatusList매핑에서 재사용할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt (2)
91-94: 잘 설계된 데이터 클래스입니다
DoseStatusUiState의 tri-statetaken필드(true/false/null)는 복용 여부를 명확하게 표현하기에 적절합니다. 깔끔한 구조네요!
88-90: nextDoseTime 타입 변경은 안전함코드 검증 결과,
nextDoseTime이String?에서String으로 변경된 것은 breaking change가 아닙니다.이유:
- 매핑 로직(lines 53-58)이
when()표현식과orEmpty()폴백으로 항상 non-null String을 보장- 모든 인스턴스화 지점에서 유효한 String 값 제공 (fallback 포함)
- 소비자 코드(HomeMedicineContainer line 124)의 elvis 연산자는 불필요하지만 오동작하지 않음
스플래시 화면 멈춤 문제는 이 타입 변경과는 무관해 보입니다.
|
lgtm |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (2)
33-37: 전달받은modifier파라미터가 사용되지 않고 있어요함수 시그니처에서
modifier파라미터를 받고 있지만, 아래Card에서는 새로운Modifier를 사용하고 있어서 외부에서 전달한 modifier가 무시되고 있습니다. 이전 리뷰 코멘트에서도 지적된 부분이에요.아래 diff처럼 수정하면 외부 modifier도 적용되고, Material3의
Card(onClick = ...)API를 활용해서 접근성과 리플 효과도 일관되게 처리할 수 있습니다:fun HomeMedicineContainer( modifier: Modifier = Modifier, medicines: List<MedicineUiState>, onClick: () -> Unit, ) { Card( - modifier = Modifier - .clickable { onClick() } + onClick = onClick, + modifier = modifier .fillMaxWidth() .figmaShadow( group = LocalMediCareCallShadowProvider.current.shadow03, cornerRadius = 14.dp, ), colors = CardDefaults.cardColors(containerColor = Color.White), shape = RoundedCornerShape(10.dp), ) {
116-121: 문자열 리소스화 고려해보세요이전 리뷰에서는
"-"처리에 대한 제안이 있었는데, PR 설명에 따르면 이제 서버가 다음 복약 시간을 직접 계산해서 내려주신다고 하셨으니 현재?: "아침약"fallback은 의도된 것으로 보입니다.다만
"다음 복약 : ","아침약","회 하루 복용","복약"등 하드코딩된 문자열들을strings.xml로 이동하면 다국어 지원이나 유지보수 측면에서 도움이 될 것 같아요.예시:
// strings.xml에 추가 // <string name="next_dose_label">다음 복약 : %s</string> // <string name="default_dose_time">아침약</string> Text( modifier = Modifier.align(Alignment.CenterVertically), text = stringResource( R.string.next_dose_label, medicine.nextDoseTime ?: stringResource(R.string.default_dose_time) ), style = MediCareCallTheme.typography.R_14, color = MediCareCallTheme.colors.main, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt(6 hunks)
🧰 Additional context used
🪛 GitHub Actions: Android CI
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt
[error] 48-48: Detekt: Indentation issue detected. Unexpected indentation (8) (should be 4). [Indentation]
[error] 76-76: Detekt: Indentation issue detected. Unexpected indentation (16) (should be 12). [Indentation]
[error] 203-203: Detekt: First line in a method block should not be empty. [NoEmptyFirstLineInMethodBlock]
[error] 182-182: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 182-182: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 182-182: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 190-190: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 190-190: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 190-190: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 206-206: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 206-206: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 206-206: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 210-210: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 210-210: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 210-210: Detekt: Argument should be on a separate line (unless all arguments can fit a single line). [ArgumentListWrapping]
[error] 34-34: Detekt: Unused parameter 'modifier'. [UnusedParameter]
🔇 Additional comments (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (1)
133-167: 복약 상태 아이콘 렌더링 로직 잘 구현되었습니다
doseStatusList가 비어있거나 부분적으로만 채워진 경우를 모두 처리하는 fallback 로직이 탄탄하네요.coerceAtLeast(0)로 방어적으로 처리한 것도 좋고, 아이콘 매핑도 명확합니다.
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt
Show resolved
Hide resolved
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (5)
38-45: 이전 리뷰 이슈: modifier 파라미터가 여전히 무시되고 있습니다.Card가
Modifier.clickable을 사용하면서 외부에서 전달된modifier를 무시하고 있어요. Material3의Card(onClick = ...)를 사용하면 리플/접근성이 자동으로 처리됩니다.수정 제안:
- Card( - modifier = modifier - .clickable { onClick() } - .fillMaxWidth() - .figmaShadow( - group = LocalMediCareCallShadowProvider.current.shadow03, - cornerRadius = 14.dp, - ), + Card( + onClick = onClick, + modifier = modifier + .fillMaxWidth() + .figmaShadow( + group = LocalMediCareCallShadowProvider.current.shadow03, + cornerRadius = 14.dp, + ),
117-122: 이전 리뷰 이슈: 하드코딩된 문자열과 null 안전성을 확인해주세요.서버에서 nextDoseTime을 항상 제공한다고 하셨지만, 타입이 nullable이라면 null 체크를 추가하는 게 안전해요. 또한 "다음 복약 : " 같은 문자열들은 strings.xml로 옮기는 걸 권장드립니다.
nextDoseTime의 null 가능성을 확인하고, 필요하다면 fallback 추가를 고려해주세요:
val nextDose = medicine.nextDoseTime ?: "미정" Text( modifier = Modifier.align(Alignment.CenterVertically), text = "다음 복약 : $nextDose", style = MediCareCallTheme.typography.R_14, color = MediCareCallTheme.colors.main, )
48-48: 이전 리뷰 이슈: 들여쓰기가 Detekt 규칙과 맞지 않습니다.Detekt이 이 줄의 들여쓰기를 지적하고 있어요.
74-76: 이전 리뷰 이슈: 들여쓰기가 Detekt 규칙과 맞지 않습니다.Column의 닫는 괄호 들여쓰기가 Detekt에서 지적되고 있어요.
211-224: 이전 리뷰 이슈: 두 번째 프리뷰의 인자 포맷팅이 일관성이 없습니다.첫 번째 프리뷰에서는 named parameter를 각 줄에 작성했는데, 여기서는 positional argument를 한 줄에 모두 작성하고 있어요. Detekt의 ArgumentListWrapping 규칙을 위반합니다.
수정 제안:
val sampleMedicines = listOf( MedicineUiState( - "당뇨약", - 0, - 3, - "아침", + medicineName = "당뇨약", + todayTakenCount = 0, + todayRequiredCount = 3, + nextDoseTime = "아침", doseStatusList = emptyList(), ), MedicineUiState( - "혈압약", - 0, - 2, - "아침", + medicineName = "혈압약", + todayTakenCount = 0, + todayRequiredCount = 2, + nextDoseTime = "아침", doseStatusList = emptyList(), ), )
🧹 Nitpick comments (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (2)
135-159: renderList 로직은 정확하지만 접근성 개선을 고려해주세요.아이콘 렌더링 로직이 엣지 케이스를 잘 처리하고 있어요. 다만
contentDescription이 "복약 상태 아이콘"으로 모든 아이콘에 동일하게 적용되어 스크린 리더 사용자가 각 아이콘의 실제 상태를 구분하기 어려울 수 있습니다.contentDescription을 상태별로 구체화하면 더 좋아요:
renderList.forEach { taken -> val iconRes = when (taken) { true -> R.drawable.ic_pill_taken false -> R.drawable.ic_pill_untaken null -> R.drawable.ic_pill_uncheck } + val description = when (taken) { + true -> "복용 완료" + false -> "미복용" + null -> "미기록" + } Image( painter = painterResource(iconRes), - contentDescription = "복약 상태 아이콘", + contentDescription = description, modifier = Modifier.size(22.dp), ) }
135-145: 선택사항: renderList 계산을 별도 함수로 추출하면 가독성이 더 좋아질 수 있어요.현재 로직은 명확하지만, 10줄 정도 되는 계산을 별도 함수로 추출하면 코드가 더 읽기 쉬워질 수 있습니다.
예시:
private fun computeRenderList( doseStatusList: List<DoseStatusUiState>, requiredCount: Int ): List<Boolean?> { val count = requiredCount.coerceAtLeast(0) if (doseStatusList.isEmpty()) { return List(count) { null } } val filled = doseStatusList.map { it.taken } return if (filled.size < count) { filled + List(count - filled.size) { null } } else { filled.take(count) } }그런 다음 사용:
val renderList = computeRenderList(medicine.doseStatusList, medicine.todayRequiredCount)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (3)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (3)
26-26: 새 DoseStatusUiState import 잘 추가됐습니다.doseStatusList 렌더링에 필요한 import가 정상적으로 추가되었습니다.
49-52: 패딩 조정 확인했습니다.Column의 패딩이 20.dp으로 설정되어 UI가 깔끔하게 정리되었네요.
182-202: 첫 번째 프리뷰 데이터가 깔끔하게 포맷팅되었습니다.MedicineUiState 생성 시 named parameter를 사용하고 각 인자를 별도 줄에 배치해서 가독성이 좋아졌어요. doseStatusList도 잘 추가되었습니다.
|
@alswlekk @ikseong00 리뷰부탁드립니다! |
| val todayRequiredCount: Int, | ||
| val nextDoseTime: String?, | ||
| val nextDoseTime: String, | ||
| val doseStatusList: List<DoseStatusUiState> = emptyList(), |
There was a problem hiding this comment.
nestDoseTime이란 게 다음에 복약을 해야하는 시간맞을까요 ?? 처음엔 다음으로 복약을 해야하는 시간으로 이해를 했었는데 PreviewHomeMedicineContainer엔 해당 시간에 false로 입력돼있어서요 !
There was a problem hiding this comment.
넵 맞습니다! 제가 프리뷰 예시를 잘못적어놨어요
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (3)
33-48: 외부 modifier가 여전히 무시되고 있습니다이전 리뷰에서 지적된 사항이 반영되지 않았네요.
Card가 새로운Modifier로 시작하면서 외부에서 전달된modifier파라미터를 무시하고 있습니다. 또한 Material3의Card는onClick파라미터를 제공하므로 별도로clickable을 사용할 필요가 없습니다.아래와 같이 수정해주세요:
Card( + onClick = onClick, - modifier = Modifier - .clickable { onClick() } + modifier = modifier .fillMaxWidth() .figmaShadow( group = LocalMediCareCallShadowProvider.current.shadow03, cornerRadius = 14.dp, ),
211-224: Preview 데이터의 인자 포맷팅을 수정해주세요이전 리뷰에서 지적된
ArgumentListWrapping문제가 여전히 남아있네요. 첫 번째 preview는 수정되었는데 두 번째 preview는 아직 수정되지 않았습니다. Detekt가 이 부분을 감지할 것입니다.각 인자를 별도 줄로 분리해주세요:
MedicineUiState( - "당뇨약", - 0, - 3, - "아침", + medicineName = "당뇨약", + todayTakenCount = 0, + todayRequiredCount = 3, + nextDoseTime = "아침", doseStatusList = emptyList(), ),
117-122: nextDoseTime의 null 처리가 필요합니다서버에서 항상 값을 내려준다고 하더라도,
nextDoseTime이 null이거나 빈 문자열일 경우 "다음 복약 : null" 또는 "다음 복약 : " 같은 텍스트가 사용자에게 노출될 수 있습니다. 방어적인 fallback 처리를 추가하는 것을 권장합니다.예를 들어:
Text( modifier = Modifier.align(Alignment.CenterVertically), - text = "다음 복약 : ${medicine.nextDoseTime}", + text = "다음 복약 : ${medicine.nextDoseTime ?: "예정된 복약 없음"}", style = MediCareCallTheme.typography.R_14, color = MediCareCallTheme.colors.main, )추가로, "다음 복약", "예정된 복약 없음" 등의 문자열은
strings.xml로 분리하는 것이 다국어 지원 및 유지보수에 유리합니다.
🧹 Nitpick comments (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (1)
135-145: renderList 로직을 좀 더 간결하게 작성할 수 있습니다현재 로직은 정확하게 동작하지만, 조건문을 조금 더 단순화할 수 있습니다. 예를 들어:
val renderList = medicine.doseStatusList .map { it.taken } .let { filled -> if (filled.size < requiredCount) { filled + List(requiredCount - filled.size) { null } } else { filled.take(requiredCount) } }이렇게 하면 빈 리스트일 때도 자연스럽게 처리됩니다. 다만 현재 코드도 충분히 읽기 좋으니 선택사항입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔗 관련 이슈
📙 작업 설명
서버에서 nextTime을 요청 시각 이후의 가장 가까운 복약 시간대로 계산해 내려주므로
아래 사항과 같이 서버 값 그대로 표시하도록 수정했습니다.
LUNCH null (아직 케어콜 전) LUNCH 다음 복약: 점심약
LUNCH null (케어콜 지남, 미기록) DINNER 다음 복약: 저녁약
데이터 없음(케어콜 전) 시 404 응답이 확인되었습니다.
서버 코드 개편 후, 미기록 시 다음 복약 표시는 추후 수정 예정입니다
💬 추가 설명 or 리뷰 포인트 (선택)
Q. 이전 pr들 머지후 homeviewmodel에서 충돌이 발생하여, 로컬에서 최신내용 반영후 에뮬을 돌렸는데 스플래시화면이후로 안넘어가지는데 어떤 문제인지 모르겠어서 질문드립니다
Summary by CodeRabbit
새로운 기능
개선사항