[Fix] 어르신이 고정되어 상세화면에서 어르신이 변경되지 않는 문제 #185 #186
Conversation
- elderId 전달 구조로 개편
- 상세 화면 이동 시 elderId 전달 누락 문제 수정
- 상세 화면 이동 시 elderId 전달되도록 함수 구조 수정
- HomeNavigation 함수 시그니처를 (Int) -> Unit 형태로 정리 - HomeScreen에서 selectedElderId 기반으로 상세 화면 이동하도록 수정
- HomeViewModel의 selectedElderId 의존 제거 - 모든 DetailScreen이 elderId 파라미터로 직접 받도록 변경
- HomeViewModel 의존 제거, elderId 인자 도입
- refreshData: 두 타이밍 0페이지 isRefresh 로 초기 로드 (처음 화면 진입, 어르신 변경시 0페이지부터 로드)
- API 여러번 호출 방지
librawish808
added
bug
Walkthrough여섯 개의 상세화면이 HomeViewModel 대신 명시적 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Home as HomeScreen
participant Nav as NavLayer
participant Route as Route(data)
participant Detail as DetailScreen
participant VM as ViewModel
User->>Home: 어르신 선택/상세 진입
Home->>Home: selectedElderId 갱신
Home->>Nav: navigateToXDetailScreen(elderId)
Nav->>Route: Route.XDetail(elderId)
Nav->>Detail: Composable 진입(백스택)
Detail->>Detail: toRoute<Route.XDetail>()로 elderId 추출
Detail->>VM: 데이터 로드/갱신(elderId)
VM-->>Detail: uiState
Detail-->>User: 상세 UI 렌더링
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/statehealth/screen/StateHealthDetailScreen.kt (1)
39-58: non‑null Int에 safe‑call을 써서 컴파일이 안 납니다
elderId가Int파라미터라 널이 될 수 없는데, 아래처럼 safe‑call을 사용하고 있어서 컴파일 에러가 날 것 같아요.LaunchedEffect(elderId, selectedDate) { elderId?.let { id -> healthViewModel.loadHealthDataForDate(id, selectedDate) } }여기서는 바로
elderId를 넘기면 될 것 같습니다.- LaunchedEffect(elderId, selectedDate) { - elderId?.let { id -> - healthViewModel.loadHealthDataForDate(id, selectedDate) - } - } + LaunchedEffect(elderId, selectedDate) { + healthViewModel.loadHealthDataForDate(elderId, selectedDate) + }라우트 기반으로 elderId를 항상 넘겨주는 구조라면 이쪽이 의도에도 더 잘 맞습니다.
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/medicine/screen/MedicineDetailScreen.kt (1)
42-58: 여기도 non‑null Int에 safe‑call 써서 컴파일 에러 발생
MedicineDetailScreen에서도elderId가 널이 아닌Int인데 safe‑call을 쓰고 있어서 컴파일이 안 날 것 같습니다.LaunchedEffect(elderId, selectedDate) { Log.d("MED_UI", "LaunchedEffect: elderId=$elderId, date=$selectedDate") elderId?.let { medicineViewModel.loadMedicinesForDate(it, selectedDate) } }여기는 그냥 바로 호출하면 될 것 같아요.
- LaunchedEffect(elderId, selectedDate) { - Log.d("MED_UI", "LaunchedEffect: elderId=$elderId, date=$selectedDate") - elderId?.let { medicineViewModel.loadMedicinesForDate(it, selectedDate) } - } + LaunchedEffect(elderId, selectedDate) { + Log.d("MED_UI", "LaunchedEffect: elderId=$elderId, date=$selectedDate") + medicineViewModel.loadMedicinesForDate(elderId, selectedDate) + }라우트에서 항상 유효한 elderId를 넘기는 구조라면 이게 자연스러운 구현 같습니다.
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/statemental/screen/StateMentalDetailScreen.kt (1)
52-57:elderId논-널 타입에서?.let연산자 제거 필요
elderId가Int(논-널)로 선언되어 있는데,LaunchedEffect내에서elderId?.let { ... }을 사용하는 것은 불필요합니다.MealDetailScreen과 동일한 방식으로 직접 전달하면 됩니다.- // 날짜/어르신 변경 시마다 로드 - LaunchedEffect(elderId, selectedDate) { - elderId?.let { id -> - mentalViewModel.loadMentalDataForDate(id, selectedDate) - } - } + // 날짜/어르신 변경 시마다 로드 + LaunchedEffect(elderId, selectedDate) { + mentalViewModel.loadMentalDataForDate(elderId, selectedDate) + }app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/sleep/screen/SleepDetailScreen.kt (1)
51-56:elderIdnon-nullInt인데 safe call (?.let) 사용으로 인한 컴파일 오류검증 완료:
elderId는 non-nullableInt이므로 safe call 연산자를 사용할 수 없습니다. MealDetailScreen에서 볼 수 있듯이 올바른 패턴은 직접 전달하는 것입니다.- // 날짜/어르신 변경 시마다 로드 - LaunchedEffect(elderId, selectedDate) { - elderId?.let { id -> - sleepViewModel.loadSleepDataForDate(id, selectedDate) - } - } + // 날짜/어르신 변경 시마다 로드 + LaunchedEffect(elderId, selectedDate) { + sleepViewModel.loadSleepDataForDate(elderId, selectedDate) + }참고: StateMentalDetailScreen에서도 동일한 패턴이 있으므로 함께 수정 필요합니다.
🧹 Nitpick comments (5)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/navigation/HomeNavigation.kt (1)
15-42: 네비게이션 시그니처 정리는 OK, 람다는 조금 더 단순화 가능
homeNavGraph가(Int) -> Unit시그니처로 정리되고,HomeScreen까지 elderId를 그대로 타고 가는 구조는 의도에 잘 맞습니다.다만 중간 람다는 그대로 전달만 하고 있어서 약간 줄일 수는 있을 것 같아요:
- HomeScreen( - navigateToMealDetailScreen = { elderId -> - navigateToMealDetailScreen(elderId) - }, - // 이하 동일 패턴 - ) + HomeScreen( + navigateToMealDetailScreen = navigateToMealDetailScreen, + navigateToMedicineDetailScreen = navigateToMedicineDetailScreen, + navigateToSleepDetailScreen = navigateToSleepDetailScreen, + navigateToStateHealthDetailScreen = navigateToStateHealthDetailScreen, + navigateToStateMentalDetailScreen = navigateToStateMentalDetailScreen, + navigateToGlucoseDetailScreen = navigateToGlucoseDetailScreen, + // 나머지 인자 동일 + )필수는 아니고, 나중에 리팩터링 여지 정도로 봐주시면 될 것 같습니다.
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/medicine/screen/MedicineDetailScreen.kt (1)
55-62: 디버그 로그는 나중에 정리 고려
"MED_UI"태그로 LaunchedEffect 진입과 렌더링마다 로그를 찍고 있는데, 디버깅 끝나면 verbose 로그는 줄이거나 Timber 등으로 레벨 조절해 두는 것도 좋겠습니다. 당장은 막 문제는 아니라 선택 사항으로 봐 주세요.app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/navigation/HomeDetailNavGraph.kt (1)
15-37: NavController 확장 함수로 상세 네비게이션 의도 명확해짐
NavController.navigateTo*DetailScreen(elderId: Int)확장 함수로 라우팅을 감싼 건 호출 측에서 의도가 잘 드러나서 좋습니다. 다만MainNavigator쪽에서도 같은 형태로Route.*Detail(elderId)를 직접 호출하고 있어서, 장기적으로는 한쪽만 쓰도록 정리해 두면 네비 경로가 분산되지 않아 유지보수에 더 좋을 것 같습니다.app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/glucoselevel/screen/GlucoseDetailScreen.kt (2)
77-105:refreshData로 초기 로딩·재진입·어르신 변경 처리하는 구조 괜찮음
refreshData에서 BEFORE/AFTER 두 타입 모두 카운터를 0으로 초기화한 뒤isRefresh = true로 한 번에 불러주고, 이를LaunchedEffect(elderId)와LifecycleEventEffect(ON_RESUME)양쪽에서 재사용하는 패턴이 깔끔합니다. 혈당 상세 화면이 다른 상세 화면들처럼 “어르신 변경 시 새 데이터로 갈아끼우는” 요구사항을 잘 만족시킬 것 같아요.
108-131: 페이지네이션에서 중복 요청 방지 플래그 처리 적절함
shouldLoad조건 +!uiState.isLoading+uiState.hasNext+!isRequestingMore.value조합으로 실제 API 호출 전에 한 번 더 걸러주고, 로딩 완료 시점(LaunchedEffect(uiState.isLoading))에isRequestingMore를 false로 되돌려주는 구조라, 스크롤 시 다중 요청 문제는 상당히 줄어들 것으로 보입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/navigation/HomeNavigation.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/glucoselevel/screen/GlucoseDetailScreen.kt(3 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/meal/screen/MealDetailScreen.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/medicine/screen/MedicineDetailScreen.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/navigation/HomeDetailNavGraph.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/sleep/screen/SleepDetailScreen.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/statehealth/screen/StateHealthDetailScreen.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/statemental/screen/StateMentalDetailScreen.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/navigation/MainNavigator.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/navigation/Route.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/navigation/HomeNavigation.kt (3)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt (1)
HomeScreen(72-145)app/src/main/java/com/konkuk/medicarecall/ui/navigation/MainNavigator.kt (6)
navigateToMealDetailScreen(136-138)navigateToMedicineDetailScreen(140-142)navigateToSleepDetailScreen(144-146)navigateToStateHealthDetailScreen(148-150)navigateToStateMentalDetailScreen(152-154)navigateToGlucoseDetailScreen(156-158)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/navigation/HomeDetailNavGraph.kt (6)
navigateToMealDetailScreen(15-17)navigateToMedicineDetailScreen(19-21)navigateToSleepDetailScreen(23-25)navigateToStateHealthDetailScreen(27-29)navigateToStateMentalDetailScreen(31-33)navigateToGlucoseDetailScreen(35-37)
app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt (6)
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/meal/screen/MealDetailScreen.kt (1)
MealDetailScreen(36-65)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/medicine/screen/MedicineDetailScreen.kt (1)
MedicineDetailScreen(39-71)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/sleep/screen/SleepDetailScreen.kt (1)
SleepDetailScreen(35-67)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/statehealth/screen/StateHealthDetailScreen.kt (1)
StateHealthDetailScreen(37-82)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/statemental/screen/StateMentalDetailScreen.kt (1)
StateMentalDetailScreen(36-67)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/glucoselevel/screen/GlucoseDetailScreen.kt (1)
GlucoseDetailScreen(53-156)
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/navigation/HomeDetailNavGraph.kt (6)
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/meal/screen/MealDetailScreen.kt (1)
MealDetailScreen(36-65)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/medicine/screen/MedicineDetailScreen.kt (1)
MedicineDetailScreen(39-71)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/sleep/screen/SleepDetailScreen.kt (1)
SleepDetailScreen(35-67)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/statehealth/screen/StateHealthDetailScreen.kt (1)
StateHealthDetailScreen(37-82)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/statemental/screen/StateMentalDetailScreen.kt (1)
StateMentalDetailScreen(36-67)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/glucoselevel/screen/GlucoseDetailScreen.kt (1)
GlucoseDetailScreen(53-156)
🪛 GitHub Actions: Android CI
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/glucoselevel/screen/GlucoseDetailScreen.kt
[error] 124-124: Detekt: Unnecessary long whitespace. [NoMultipleSpaces]
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/navigation/HomeDetailNavGraph.kt
[error] 70-70: Detekt: Missing spacing after "***". [SpacingAroundCurly]
[error] 79-79: Detekt: Missing spacing after "***". [SpacingAroundCurly]
🔇 Additional comments (9)
app/src/main/java/com/konkuk/medicarecall/ui/navigation/Route.kt (1)
48-63: 상세 라우트에 elderId를 올바르게 추가한 것 같아요기존 싱글톤 라우트를
@Serializable data class ...(val elderId: Int)로 바꿔서 네비게이션 계층에서 어르신 ID를 명시적으로 전달할 수 있게 된 구조가 PR 목적과 잘 맞습니다. 다른 부분에서Route.XxxDetail(elderId)로만 생성된다면 특별한 문제 없어 보입니다.app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/meal/screen/MealDetailScreen.kt (2)
39-55: 식사 상세의 elderId 라우트 연동 잘 된 것 같아요
MealDetailScreen(elderId: Int, ...)로 시그니처를 바꾸고,LaunchedEffect(elderId, selectedDate)에서mealViewModel.loadMealsForDate(elderId, selectedDate)를 호출하는 구조라, 어르신 변경/날짜 변경 모두 자연스럽게 다시 로드될 것 같습니다. HomeViewModel 의존성도 깔끔하게 빠져서 괜찮네요.
48-63: 캘린더 연동도 일관성 있게 잘 정리됨
selectedDate를CalendarViewModel에서만 observe 하고,onDateSelected = calendarViewModel::selectDate로 바로 넘기는 패턴이 다른 상세 화면들과도 잘 맞습니다. 재사용성과 가독성 측면에서도 이대로 가면 좋을 것 같습니다.app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt (1)
129-181: 상세 라우트에서elderId추출·전달 흐름 적절함각 상세 composable에서
toRoute<...>()로elderId를 추출해서 화면에 넘기는 구조가 일관적이고, 기존HomeViewModel의존 제거라는 PR 목적에도 잘 맞습니다. 특별히 문제 보이지 않습니다.app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/navigation/HomeDetailNavGraph.kt (1)
43-49: homeDetailNavGraph 내elderId전달 플로우 정합성 OK각 상세 화면 composable에서
backStackEntry.toRoute<...>()로elderId를 꺼내서 곧바로 해당 DetailScreen에 넘기는 구조가 NavGraph.kt와도 일관되고, PR 목표였던 “라우트 기반 elderId 흐름”을 잘 만족시키고 있습니다. 기능적으로는 문제 없어 보입니다.Also applies to: 52-58, 61-67, 88-94
app/src/main/java/com/konkuk/medicarecall/ui/navigation/MainNavigator.kt (1)
136-158: 메인 네비게이터에서elderId필수로 받도록 변경된 부분 👍
navigateTo*DetailScreen(elderId: Int)들이 모두Route.*Detail(elderId)로 직접 네비게이션하도록 바뀐 덕분에, 더 이상 내부에서HomeViewModel상태에 의존하지 않고 호출 시점의 어르신이 명시적으로 전달되네요. 이 PR의 핵심 버그 원인을 잘 짚어낸 변경으로 보입니다.app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/glucoselevel/screen/GlucoseDetailScreen.kt (1)
146-153: 타이밍 변경 시 스크롤을 맨 위로 올리는 UX 처리 좋음
onTimingChange에서glucoseViewModel.updateTiming(newTiming)후에scrollState.scrollTo(0)를 코루틴으로 호출해 리스트를 맨 위로 올리는 동작은 사용성 측면에서 자연스럽고, 타이밍 전환 시 예상 가능한 UX를 만들어 줍니다.app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt (2)
76-81: 공개 API 변경 승인 - elderId 파라미터 추가여섯 개의 상세화면 네비게이션 콜백이
() -> Unit에서(Int) -> Unit으로 변경되어 elderId를 명시적으로 전달할 수 있게 되었습니다. PR 목표(어르신 선택 변경 시 상세화면 데이터 반영)와 일치하는 올바른 변경입니다.
159-164: AI 요약과 코드 불일치AI 요약에서는 "HomeScreenLayout 파라미터 리스트를 (Int) -> Unit으로 변경했다"고 하지만, 실제 코드에서는 이 파라미터들이 여전히
() -> Unit타입입니다(변경되지 않음 -~마커 없음).실제 구현은 올바릅니다: HomeScreen이
(Int) -> Unit콜백을 받아서 selectedElderId로 감싸고, HomeScreenLayout에는() -> Unit형태로 전달하는 구조입니다. 요약 설명만 부정확합니다.
| navigateToMealDetailScreen = { selectedElderId?.let(navigateToMealDetailScreen) }, | ||
| navigateToMedicineDetailScreen = { selectedElderId?.let(navigateToMedicineDetailScreen) }, | ||
| navigateToSleepDetailScreen = { selectedElderId?.let(navigateToSleepDetailScreen) }, | ||
| navigateToStateHealthDetailScreen = { selectedElderId?.let(navigateToStateHealthDetailScreen) }, | ||
| navigateToStateMentalDetailScreen = { selectedElderId?.let(navigateToStateMentalDetailScreen) }, | ||
| navigateToGlucoseDetailScreen = { selectedElderId?.let(navigateToGlucoseDetailScreen) }, |
There was a problem hiding this comment.
🧩 Analysis chain
selectedElderId가 null일 때 동작 확인 필요
selectedElderId?.let(...) 패턴을 사용해 elderId가 null이면 네비게이션이 조용히 실패합니다. 사용자가 컨테이너를 클릭했는데 아무 반응이 없으면 혼란스러울 수 있습니다.
다음을 확인해주세요:
- selectedElderId가 실제로 null이 될 수 있는 시나리오가 있나요?
- null일 때 컨테이너들이 비활성화되거나 사용자에게 피드백이 제공되나요?
확인을 위해 다음 스크립트를 실행해보세요:
🏁 Script executed:
#!/bin/bash
# HomeViewModel에서 selectedElderId의 초기값과 null 가능성 확인
rg -n -A5 -B5 'selectedElderId.*StateFlow|StateFlow.*selectedElderId' --type=ktLength of output: 26
🏁 Script executed:
#!/bin/bash
# Search for selectedElderId declaration in HomeViewModel
rg -n -A5 -B5 'selectedElderId' app/src/main/java/com/konkuk/medicarecall --type=kotlin || \
rg -n -A5 -B5 'selectedElderId' app/src/main/java/com/konkuk/medicarecall -g '*.kt'Length of output: 33097
🏁 Script executed:
#!/bin/bash
# Find HomeViewModel file
fd -e 'kt' -path '*HomeViewModel*' app/src/main/java/com/konkuk/medicarecallLength of output: 168
🏁 Script executed:
#!/bin/bash
# Search for container onClick callbacks around the referenced lines
rg -n -B2 -A2 'onClick.*navigateToMeal|onClick.*navigateTo' app/src/main/java/com/konkuk/medicarecall -g '*.kt' | head -50Length of output: 6472
🏁 Script executed:
#!/bin/bash
# Check if containers have any conditional enable/disable logic
rg -n -B5 -A5 'HomeMealContainer|HomeMedicineContainer|HomeSleepContainer|HomeStateHealthContainer|HomeStateMentalContainer|HomeGlucoseLevelContainer' app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt -g '*.kt' | grep -A5 -B5 'enabled\|isEnabled\|if.*selectedElderId'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check fetchElderList timing and completion
rg -n -A10 'fun fetchElderList' app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.ktLength of output: 566
선택된 어르신이 없는 상태에서 컨테이너 클릭 시 사용자 피드백 부재
selectedElderId는 앱 시작 시 null로 초기화되며, fetchElderList() 비동기 작업 완료 후에 첫 번째 어르신으로 자동 설정됩니다. 이 로딩 구간 동안 사용자가 컨테이너를 클릭하면 { selectedElderId?.let(...) } 패턴으로 인해 네비게이션이 조용히 실패합니다. 현재 컨테이너들은 selectedElderId 상태와 무관하게 항상 활성화되어 있어 사용자에게 피드백이 없습니다.
해결 방안:
- 로딩 중(
homeUiState.isLoading == true)일 때 컨테이너를 비활성화하거나 - 선택된 어르신이 없을 때 컨테이너에 시각적 비활성화 처리(alpha, 그레이아웃 등)를 추가
🤖 Prompt for AI Agents
In
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt
around lines 120-125, the click lambdas silently no-op when selectedElderId is
null causing no user feedback; update the container composables to: 1) derive an
enabled flag (e.g. enabled = !homeUiState.isLoading && selectedElderId != null),
2) pass navigation lambdas only when enabled (or wrap them to show a
Snackbar/Toast when disabled), and 3) apply a visual disabled state (alpha or
gray-out) and disable click handling (use enabled parameter or clickable(enabled
= enabled)) so taps during loading/no-selection provide visible and/or explicit
feedback.
...ava/com/konkuk/medicarecall/ui/feature/homedetail/glucoselevel/screen/GlucoseDetailScreen.kt
Show resolved
Hide resolved
...src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/navigation/HomeDetailNavGraph.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/navigation/HomeDetailNavGraph.kt (1)
43-47: 모든 상세 composable이 route 기반elderId를 일관되게 사용해서 버그 원인은 잘 정리된 듯합니다
- 각 블록에서
val route = backStackEntry.toRoute<...>()→Screen(elderId = route.elderId, onBack = popBackStack)패턴으로 통일되어 있어서, 어떤 상세 화면이든 항상 네비게이션 라우트에 실린 어르신 id를 기준으로 동작하게 된 점이 좋습니다.- PR 설명에 나온 “네임드롭에서 어르신 변경 시 상세에 반영되지 않던 문제”는 이 wiring 변경으로 해결될 것 같아요.
추가로, 여섯 개 블록 모두 동일한 패턴이라 필요하다면 나중에 다음처럼 공통 helper로 뺄 수도 있겠습니다만, 지금도 충분히 읽기 쉬운 수준이라 꼭 바꿀 필요는 없어 보입니다.
예) (아이디어만 참고용)
inline fun <reified T : Route.DetailWithElderId> NavGraphBuilder.elderDetailComposable( noinline content: @Composable (elderId: Int) -> Unit, ) { composable<T> { backStackEntry -> val route = backStackEntry.toRoute<T>() content(route.elderId) } }지금 상태로도 구조는 명확하니, 이 부분은 선택 사항으로만 봐주시면 될 것 같아요.
Also applies to: 52-58, 61-67, 70-76, 79-85, 88-94
📜 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/homedetail/glucoselevel/screen/GlucoseDetailScreen.kt(3 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/navigation/HomeDetailNavGraph.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/glucoselevel/screen/GlucoseDetailScreen.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/navigation/HomeDetailNavGraph.kt (6)
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/meal/screen/MealDetailScreen.kt (1)
MealDetailScreen(36-65)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/medicine/screen/MedicineDetailScreen.kt (1)
MedicineDetailScreen(39-71)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/sleep/screen/SleepDetailScreen.kt (1)
SleepDetailScreen(35-67)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/statehealth/screen/StateHealthDetailScreen.kt (1)
StateHealthDetailScreen(37-82)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/statemental/screen/StateMentalDetailScreen.kt (1)
StateMentalDetailScreen(36-67)app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/glucoselevel/screen/GlucoseDetailScreen.kt (1)
GlucoseDetailScreen(53-156)
🔇 Additional comments (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/homedetail/navigation/HomeDetailNavGraph.kt (2)
6-6:toRoute도입으로 타입 세이프한 라우트 파싱 👍
backStackEntry에서Route.*Detail을 직접 파싱하는 구조라, 기존의 문자열 기반 argument 처리보다 타입 오류를 줄여줄 것 같아요. 이 파일에서의 사용 방식은 자연스럽습니다.
15-37: 모든 상세 화면 이동 함수 호출이elderId인자를 정상적으로 전달하고 있습니다검색 결과 HomeNavigation.kt의 모든 6개 호출 지점이
elderId를 명시적으로 넘기고 있으며, HomeDetailNavGraph.kt와 MainNavigator.kt의 함수 정의도 모두elderId: Int파라미터를 포함하고 있습니다. 함수 시그니처 변경으로 인한 잔여 호출이나 누락된 인자는 없는 상태입니다.
ikseong00
left a comment
ikseong00
left a comment
There was a problem hiding this comment.
음... 이 부분은 매번 네비게이션 시에 인자를 주고받는 것 같은데,
선택된 ElderId 를 가지고 있는 HomeViewModel 을 재사용하는 건 어떨까요 ??
HomeScreen 에서도 네비게이션 시에 null 체크를 계속 하고 있어서 불필요한 감이 없지않아 있습니다.
https://github.com/Project-Unifest/unifest-android/blob/develop/core/common/src/main/kotlin/com/unifest/android/core/common/extension/NavBackStackEntry.kt
https://youtu.be/h61Wqy3qcKg?si=OqctoATR5MGbypOW
위 링크들 참고하시면 됩니다!!
alswlekk
left a comment
alswlekk
left a comment
There was a problem hiding this comment.
상세 화면들이 HomeViewModel.selectedElderId 에 의존해서 그렇다고 원인을 작성해주셨었는데, 그러면 어르신이 변경됐을 때 해당값은 변경이 안되는건가용...?? 원래 선택된 어르신이 변경되면 저값도 변경되니까 정상적으로 뜨지 않나라는 생각이 들어서요
|
@ikseong00 리뷰 감사합니다! 제가 이해한 건 홈 화면과 상세 화면에서 서로 다른 HomeViewModel을 보고 있어서 남겨주신 내용을 보고 생각해봤을때 home과 detail의 navgraph가 따로 있어서 뷰모델이 공유가 안된것같은데 |
|
@alswlekk 리뷰 감사합니다! |
|
@alswlekk @ikseong00 리뷰답글 확인부탁드려요! |
@librawish808 아하... 넵 말씀하신 내용 확인했습니다. |
ProtossManse
left a comment
ProtossManse
left a comment
There was a problem hiding this comment.
말씀해주신 문제는 elderId 관련 문제 해결하고 한번 더 살펴봐야 할 것 같긴 합니다!
수고하셨습니다~
|
@alswlekk 넵 해결되었습니다 이번 pr은 머지하고 해주신 리뷰는 추후 리팩토링 과정에서 반영하도록 하겠습니다! |
|
@ProtossManse 넵! 감사합니다 |
🔗 관련 이슈
📙 작업 설명
문제
AS-IS : 어르신 드롭다운으로 정대식(1043) → 상세화면 진입하면 여전히 양정애(1042) 기준
원인
해결
상세 화면을 route 기반 elderId 구조로 변경해 어르신 변경 미반영 문제 해결,
혈당 페이지네이션 로직 수정했습니다.
📸 스크린샷 또는 시연 영상 (선택)
Screen_recording_20251116_161751.webm
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
릴리즈 노트