[Feat] PeopleBottomSheet 구현#72
Conversation
giovannijunseokim
added
🧩 Component
📝 WalkthroughWalkthrough새로운 Compose 파일 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 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
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 (1)
app/src/main/java/com/flint/core/designsystem/component/collection/PeopleBottomSheet.kt (1)
84-102: LazyColumnitems에key파라미터 추가를 권장합니다.
items(people)호출에key파라미터가 없으면, 리스트가 변경되거나 재정렬될 때 아이템 재사용이 올바르게 동작하지 않을 수 있습니다.AuthorModel의 고유 식별자를 key로 사용하면 Compose가 아이템을 효율적으로 추적할 수 있습니다.♻️ 권장 수정
LazyColumn( modifier = Modifier.fillMaxWidth(), verticalArrangement = Arrangement.spacedBy(4.dp), ) { - items(people) { author: AuthorModel -> + items(people, key = { it.userId }) { author: AuthorModel -> Author( author = author, onClick = onClickPeople, modifier = Modifier .fillMaxWidth() .padding(horizontal = 32.dp), ) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/flint/core/designsystem/component/collection/PeopleBottomSheet.ktapp/src/main/res/drawable/ic_qualified.xml
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/flint/core/designsystem/component/collection/PeopleBottomSheet.kt (3)
app/src/main/java/com/flint/core/designsystem/component/bottomsheet/FlintBasicBottomSheet.kt (1)
FlintBasicBottomSheet(32-82)app/src/main/java/com/flint/core/designsystem/component/image/ProfileImage.kt (1)
ProfileImage(11-29)app/src/main/java/com/flint/core/designsystem/theme/Theme.kt (1)
FlintTheme(8-16)
⏰ 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 (4)
app/src/main/java/com/flint/core/designsystem/component/collection/PeopleBottomSheet.kt (4)
1-42: LGTM!파일 레벨
@OptIn어노테이션과 import 구성이 적절합니다.
107-142: LGTM!
Authorcomposable이 잘 구현되어 있습니다.ProfileImage, 닉네임, 그리고 조건부 FLINER 배지 표시 로직이 적절합니다.
144-172: LGTM!Preview composable들이
FlintTheme으로 적절히 래핑되어 있고,PreviewParameter를 활용하여 다양한 상태를 미리보기할 수 있도록 잘 구성되어 있습니다.
186-206: 샘플 데이터 다양성 개선을 고려해 주세요.모든 샘플 AuthorModel이 동일한
userId = 0과UserRoleType.FLINER를 사용하고 있습니다. Preview에서 다양한 케이스를 테스트하려면 고유한 userId와 다른 userRole 타입(ADMIN, FLING 등)을 포함하는 것이 좋습니다.♻️ 권장 수정
private val sampleAuthors: List<AuthorModel> = listOf( AuthorModel( - userId = 0, + userId = 1, nickname = "사용자 이름", profileUrl = "", userRole = UserRoleType.FLINER, ), AuthorModel( - userId = 0, - nickname = "사용자 이름", + userId = 2, + nickname = "관리자", profileUrl = "", - userRole = UserRoleType.FLINER, + userRole = UserRoleType.ADMIN, ), AuthorModel( - userId = 0, + userId = 3, nickname = "사용자 이름", profileUrl = "", - userRole = UserRoleType.FLINER, + userRole = UserRoleType.FLING, ), )Likely an incorrect or invalid review comment.
nahy-512
left a comment
nahy-512
left a comment
There was a problem hiding this comment.
작업 고생하셨습니다~!
리뷰 확인 부탁드립니다
| @Composable | ||
| fun PeopleBottomSheet( | ||
| people: List<AuthorModel>, | ||
| onClickPeople: (AuthorModel) -> Unit, |
There was a problem hiding this comment.
p2
onPeopleClick로 통일해보면 어떨지!
There was a problem hiding this comment.
실제로는 People(사람들)을 클릭했을 때가 아니라 Author(한 명의 사용자)를 선택했을 때라,
onAuthorClick으로 변경했습니다! 👍
| @Composable | ||
| fun PeopleBottomSheet( | ||
| people: List<AuthorModel>, | ||
| onClickPeople: (AuthorModel) -> Unit, |
There was a problem hiding this comment.
p2
AuthorModel를 넘기는 이유가 있나요?
프로필 조회로 넘어갈 거면 userId만 넘겨받아도 되지 않을까 싶어서요!
There was a problem hiding this comment.
필요한 값만 넘기도록 수정했습니다! 👍
refactor: PeopleBottomSheet의 클릭 콜백 파라미터를 AuthorModel에서 userId로 변경
| Spacer(Modifier.width(10.dp)) | ||
|
|
||
| Image( | ||
| painter = painterResource(R.drawable.ic_qualified), |
There was a problem hiding this comment.
p2
painterResource 대신 ImageVector로 받아보는 건 어떨까요?
There was a problem hiding this comment.
| AuthorModel( | ||
| userId = 0, | ||
| nickname = "사용자 이름", | ||
| profileUrl = "", | ||
| userRole = UserRoleType.FLINER, | ||
| ), | ||
| AuthorModel( | ||
| userId = 0, | ||
| nickname = "사용자 이름", | ||
| profileUrl = "", | ||
| userRole = UserRoleType.FLINER, | ||
| ), | ||
| AuthorModel( | ||
| userId = 0, | ||
| nickname = "사용자 이름", | ||
| profileUrl = "", | ||
| userRole = UserRoleType.FLINER, | ||
| ), |
There was a problem hiding this comment.
p2
데이터 들어있는 게 다 똑같은데.. PreviewParameterProvider를 쓰고 있는 이유가 있을까요?
There was a problem hiding this comment.
- 각
AuthorModel의userRole이ADMIN,FLINER,FLING을 의도했는데 누락했네요. 이 부분 수정했습니다!
refactor: Preview용 샘플 데이터 변경 PreviewParameterProvider내부적으로 리스트 크기를 늘려 스크롤이 잘 되는지 확인할 수 있는 프리뷰를 추가했습니다.
|
|
||
| Image( | ||
| painter = painterResource(R.drawable.ic_qualified), | ||
| contentDescription = "플리너", |
There was a problem hiding this comment.
p3
contentDescription를 쓰고있는 이유가 있나요?
There was a problem hiding this comment.
접근성을 위함입니다.
contentDescription을 null로 주게되면 Screen Reader 사용자들은 플리너 인증 뱃지가 있는 사용자와 그렇지 않은 사용자를 구분할 수 없게 됩니다. 😢
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/flint/core/designsystem/component/collection/PeopleBottomSheet.kt:
- Around line 186-206: The sampleAuthors list uses the same userId (0) for every
AuthorModel which will cause key collisions in LazyColumn previews; update the
sampleAuthors entries so each AuthorModel has a unique userId (e.g., 1, 2, 3)
and ensure any LazyColumn key selector (where you reference author.userId) will
now produce unique keys, keeping AuthorModel, sampleAuthors and the userId
property consistent.
- Around line 84-97: The LazyColumn items call (items(people) { ... }) lacks a
stable key, which can cause inefficient recompositions and broken
scroll/animation behavior; update the items call in PeopleBottomSheet to pass a
key parameter (e.g., items(people, key = { it.id })) using a stable unique
property from AuthorModel (such as id) so Compose can track Author entries
correctly while still rendering Author(author = author, onClick = onAuthorClick,
modifier = ...).
🧹 Nitpick comments (2)
app/src/main/java/com/flint/core/designsystem/component/collection/PeopleBottomSheet.kt (2)
60-60: 매직 넘버543.dp를 상수로 추출하는 것을 고려하세요.하드코딩된 높이 값의 의미를 명확히 하고 재사용성을 높이기 위해 상수로 추출하면 좋습니다.
private val BottomSheetMaxHeight = 543.dp
56-103: 빈 리스트 상태 처리를 고려해 보세요.
people리스트가 비어있을 때 현재는 헤더에 "0"만 표시됩니다. 빈 상태에 대한 안내 메시지를 추가하면 사용자 경험이 향상될 수 있습니다.if (people.isEmpty()) { Text( text = "아직 저장한 사람이 없습니다", color = FlintTheme.colors.gray400, style = FlintTheme.typography.body2R14, modifier = Modifier.padding(vertical = 24.dp), ) } else { LazyColumn(/* ... */) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/flint/core/designsystem/component/collection/PeopleBottomSheet.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/flint/core/designsystem/component/collection/PeopleBottomSheet.kt (3)
app/src/main/java/com/flint/core/designsystem/component/bottomsheet/FlintBasicBottomSheet.kt (1)
FlintBasicBottomSheet(32-82)app/src/main/java/com/flint/core/designsystem/component/image/ProfileImage.kt (1)
ProfileImage(11-29)app/src/main/java/com/flint/core/designsystem/theme/Theme.kt (1)
FlintTheme(8-16)
⏰ 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/flint/core/designsystem/component/collection/PeopleBottomSheet.kt (3)
1-41: LGTM!파일 레벨
@OptIn어노테이션과 패키지 구조가 적절합니다.ExperimentalMaterial3Api는SheetState사용에 필요하며, import 목록도 깔끔하게 정리되어 있습니다.
107-142: LGTM!
Author컴포저블이 잘 구현되어 있습니다. Private 접근 제한자로 적절히 캡슐화되었고,UserRoleType.FLINER조건부 배지 표시와 접근성을 위한contentDescription도 적절합니다.
144-172: LGTM!
PreviewParameter를 활용한 프리뷰 구현이 좋습니다. 다양한 케이스를 미리 확인할 수 있어 개발 효율성이 높아집니다.
2 participants
📮 관련 이슈
📌 작업 내용
PeopleBottomSheet를 구현했습니다.📸 스크린샷
short.mp4
long.mp4
🫛 To. 리뷰어
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.