Conversation
Walkthrough새 ModalBottomSheet 컴포저블과 개별 시술 항목 컴포저블, 불변 데이터 모델 및 삭제 아이콘 드로어블이 추가되어 선택된 시술 목록 표시, 항목 삭제, "다음" 액션 및 닫기 콜백을 제공한다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Host as Host/Screen
participant UI as SelectedProcedureBottomSheet
participant Item as SelectedProcedureItem
User->>Host: 바텀시트 열기 요청
Host->>UI: isVisible=true / sheetState 업데이트
UI->>Item: selectedProcedure 리스트로 항목 렌더링
User->>Item: 삭제 아이콘 클릭 (procedureId)
Item->>Host: onDeletedClick(procedureId)
User->>UI: 다음 버튼 클릭
UI->>Host: onButtonClick()
User->>UI: 바텀시트 닫기(뒤로/배경)
UI->>Host: onDismiss()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.kt:
- Around line 61-67: The derivedStateOf for isLastItemVisible currently only
tracks reads from listState and won’t re-evaluate when the parameter
selectedProcedure (its size) changes; replace this with a snapshotFlow-based
approach that explicitly observes both the listState.layoutInfo.visibleItemsInfo
(or its last visible index) and selectedProcedure.size, map them to the Boolean
(lastVisibleIndex == selectedProcedure.size - 1), and collectAsState to produce
isLastItemVisible; alternatively ensure selectedProcedure.size is read from a
State object inside the derived computation so the derivedStateOf will re-run
when the list changes (referencing isLastItemVisible, derivedStateOf,
selectedProcedure.size, and listState.layoutInfo.visibleItemsInfo).
🧹 Nitpick comments (3)
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt (2)
49-57: 접근성(Accessibility)을 위해contentDescription추가를 권장합니다.삭제 아이콘에
contentDescription = null이 설정되어 있어 스크린 리더 사용자가 버튼의 용도를 알 수 없습니다.♻️ 권장 수정
Icon( imageVector = ImageVector.vectorResource(id = R.drawable.ic_deletebox), - contentDescription = null, + contentDescription = "삭제", tint = CherrishTheme.colors.gray600, modifier = Modifier .align(Alignment.CenterEnd) .noRippleClickable(onClick = { onDeletedClick(procedureId) }) .padding(vertical = 5.dp) )
68-91: 긴 시술명에서 삭제 아이콘과 겹칠 수 있습니다.
SelectedProcedureTitle의Row가fillMaxWidth()를 사용하고 있어, 시술명이 길 경우 오른쪽에 배치된 삭제 아이콘과 겹칠 수 있습니다. 삭제 아이콘 영역을 고려한end패딩 추가를 권장합니다.♻️ 권장 수정
Row( - modifier = modifier - .fillMaxWidth(), + modifier = modifier + .fillMaxWidth() + .padding(end = 32.dp), verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(12.dp) )app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.kt (1)
146-184: 그라데이션 오버레이 구현이 적절합니다.스크롤 경계 표시를 위한 그라데이션 UX가 잘 구현되어 있습니다. 다만,
49.dp,68.dp등의 매직 넘버를 상수로 추출하면 유지보수성이 향상됩니다.♻️ 선택적 개선
private object BottomSheetDimens { val HeaderHeight = 49.dp val ButtonAreaHeight = 68.dp val GradientHeight = 70.dp }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.ktapp/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.ktapp/src/main/java/com/cherrish/android/presentation/calendar/procedure/model/SelectedProcedureModel.ktapp/src/main/res/drawable/ic_deletebox.xml
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: - Jetpack Compose 구조, 상태 관리, recomposition 최적화에 집중
- ViewModel, UiState, 단방향 데이터 흐름 패턴 검토
- 불필요한 recomposition 가능성, remember/derivedStateOf 적절한 사용 확인
- 네이밍 컨벤션, 가독성, Google 권장 Android 아키텍처 준수 여부
Files:
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/model/SelectedProcedureModel.ktapp/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.ktapp/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt
🧠 Learnings (1)
📚 Learning: 2026-01-12T19:49:27.085Z
Learnt from: nhyeonii
Repo: TEAM-Cherrish/Cherrish-Android PR: 41
File: app/src/main/java/com/cherrish/android/presentation/challenge/ChallengeScreen.kt:30-39
Timestamp: 2026-01-12T19:49:27.085Z
Learning: When a Jetpack Compose screen composable receives a Scaffold paddingValues: PaddingValues parameter (commonly from Scaffold), apply it to the root container's modifier first (e.g., .padding(paddingValues)) before applying any additional padding. This ensures content respects system bars (status/navigation) and avoids layout overlap. This pattern should be followed across presentation screens under app/src/main/java/com/cherrish/android/presentation/ to maintain consistent insets handling.
Applied to files:
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/model/SelectedProcedureModel.ktapp/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.ktapp/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt
🧬 Code graph analysis (2)
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.kt (3)
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt (1)
SelectedProcedureItem(24-59)app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.kt (1)
CherrishButton(27-79)app/src/main/java/com/cherrish/android/core/designsystem/theme/Theme.kt (1)
CherrishTheme(38-61)
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt (1)
app/src/main/java/com/cherrish/android/core/designsystem/theme/Theme.kt (1)
CherrishTheme(38-61)
⏰ 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). (2)
- GitHub Check: PR Build Check
- GitHub Check: PR Lint Check
🔇 Additional comments (5)
app/src/main/res/drawable/ic_deletebox.xml (1)
1-18: LGTM!벡터 드로어블 구조가 적절합니다. 하드코딩된 색상(#9FA4A9)은
SelectedProcedureItem.kt에서tint속성을 통해 테마 색상으로 오버라이드되므로 문제없습니다.app/src/main/java/com/cherrish/android/presentation/calendar/procedure/model/SelectedProcedureModel.kt (1)
5-11: LGTM!
@Immutable어노테이션과data class조합이 Compose의 안정성(stability) 최적화에 적합합니다.ImmutableList<SelectedProcedureModel>와 함께 사용되어 불필요한 recomposition을 방지합니다.app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt (1)
94-106: LGTM!Preview가
CherrishTheme으로 올바르게 래핑되어 있고 샘플 데이터가 적절합니다.app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.kt (2)
136-143: 빈 목록 상태에서 버튼 비활성화를 고려해 주세요.
selectedProcedure가 비어있을 때도 "다음" 버튼이 활성화되어 있습니다. 선택된 시술이 없는 상태에서 다음 단계로 진행하는 것이 의도된 동작인지 확인이 필요합니다.♻️ 비활성화가 필요한 경우 수정 예시
CherrishButton( text = "다음", onClick = onButtonClick, + enabled = selectedProcedure.isNotEmpty(), modifier = Modifier .fillMaxWidth() .padding(horizontal = 24.dp) .padding(bottom = 20.dp) )
120-131: LGTM!
items에서key = { it.procedureId }를 사용하여 리스트 항목의 안정적인 식별을 보장하고 있습니다. 이는 항목 추가/삭제 시 불필요한 recomposition을 방지하는 좋은 패턴입니다.
usuuhyn
left a comment
usuuhyn
left a comment
There was a problem hiding this comment.
수고하셨습니당 ! !!! ! ! ! 🐒🐒🐒🍒
| modifier = Modifier | ||
| .align(Alignment.CenterEnd) | ||
| .noRippleClickable(onClick = { onDeletedClick(procedureId) }) | ||
| .padding(vertical = 5.dp) |
There was a problem hiding this comment.
P3: vertical만 패딩 값 주신 이유가 있나용?? 박스 기준으로 생각해도 조금 헷갈려서 여쭤봅니당 !
hyeminililo
left a comment
hyeminililo
left a comment
There was a problem hiding this comment.
작업속도 완전 빠르시네용 ~~!! 궁금한 것 있습니다아 !!!
| } | ||
| } | ||
|
|
||
| if (sheetState.isVisible) { |
There was a problem hiding this comment.
P4: 위에서 LanchedEffect를 통해 사이드 이펙트를 관리한 것같은데 한 번 if문으로 제어해주신 이유가 궁금합니다 !
There was a problem hiding this comment.
LaunchedEffect와 if (sheetState.isVisible)를 함께 사용한 이유는 두 가지 다른 목적을 위해서 입니당
- LaunchedEffect(isVisible) - 외부 상태 → Sheet 상태 동기화
LaunchedEffect(isVisible) {
if (isVisible) {
sheetState.show() // 외부에서 true가 되면 Sheet 열기
} else {
sheetState.hide() // 외부에서 false가 되면 Sheet 닫기
}
}
ViewModel이나 부모 Composable에서 isVisible 상태를 변경하면 Sheet가 열리거나 닫힘
side effect: 상태 변경에 따라 애니메이션을 트리거
- if (sheetState.isVisible) - 불필요한 컴포저블 생성 방지
if (sheetState.isVisible) {
ModalBottomSheet { ... } // Sheet가 실제로 표시될 때만 컴포저블 생성
}
Sheet가 완전히 닫힌 후에는 컴포저블 자체를 생성하지 않아 성능 최적화
메모리 절약
LaunchedEffect만 사용한다면 Sheet는 항상 존재하지만 보이지 않을 뿐 (메모리 낭비) 이라는 문제가, if문만 사용한다면 외부 상태 변경을 감지 못해 애니메이션 타이밍 문제 발생 가능성이 존재하게 됩니당 따라서 외부 상태 동기화 + 성능 최적화 모두를 위해 작성을 한 것이라고 생각하시면 됩니다 !
| onDeletedClick: (Long) -> Unit, | ||
| modifier: Modifier = Modifier | ||
| ) { | ||
| Box( |
There was a problem hiding this comment.
p3: 좌, 우 정렬 목적의 레이아웃이라 Box에서 각 컴포넌트에 align을 주기보다는 Row + Arrangement.SpaceBetween이 레이아웃 의도를 더 잘 드러낼 것 같습니다!
또한 각 컴포넌트에 개별 vertical padding을 주는 대신 verticalAlignment = CenterVertically로 정렬을 통일하고 아이콘 기준으로만 padding을 두는 쪽이 유지보수 측면에서도 좋아 보입니다!
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(vertical = 9.dp, horizontal = 24.dp), | ||
| textAlign = TextAlign.Start |
There was a problem hiding this comment.
p3: 단순 궁금인데여! 이거 안 써도 align이 start 아닌가요??
There was a problem hiding this comment.
마자욯ㅎㅎ 근데 명시적으로 나타내고 시퍼서 한번더 썻던건긴한데 지우꼐요
| color = CherrishTheme.colors.gray400 | ||
| ) | ||
|
|
||
| Spacer(modifier = Modifier.height(14.dp)) |
There was a problem hiding this comment.
p3: 이 부분 HorizontalDivider 14.dp 높이 밑으로만큼은 스크롤 안 먹는건가요??
아니면 LazyColumn에 contentPadding 줘야 할 것 같습니다!
There was a problem hiding this comment.
허거덩 맞네요 !!! 확인햇습니다 !!
|
|
||
| val configuration = LocalConfiguration.current | ||
| val screenHeightDp = configuration.screenHeightDp.dp | ||
| val maxSheetHeight = screenHeightDp * 0.36f |
There was a problem hiding this comment.
p3: (단순 궁금)0.36f 어떻게 계산하신거예요??
There was a problem hiding this comment.
이거 피그마에 있는 전체 화면에서 바텀시트 비율 정해서 작성한거에용 !!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.kt`:
- Around line 78-84: SelectedProcedureBottomSheet.kt has an edge case where
selectedProcedure being empty yields lastItemIndex = -1 and causes
isLastItemVisible to be incorrect; update the derivedStateOf in
isLastItemVisible to first check selectedProcedure.isNotEmpty() (or compute
lastItemIndex and if it's < 0 return false) and only then compare
lastVisibleItem?.index == lastItemIndex so the bottom gradient won't show for an
empty list; reference the isLastItemVisible val, selectedProcedure,
listState.layoutInfo.visibleItemsInfo, and lastItemIndex when making this
change.
🧹 Nitpick comments (3)
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt (2)
47-54: 삭제 아이콘의 접근성 향상이 필요합니다.
contentDescription = null로 설정되어 있어 TalkBack 등 스크린 리더 사용자가 삭제 버튼을 인식할 수 없습니다.♻️ 권장 수정
Icon( imageVector = ImageVector.vectorResource(id = R.drawable.ic_deletebox), - contentDescription = null, + contentDescription = "시술 삭제", tint = CherrishTheme.colors.gray600, modifier = Modifier .noRippleClickable(onClick = { onDeletedClick(procedureId) }) - )
82-86: 문자열 리소스 사용을 권장합니다.하드코딩된 텍스트
"다운타임* $minDowntimeDays-$maxDowntimeDays"는 향후 다국어 지원을 위해 string resource로 분리하면 좋습니다.app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.kt (1)
163-180: 그라데이션 위치의 매직 넘버를 상수로 추출하는 것을 권장합니다.
49.dp,68.dp등 하드코딩된 값들은 헤더나 버튼 크기가 변경될 때 유지보수가 어려울 수 있습니다. 상수로 추출하거나 관련 컴포넌트 크기와 연동하면 더 안전합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.ktapp/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: - Jetpack Compose 구조, 상태 관리, recomposition 최적화에 집중
- ViewModel, UiState, 단방향 데이터 흐름 패턴 검토
- 불필요한 recomposition 가능성, remember/derivedStateOf 적절한 사용 확인
- 네이밍 컨벤션, 가독성, Google 권장 Android 아키텍처 준수 여부
Files:
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.ktapp/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt
🧠 Learnings (1)
📚 Learning: 2026-01-12T19:49:27.085Z
Learnt from: nhyeonii
Repo: TEAM-Cherrish/Cherrish-Android PR: 41
File: app/src/main/java/com/cherrish/android/presentation/challenge/ChallengeScreen.kt:30-39
Timestamp: 2026-01-12T19:49:27.085Z
Learning: When a Jetpack Compose screen composable receives a Scaffold paddingValues: PaddingValues parameter (commonly from Scaffold), apply it to the root container's modifier first (e.g., .padding(paddingValues)) before applying any additional padding. This ensures content respects system bars (status/navigation) and avoids layout overlap. This pattern should be followed across presentation screens under app/src/main/java/com/cherrish/android/presentation/ to maintain consistent insets handling.
Applied to files:
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.ktapp/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt
🧬 Code graph analysis (1)
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.kt (3)
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt (1)
SelectedProcedureItem(23-56)app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.kt (1)
CherrishButton(27-79)app/src/main/java/com/cherrish/android/core/designsystem/theme/Theme.kt (1)
CherrishTheme(38-61)
⏰ 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). (2)
- GitHub Check: PR Build Check
- GitHub Check: PR Lint Check
🔇 Additional comments (4)
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureItem.kt (1)
90-102: LGTM!Preview가 CherrishTheme으로 적절히 래핑되어 있고, 샘플 데이터로 UI를 검증할 수 있습니다.
app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.kt (3)
45-64: 상태 동기화 패턴이 적절합니다.
LaunchedEffect(isVisible)를 통한 외부 상태와sheetState간의 동기화, 그리고if (sheetState.isVisible)조건부 렌더링을 통한 성능 최적화가 잘 구현되어 있습니다.ImmutableList사용으로 Compose 안정성도 확보되었습니다.
137-148: LGTM!
key = { it.procedureId }를 사용하여 안정적인 키를 제공하고 있어 리스트 변경 시 불필요한 recomposition을 방지합니다.
207-286: Preview가 상호작용 시나리오를 잘 보여줍니다.삭제 동작, 빈 리스트 시 자동 닫힘 등 실제 사용 시나리오를 포함한 Preview로 컴포넌트 동작을 효과적으로 검증할 수 있습니다.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| val isLastItemVisible = remember(selectedProcedure.size) { | ||
| derivedStateOf { | ||
| val lastVisibleItem = listState.layoutInfo.visibleItemsInfo.lastOrNull() | ||
| val lastItemIndex = selectedProcedure.size - 1 | ||
| lastVisibleItem?.index == lastItemIndex | ||
| } | ||
| } |
There was a problem hiding this comment.
빈 리스트 처리 시 edge case가 존재합니다.
selectedProcedure가 빈 리스트일 경우 lastItemIndex가 -1이 되어 isLastItemVisible이 항상 false가 됩니다. 이로 인해 리스트가 비어있어도 하단 그라데이션이 표시될 수 있습니다.
🐛 권장 수정
val isLastItemVisible = remember(selectedProcedure.size) {
derivedStateOf {
+ if (selectedProcedure.isEmpty()) return@derivedStateOf true
val lastVisibleItem = listState.layoutInfo.visibleItemsInfo.lastOrNull()
val lastItemIndex = selectedProcedure.size - 1
lastVisibleItem?.index == lastItemIndex
}
}🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/SelectedProcedureBottomSheet.kt`
around lines 78 - 84, SelectedProcedureBottomSheet.kt has an edge case where
selectedProcedure being empty yields lastItemIndex = -1 and causes
isLastItemVisible to be incorrect; update the derivedStateOf in
isLastItemVisible to first check selectedProcedure.isNotEmpty() (or compute
lastItemIndex and if it's < 0 return false) and only then compare
lastVisibleItem?.index == lastItemIndex so the bottom gradient won't show for an
empty list; reference the isLastItemVisible val, selectedProcedure,
listState.layoutInfo.visibleItemsInfo, and lastItemIndex when making this
change.
4 participants
Related issue 🛠
Work Description ✏️
Screenshot 📸
Uncompleted Tasks 😅
To Reviewers 📢
시술 선택 바텀시트를 구현했어요!
Summary by CodeRabbit
새로운 기능
✏️ Tip: You can customize this high-level summary in your review settings.