[Feat] collection create modal and bottomsheet#65
Conversation
📝 Walkthrough워크스루컬렉션 생성 기능의 UI를 개선하기 위해 두 개의 새로운 Jetpack Compose 컴포넌트를 추가합니다. 갤러리 선택 및 커버 삭제 옵션을 제공하는 바텀 시트와 삭제 확인 모달을 구현하여 사용자 상호작용 흐름을 완성합니다. 변경사항
예상 코드 리뷰 노력🎯 2 (Simple) | ⏱️ ~12분 추천 리뷰어
시
🚥 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
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateModal.kt (1)
12-25: 이름과 기능 불일치 및 재사용 불가능한 구조
네이밍 불일치:
CollectionCreateModal이라는 이름이지만, UI 텍스트는 삭제("작품을 삭제할까요?", "삭제")에 관한 내용입니다. 네이밍을CollectionDeleteModal또는 실제 용도에 맞게 수정하는 것이 좋습니다.재사용성 부재:
onConfirm,onDismiss콜백이 빈 람다로 하드코딩되어 있어 외부에서 사용할 수 없습니다. 현재private이므로 프리뷰 외에는 사용이 불가능합니다.문자열 리소스: 한국어 문자열들이 하드코딩되어 있습니다.
strings.xml리소스를 사용하면 다국어 지원과 유지보수에 유리합니다.♻️ 재사용 가능한 구조로 개선 제안
@Composable -private fun CollectionCreateModal() { +fun CollectionDeleteModal( + onConfirm: () -> Unit, + onDismiss: () -> Unit, +) { TwoButtonModal( modifier = Modifier.background(Color.White), message = "작성한 내용이 모두 삭제돼요.", cancelText = "취소", confirmText = "삭제", - onConfirm = {}, - onDismiss = {}, + onConfirm = onConfirm, + onDismiss = onDismiss, title = "작품을 삭제할까요?", icon = R.drawable.ic_gradient_trash, isDestructive = true ) }app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateBottomSheet.kt (2)
11-33: 외부에서 사용 불가능한 구조현재
CollectionCreateBottomSheet는public함수이지만, 모든 콜백(clickAction,onDismiss)이 빈 람다로 하드코딩되어 있어 실제 기능 연동이 불가능합니다.♻️ 콜백을 파라미터로 노출하는 구조로 개선 제안
@OptIn(ExperimentalMaterial3Api::class) @Composable -fun CollectionCreateBottomSheet() { +fun CollectionCreateBottomSheet( + onSelectFromGallery: () -> Unit, + onDeleteCoverPhoto: () -> Unit, + onDismiss: () -> Unit, +) { val menuBottomSheetDataList = listOf( MenuBottomSheetData( label = "갤러리에서 선택", - clickAction = {} + clickAction = onSelectFromGallery ), MenuBottomSheetData( label = "커버 사진 삭제", color = FlintTheme.colors.error500, - clickAction = {} + clickAction = onDeleteCoverPhoto ) ) val sheetState = rememberModalBottomSheetState() MenuBottomSheet( menuBottomSheetDataList = menuBottomSheetDataList, - onDismiss = {}, + onDismiss = onDismiss, sheetState = sheetState ) }프리뷰도 함께 수정:
@OptIn(ExperimentalMaterial3Api::class) @Preview @Composable private fun CollectionCreateBottomSheetPreview() { FlintTheme { - CollectionCreateBottomSheet() + CollectionCreateBottomSheet( + onSelectFromGallery = {}, + onDeleteCoverPhoto = {}, + onDismiss = {} + ) } }
35-42: 프리뷰에showBackground = true추가 권장
CollectionCreateModal의 프리뷰와 일관성을 위해showBackground = true를 추가하는 것이 좋습니다.♻️ 제안
@OptIn(ExperimentalMaterial3Api::class) -@Preview +@Preview(showBackground = true) @Composable private fun CollectionCreateBottomSheetPreview() {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateBottomSheet.ktapp/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateModal.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). (2)
- GitHub Check: PR Build Check
- GitHub Check: PR Lint Check
nahy-512
left a comment
There was a problem hiding this comment.
고생했어용 다른 사람 컴포 따라쓰기도.. 쉽지않죠??ㅎㅎ
리뷰 내용 수정하고 다시 리뷰 요청주세요~~
|
|
||
| @OptIn(ExperimentalMaterial3Api::class) | ||
| @Composable | ||
| fun CollectionCreateBottomSheet() { |
There was a problem hiding this comment.
p1
onDismiss랑 onGalleryClick, onCoverdeleteClick 등의 이벤트 미리 뚫어서 아래 넘겨주시면 좋을 것 같습니다!
There was a problem hiding this comment.
으아! 무작정 따라썼네요 ~~ 변경하겠습니다!
| import com.flint.core.designsystem.theme.FlintTheme | ||
|
|
||
| @Composable | ||
| private fun CollectionCreateModal() { |
There was a problem hiding this comment.
p1
여기도 클릭 이벤트 미리 뚫어주면 좋을듯!
onConfirm이랑 onDismiss
There was a problem hiding this comment.
p1
사용처보다도 용도를 하나 더 적어주면 좋을 것 같아요! 삭제를 위한 모달이니까 차라리 FilmDeleteModal 식으로 명칭해주는 것도 좋을듯??
| @Composable | ||
| private fun CollectionCreateModal() { | ||
| TwoButtonModal( | ||
| modifier = Modifier.background(Color.White), |
There was a problem hiding this comment.
p1
background가 들어가는 이유가 있나요?
There was a problem hiding this comment.
삭제했습니다!! 프리뷰 따라하지 않기 ~~
| confirmText = "삭제", | ||
| onConfirm = {}, | ||
| onDismiss = {}, | ||
| title = "작품을 삭제할까요?", |
There was a problem hiding this comment.
p2
title이 message보다 먼저 들어가는 게 조금 더 보기 편할 것 같아요..!
| onDismiss = {}, | ||
| title = "작품을 삭제할까요?", | ||
| icon = R.drawable.ic_gradient_trash, | ||
| isDestructive = true |
There was a problem hiding this comment.
아래 옵션이 기본으로 false를 선택되어있는데, 그럴경우 파란색 버튼이 생성됩니다! 빨간색 버튼으로 변경하려면 true로 설정해줘야된다고 적혀있더라구요 !!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionAddFilmBottomSheet.kt (1)
13-16: 파라미터 명명 규칙 불일치:onCoverdeleteClick→onCoverDeleteClick
onCoverdeleteClick은 camelCase 규칙에 맞지 않습니다.onCoverDeleteClick으로 수정하는 것이 좋습니다.♻️ 제안된 수정
@Composable fun CollectionAddFilmBottomSheet( onGalleryClick: () -> Unit, - onCoverdeleteClick: () -> Unit, + onCoverDeleteClick: () -> Unit, onDismiss: () -> Unit ) { val menuBottomSheetDataList = listOf( MenuBottomSheetData( label = "갤러리에서 선택", clickAction = onGalleryClick ), MenuBottomSheetData( label = "커버 사진 삭제", color = FlintTheme.colors.error500, - clickAction = onCoverdeleteClick + clickAction = onCoverDeleteClick ) )
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionAddFilmBottomSheet.ktapp/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateModal.ktgradle/libs.versions.toml
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionAddFilmBottomSheet.kt (2)
app/src/main/java/com/flint/core/designsystem/component/bottomsheet/MenuBottomSheet.kt (1)
MenuBottomSheet(30-77)app/src/main/java/com/flint/core/designsystem/theme/Theme.kt (1)
FlintTheme(8-16)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateModal.kt (2)
app/src/main/java/com/flint/core/designsystem/component/modal/TwoButtonModal.kt (1)
TwoButtonModal(25-90)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/presentation/collectioncreate/component/CollectionCreateModal.kt (1)
9-24: LGTM!
TwoButtonModal을 적절하게 활용하여 삭제 확인 모달을 구현했습니다.isDestructive = true설정으로 삭제 버튼이 올바르게 강조됩니다.app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionAddFilmBottomSheet.kt (1)
18-36: 액션 클릭 후 바텀시트 자동 닫힘 동작 확인 필요현재 구현에서 메뉴 항목 클릭 시
clickAction만 호출되고 바텀시트가 자동으로 닫히지 않습니다. 호출하는 쪽에서onGalleryClick이나onCoverDeleteClick콜백 내에서 바텀시트를 닫는 로직을 포함해야 합니다. 의도된 동작이라면 무시해주세요.gradle/libs.versions.toml (1)
3-3: Material3 ModalBottomSheet는 API 27과 완벽하게 호환됩니다.ModalBottomSheet는 Jetpack Compose Material3의 일부로, API 레벨 21(Android 5.0) 이상에서 지원됩니다. minSdk 27 환경에서는 호환성 문제가 없습니다.
또한 코드베이스에서 @RequiresApi 어노테이션이나 명시적인 API 레벨 체크가 발견되지 않아, 이 변경으로 인한 다른 호환성 문제도 파악되지 않습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateFilmDeleteModal.kt (2)
10-13:modifier파라미터 추가를 권장합니다.Compose 컴포넌트의 일반적인 관례에 따라
modifier파라미터를 노출하면 호출 측에서 레이아웃 조정이 가능해져 재사용성이 향상됩니다.♻️ 제안하는 수정
@Composable fun CollectionCreateFilmDeleteModal( onConfirm: () -> Unit, - onDismiss: () -> Unit + onDismiss: () -> Unit, + modifier: Modifier = Modifier ) { TwoButtonModal( title = "작품을 삭제할까요?", message = "작성한 내용이 모두 삭제돼요.", cancelText = "취소", confirmText = "삭제", onConfirm = onConfirm, onDismiss = onDismiss, + modifier = modifier, icon = R.drawable.ic_gradient_trash, isDestructive = true ) }
14-23: 하드코딩된 문자열을 string resource로 분리하는 것을 고려해 주세요.현재 UI 텍스트("작품을 삭제할까요?", "작성한 내용이 모두 삭제돼요." 등)가 코드에 직접 작성되어 있습니다. 향후 다국어 지원(i18n)이나 텍스트 일괄 관리를 위해
strings.xml로 분리하면 유지보수에 유리합니다.참고:
R.drawable.ic_gradient_trash아이콘 리소스는 정상적으로 존재합니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionAddFilmBottomSheet.ktapp/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateFilmDeleteModal.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionAddFilmBottomSheet.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateFilmDeleteModal.kt (2)
app/src/main/java/com/flint/core/designsystem/component/modal/TwoButtonModal.kt (1)
TwoButtonModal(25-90)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 Build Check
- GitHub Check: PR Lint Check
🔇 Additional comments (1)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateFilmDeleteModal.kt (1)
26-35: LGTM!Preview 함수가
FlintTheme으로 올바르게 감싸져 있고,showBackground = true설정으로 미리보기에서 모달을 확인하기 좋습니다.
📮 관련 이슈
📌 작업 내용
📸 스크린샷
😅 미구현
🫛 To. 리뷰어
Summary by CodeRabbit
새 기능
✏️ Tip: You can customize this high-level summary in your review settings.