Conversation
Walkthrough타이틀과 선택적 설명, 2열 그리드로 구성된 새 Compose 섹션 Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Section as CherrishSelectionSection
participant Grid as SelectionChipGrid
participant Item as CherrishSelectionChip / CherrishMissionCard
participant Host as Parent (onItemClick handler)
User->>Section: 화면 표시 / 상호작용
Section->>Grid: items, chipType, selectedIndex 전달
Grid->>Item: 각 아이템 렌더링
User->>Item: 아이템 클릭
Item->>Section: 클릭 인덱스 전달
Section->>Host: onItemClick(index) 호출
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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: 0
🧹 Nitpick comments (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt (2)
57-72:LazyVerticalGrid를 스크롤 비활성화 상태로 사용 시 레이아웃 문제 가능성
userScrollEnabled = false로 설정된LazyVerticalGrid가 고정 높이 없이 사용되고 있습니다. 이 컴포넌트가 스크롤 가능한 부모(예:LazyColumn, scrollableColumn) 내부에 배치될 경우, lazy 컴포저블의 측정 방식으로 인해 레이아웃 문제가 발생할 수 있습니다.아이템 수가 적고(2-6개) 스크롤이 필요 없다면, 일반
Column과Row조합을 사용하는 것이 lazy composition 오버헤드를 피하고 레이아웃 안정성을 높일 수 있습니다.♻️ 대안 구현 제안
@Composable private fun SelectionChipGrid( items: List<String>, modifier: Modifier = Modifier, selectedIndex: Int? = null, onItemClick: (index: Int) -> Unit ) { Column( modifier = modifier.fillMaxWidth(), verticalArrangement = Arrangement.spacedBy(12.dp) ) { items.chunked(2).forEachIndexed { rowIndex, rowItems -> Row( modifier = Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.spacedBy(12.dp) ) { rowItems.forEachIndexed { columnIndex, text -> val index = rowIndex * 2 + columnIndex CherrishSelectionChip( text = text, onClick = { onItemClick(index) }, modifier = Modifier.weight(1f), isSelected = selectedIndex == index ) } // 홀수 개일 때 빈 공간 채우기 if (rowItems.size == 1) { Spacer(modifier = Modifier.weight(1f)) } } } } }
Row와Column사용 시 추가 import 필요:import androidx.compose.foundation.layout.Row
105-105: Preview 변수명이 실제 타입과 불일치
isSelected는 boolean을 암시하지만 실제로는 인덱스 값을 저장합니다. 컴포넌트의 파라미터명selectedIndex와 일관성을 위해 변수명을 수정하는 것이 좋습니다.♻️ 제안된 수정
- var isSelected by remember { mutableIntStateOf(-1) } + var selectedIndex by remember { mutableIntStateOf(-1) } CherrishSelectionSection( ... - selectedIndex = isSelected, - onItemClick = { isSelected = it } + selectedIndex = selectedIndex, + onItemClick = { selectedIndex = it } )Also applies to: 121-121
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.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/core/designsystem/component/section/CherrishSelectionSection.kt
🧬 Code graph analysis (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/chip/CherrishSelectionChip.kt (1)
CherrishSelectionChip(18-42)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 Lint Check
- GitHub Check: PR Build Check
🔇 Additional comments (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt (2)
23-48: LGTM!컴포저블 구조가 잘 설계되어 있습니다. 파라미터 순서가 Compose 컨벤션을 따르고 있으며, 상태 호이스팅이 적절하게 구현되어
selectedIndex와onItemClick을 통해 단방향 데이터 흐름을 지원합니다.
75-99: LGTM!
TitleDescriptionSection구현이 깔끔합니다.isNullOrBlank()검사로 null과 빈 문자열 모두 적절히 처리하고 있으며, 테마 타이포그래피와 색상 사용이 일관적입니다.
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/core/designsystem/component/section/CherrishSelectionSection.kt:
- Around line 58-95: SelectionChipGrid currently uses LazyVerticalGrid (a
scrollable/lazy container) which can crash when nested in other scrollables and
lacks stable keys; replace the LazyVerticalGrid with a non-lazy two-column
layout built from Column and Rows (e.g., chunk the items into pairs and render
each row with two children) so the component is safe to embed inside other
scroll containers, keep user scrolling disabled by not using any lazy/scrollable
composable, and preserve selection consistency by supplying stable keys for
items (replace itemsIndexed usage with a keyed iteration or ensure keys are
derived from the item value or a stable id) — update SelectionChipGrid, its
itemsIndexed usage, and the places creating rows so CherrishSelectionChip /
CherrishMissionCard rendering remains identical otherwise.
🧹 Nitpick comments (3)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt (3)
24-27:CherrishSectionChipType는 OK — KDoc만 있으면 더 좋아요.
외부에 노출되는 enum이라, 각 값이 어떤 UI를 의미하는지 짧은 KDoc 주석만 추가하면 사용처에서 실수 줄이기 좋습니다.
29-56:CherrishSelectionSection의modifier.fillMaxWidth()가 호출자 제어를 덮을 수 있어요.
현재modifier를 받아도 내부에서 항상fillMaxWidth()를 강제합니다( Line 40 ). 디자인 시스템 컴포넌트면 “기본은 fillMaxWidth, 필요 시 호출자가 override”가 더 안전해서, 기본값을Modifier.fillMaxWidth()로 두고 내부에선modifier를 그대로 쓰는 쪽을 권장합니다. (Compose 가이드라인의 재사용/조합성 관점)
97-121: 섹션 타이틀에 heading semantics 고려해볼 만합니다.
디자인 시스템 컴포넌트라 접근성까지 챙기려면, 제목 Text에Modifier.semantics { heading() }를 부여하는 선택지도 있습니다(칩/카드 선택 UI의 탐색성이 좋아짐).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.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/core/designsystem/component/section/CherrishSelectionSection.kt
⏰ 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: PR Build Check
🔇 Additional comments (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt (1)
123-155: Preview 문구(“복수 선택”)와 API(selectedIndex: Int?)가 불일치합니다 — 요구사항 확인 필요.
미션 카드가 실제로 다중 선택이면selectedIndices: Set<Int>같은 형태로 API가 바뀌어야 합니다. 단일 선택이 맞다면, Preview 문구를 단일 선택에 맞게 조정하는 게 혼동이 적습니다.
| @Composable | ||
| private fun SelectionChipGrid( | ||
| items: List<String>, | ||
| chipType: CherrishSectionChipType, | ||
| onItemClick: (index: Int) -> Unit, | ||
| modifier: Modifier = Modifier, | ||
| selectedIndex: Int? = null | ||
| ) { | ||
| LazyVerticalGrid( | ||
| modifier = modifier.fillMaxWidth(), | ||
| columns = GridCells.Fixed(2), | ||
| horizontalArrangement = Arrangement.spacedBy(12.dp), | ||
| verticalArrangement = Arrangement.spacedBy(12.dp), | ||
| userScrollEnabled = false | ||
| ) { | ||
| itemsIndexed(items) { index, text -> | ||
| when (chipType) { | ||
| CherrishSectionChipType.SELECTION_CHIP -> { | ||
| CherrishSelectionChip( | ||
| text = text, | ||
| onClick = { onItemClick(index) }, | ||
| modifier = Modifier.fillMaxWidth(), | ||
| isSelected = selectedIndex == index | ||
| ) | ||
| } | ||
|
|
||
| CherrishSectionChipType.MISSION_CARD -> { | ||
| CherrishMissionCard( | ||
| text = text, | ||
| onClick = { onItemClick(index) }, | ||
| modifier = Modifier.fillMaxWidth(), | ||
| isSelected = selectedIndex == index | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
LazyVerticalGrid는 스크롤 컨테이너 안에서 런타임 크래시 위험이 커서(중첩 스크롤) 공용 컴포넌트로는 위험합니다.
이 섹션이 LazyColumn/verticalScroll 내부에 들어가면 “무한 높이로 측정” 문제로 크래시 나는 케이스가 흔합니다. userScrollEnabled = false여도 “스크롤러블 레이아웃”인 점은 변하지 않는 경우가 많아서, 공용 컴포넌트라면 비-lazy 2열 레이아웃(아이템 수가 작은 전제) 로 바꾸는 쪽을 추천드립니다.
또한 itemsIndexed(items)에 key가 없어 리스트가 바뀔 때 선택 상태가 어긋날 여지도 있습니다.
제안 변경안 (중첩 스크롤 안전한 2열 Row/Column 그리드)
@@
-import androidx.compose.foundation.lazy.grid.GridCells
-import androidx.compose.foundation.lazy.grid.LazyVerticalGrid
-import androidx.compose.foundation.lazy.grid.itemsIndexed
+import androidx.compose.foundation.layout.Row
+import androidx.compose.foundation.layout.width
@@
private fun SelectionChipGrid(
items: List<String>,
chipType: CherrishSectionChipType,
onItemClick: (index: Int) -> Unit,
modifier: Modifier = Modifier,
selectedIndex: Int? = null
) {
- LazyVerticalGrid(
- modifier = modifier.fillMaxWidth(),
- columns = GridCells.Fixed(2),
- horizontalArrangement = Arrangement.spacedBy(12.dp),
- verticalArrangement = Arrangement.spacedBy(12.dp),
- userScrollEnabled = false
- ) {
- itemsIndexed(items) { index, text ->
- when (chipType) {
- CherrishSectionChipType.SELECTION_CHIP -> {
- CherrishSelectionChip(
- text = text,
- onClick = { onItemClick(index) },
- modifier = Modifier.fillMaxWidth(),
- isSelected = selectedIndex == index
- )
- }
-
- CherrishSectionChipType.MISSION_CARD -> {
- CherrishMissionCard(
- text = text,
- onClick = { onItemClick(index) },
- modifier = Modifier.fillMaxWidth(),
- isSelected = selectedIndex == index
- )
- }
- }
- }
- }
+ Column(
+ modifier = modifier.fillMaxWidth(),
+ verticalArrangement = Arrangement.spacedBy(12.dp),
+ ) {
+ items.chunked(2).forEachIndexed { rowIndex, row ->
+ Row(
+ modifier = Modifier.fillMaxWidth(),
+ horizontalArrangement = Arrangement.spacedBy(12.dp),
+ ) {
+ row.forEachIndexed { colIndex, text ->
+ val index = rowIndex * 2 + colIndex
+ val cellModifier = Modifier.weight(1f)
+ when (chipType) {
+ CherrishSectionChipType.SELECTION_CHIP -> CherrishSelectionChip(
+ text = text,
+ onClick = { onItemClick(index) },
+ modifier = cellModifier,
+ isSelected = selectedIndex == index,
+ )
+ CherrishSectionChipType.MISSION_CARD -> CherrishMissionCard(
+ text = text,
+ onClick = { onItemClick(index) },
+ modifier = cellModifier,
+ isSelected = selectedIndex == index,
+ )
+ }
+ }
+ if (row.size == 1) {
+ Spacer(modifier = Modifier.weight(1f))
+ }
+ }
+ }
+ }
}🤖 Prompt for AI Agents
In
@app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt
around lines 58 - 95, SelectionChipGrid currently uses LazyVerticalGrid (a
scrollable/lazy container) which can crash when nested in other scrollables and
lacks stable keys; replace the LazyVerticalGrid with a non-lazy two-column
layout built from Column and Rows (e.g., chunk the items into pairs and render
each row with two children) so the component is safe to embed inside other
scroll containers, keep user scrolling disabled by not using any lazy/scrollable
composable, and preserve selection consistency by supplying stable keys for
items (replace itemsIndexed usage with a keyed iteration or ensure keys are
derived from the item value or a stable id) — update SelectionChipGrid, its
itemsIndexed usage, and the places creating rows so CherrishSelectionChip /
CherrishMissionCard rendering remains identical otherwise.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt (1)
62-68: 중첩 스크롤 환경에서의 높이 제약 검토 필요
LazyVerticalGrid에userScrollEnabled = false를 설정했지만, 명시적인 높이 제약이 없습니다. 이 섹션이LazyColumn등 스크롤 가능한 부모 컨테이너 내에서 사용될 경우, 레이아웃 측정 이슈나 중첩 스크롤 문제가 발생할 수 있습니다.사용 환경에 따라
heightIn()또는 아이템 수 기반 동적 높이 계산을 고려해 보세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.ktapp/src/main/java/com/cherrish/android/core/designsystem/component/type/CherrishSectionChipType.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/core/designsystem/component/type/CherrishSectionChipType.ktapp/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt
🧬 Code graph analysis (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt (3)
app/src/main/java/com/cherrish/android/core/designsystem/component/chip/CherrishSelectionChip.kt (1)
CherrishSelectionChip(18-42)app/src/main/java/com/cherrish/android/core/designsystem/component/chip/CherrishSelectionMissionChip.kt (1)
CherrishMissionCard(27-70)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/core/designsystem/component/type/CherrishSectionChipType.kt (1)
3-6: LGTM!enum 정의가 깔끔하고 명확합니다. 별도 파일로 분리하여 재사용성을 높인 점이 좋습니다.
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt (3)
25-52: LGTM!컴포넌트 구조가 깔끔하고, 파라미터 순서가 Compose 컨벤션(필수 파라미터 → modifier → 옵션 파라미터)을 잘 따르고 있습니다.
93-117: LGTM!
isNullOrBlank()체크를 통해 빈 문자열까지 처리하는 점이 좋습니다. 테마 색상과 타이포그래피 사용이 일관적입니다.
119-134: LGTM!
mutableIntStateOf를 사용하여 primitive int에 최적화된 상태 관리를 적용한 점이 좋습니다. Preview 구성이 적절합니다.
| title = "챌린지 기간 동안\n진행할 미션을 선택해주세요.", | ||
| description = "복수 선택이 가능해요.", | ||
| items = listOf("반신욕 20분", "진정 토너+세럼", "피부결 정돈", "괄사", "톤 개선", "선크림 바르기"), | ||
| selectedIndex = isSelected, | ||
| onItemClick = { isSelected = it }, |
There was a problem hiding this comment.
Preview 설명과 구현 간 불일치
Preview의 description이 "복수 선택이 가능해요"라고 안내하지만, 현재 구현은 selectedIndex: Int?로 단일 선택만 지원합니다.
실제 요구사항이 복수 선택이라면 selectedIndices: Set<Int> 또는 List<Int>로 변경이 필요하고, 단일 선택이 의도된 것이라면 Preview의 description 텍스트를 수정해야 합니다.
nhyeonii
left a comment
nhyeonii
left a comment
There was a problem hiding this comment.
굿굿띠 ~~ 깔끔띠 ~~~ 코드리뷰 확인 부탁해용 ㅎㅎㅎ 🫰
|
|
||
| @Composable | ||
| private fun SelectionChipGrid( | ||
| items: List<String>, |
There was a problem hiding this comment.
P1 : 요거 immutableList 사용해줍시닿ㅎㅎㅎㅎ immutableList와 List의 차이점이 무엇일까요오옹 ~~ ?!?!?
There was a problem hiding this comment.
넵 !!
List는 내부 값이 변경될 수 있는 반면,
ImmutableList는 생성 이후 값이 변경되지 않는 불변 리스트입니다.
ImmutableList는 값이 변경될 경우 새 리스트를 생성하는 방식이라
Compose에서 상태 변경을 명확하게 인지할 수 있고
불필요한 recomposition을 방지해 상태 안정성이 높습니다 ! ! ! !
또한 UI에서 사용하는 읽기 전용 데이터라는 의도를 명확하게 표현할 수 있다는 특징이 있습니당 ~~
| private fun SelectionChipGrid( | ||
| items: List<String>, | ||
| chipType: CherrishSectionChipType, | ||
| onItemClick: (index: Int) -> Unit, |
There was a problem hiding this comment.
요거 index 들어간다고 넣어주느거 참 좋타 ㅎ.ㅎ 🫰
| @Composable | ||
| fun CherrishSelectionSection( | ||
| title: String, | ||
| items: List<String>, |
There was a problem hiding this comment.
P1 : 요거 immutableList 사용해쥬세요오옹ㅇ ~~ ~!!
| Spacer(modifier = Modifier.height(4.dp)) | ||
| Text( | ||
| text = description, | ||
| style = CherrishTheme.typography.body1R14, |
There was a problem hiding this comment.
P1 : 요거 설명 글씨체가 다 달라서 상위에서 받아오도록 로직 수정해야할것 같숨둥 ~~ 🫰
| CherrishSelectionSection( | ||
| title = "챌린지 기간 동안\n진행할 미션을 선택해주세요.", | ||
| description = "복수 선택이 가능해요.", | ||
| items = listOf("반신욕 20분", "진정 토너+세럼", "피부결 정돈", "괄사", "톤 개선", "선크림 바르기"), |
There was a problem hiding this comment.
| items = listOf("반신욕 20분", "진정 토너+세럼", "피부결 정돈", "괄사", "톤 개선", "선크림 바르기"), | |
| items = persistentListOf("반신욕 20분", "진정 토너+세럼", "피부결 정돈", "괄사", "톤 개선", "선크림 바르기"), |
persistentListOf 사용해줍시닿ㅎㅎ ~~~ 컴포즈 상태로 사용되는 리스트라 불변성을 유지하면서도 변경 시 안전하게 새 객체를 만들 수 잇더용 !! 얘 쓰면 사이드 이펙트를 줄이고 리컴포지션 기준이 명확해져 상태 관리 측면에서 더 안정적인 구조를 유지할 수 있답니동 ~~
| CherrishSelectionSection( | ||
| title = "요즘 가장 신경 쓰이는\n피부 고민은 무엇인가요?", | ||
| description = "선택한 고민을 기준으로 시술 정보를 정리해줘요.", | ||
| items = listOf("피부결·각질", "색소·잡티", "홍조", "탄력·주름", "모공", "트러블"), |
There was a problem hiding this comment.
| items = listOf("피부결·각질", "색소·잡티", "홍조", "탄력·주름", "모공", "트러블"), | |
| items = persistentListOf("피부결·각질", "색소·잡티", "홍조", "탄력·주름", "모공", "트러블"), |
P1 : 여기두 ~~~~
| columns = GridCells.Fixed(2), | ||
| horizontalArrangement = Arrangement.spacedBy(12.dp), | ||
| verticalArrangement = Arrangement.spacedBy(12.dp), | ||
| userScrollEnabled = false |
There was a problem hiding this comment.
P2 : 요 코드 지금 필요할까용 ?!? 요거 어차피 컴포넌트들의 개수가 스크롤 될 수 있는 개수가 아니라서 없어두 갠찮을 거 가튼데 !!
| verticalArrangement = Arrangement.spacedBy(12.dp), | ||
| userScrollEnabled = false | ||
| ) { | ||
| itemsIndexed(items) { index, text -> |
There was a problem hiding this comment.
P1 : 요기 key(index) 이거 넣어줄까용 ??
There was a problem hiding this comment.
넵 좋아용잉
- Compose가 아이템을 안정적으로 추적할 수 있음
- 불필요한 recomposition / 상태 꼬임 방지
- index 보단 item + index 조합이 더 안전
|
|
||
| @Preview(showBackground = true) | ||
| @Composable | ||
| private fun CherrishSelectionSectionPreview_SelectionChip() { |
There was a problem hiding this comment.
p2: 별견 아니지만 네이밍 규칙 지켜주세여!!
밑에 프리뷰도 네이밍 규칙 확인 부탁드립니다!!
| ) | ||
|
|
||
| if (!description.isNullOrBlank()) { | ||
| Spacer(modifier = Modifier.height(4.dp)) |
There was a problem hiding this comment.
p2: 사이 간격 한 줄 띄어주세요!!
가독성을 위해~~ㅎㅎ
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt (2)
67-72:LazyVerticalGrid가 스크롤 가능한 부모 내에서 사용될 경우 중첩 스크롤 문제 발생 가능이 섹션이
LazyColumn이나 스크롤 가능한 컨테이너 내부에서 사용될 경우 중첩 스크롤 이슈가 발생할 수 있습니다. 고정된 아이템 수를 표시하는 용도라면userScrollEnabled = false를 추가하거나, 높이를 고정하는 것을 권장합니다.♻️ 제안하는 수정
LazyVerticalGrid( modifier = modifier.fillMaxWidth(), columns = GridCells.Fixed(2), horizontalArrangement = Arrangement.spacedBy(12.dp), - verticalArrangement = Arrangement.spacedBy(12.dp) + verticalArrangement = Arrangement.spacedBy(12.dp), + userScrollEnabled = false ) {
73-76:contentType추가로 recomposition 성능 최적화 가능
chipType에 따라 다른 컴포저블을 렌더링하므로,contentType을 지정하면 Compose가 아이템 재사용을 더 효율적으로 할 수 있습니다.♻️ 제안하는 수정
itemsIndexed( items = items, - key = { index, item -> "$item-$index" } + key = { index, item -> "$item-$index" }, + contentType = { _, _ -> chipType } ) { index, text ->
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.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/core/designsystem/component/section/CherrishSelectionSection.kt
🧬 Code graph analysis (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt (3)
app/src/main/java/com/cherrish/android/core/designsystem/component/chip/CherrishSelectionChip.kt (1)
CherrishSelectionChip(18-42)app/src/main/java/com/cherrish/android/core/designsystem/component/chip/CherrishSelectionMissionChip.kt (1)
CherrishMissionCard(27-70)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 Lint Check
- GitHub Check: PR Build Check
🔇 Additional comments (3)
app/src/main/java/com/cherrish/android/core/designsystem/component/section/CherrishSelectionSection.kt (3)
28-57: LGTM!컴포넌트 API 설계가 잘 되어 있습니다.
ImmutableList사용으로 recomposition 안정성을 확보했고, 파라미터 순서도 Compose 컨벤션(required → modifier → optional)을 따르고 있습니다.
100-126: LGTM!
isNullOrBlank()체크로 null과 빈 문자열을 모두 처리하고 있으며, 테마 적용이 일관되게 잘 되어 있습니다.
152-160: Preview 설명과 실제 기능 불일치 확인 필요Preview의 description이 "복수 선택이 가능해요"라고 되어 있지만, 현재 컴포넌트는
selectedIndex: Int?로 단일 선택만 지원합니다. 복수 선택이 필요한 경우selectedIndices: Set<Int>또는ImmutableSet<Int>로 변경이 필요합니다.의도적으로 단일 선택으로 구현한 것이라면 preview 데이터를 수정하거나, 향후 복수 선택 기능이 필요하다면 API 변경을 검토해 주세요.
nhyeonii
left a comment
nhyeonii
left a comment
There was a problem hiding this comment.
LGTM 🚀🚀🚀🚀 최고최고 ~~~ 고생했슴둥 !!!!
| ) { | ||
| itemsIndexed( | ||
| items = items, | ||
| key = { index, item -> "$item-$index" } |
There was a problem hiding this comment.
P2 : 요거 걍 index 값으로만 줘도 대지 않나용 ?!
There was a problem hiding this comment.
item이랑 섞으면 더 좋다길래 ㅎㅎ ㅎㅎㅎ ㅎㅎ
3 participants
Related issue 🛠
Work Description ✏️
Screenshot 📸
Uncompleted Tasks 😅
N/A
To Reviewers 📢
시술 플로우와 챌린지 플로우에서 사용 가능한 공통 섹션 컴포넌트를 구현했슴!
칩 타입을 선택할 수 있어요.
프리뷰 코드도 넣어놧어요.
혜민띠 챌린지 뷰 구현할 때 사용하시면 됩니다옹 공통 컴포넌트 section 패키지에 넣어놧어요 !!
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.