-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 피드 생성 이미지 컨테이너 UI 구현 #707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough피드 작성 화면에 이미지를 수평 스크롤로 보여주고 삭제할 수 있는 Compose UI 컴포넌트가 추가되었습니다. 관련 레이아웃 XML에 ComposeView가 배치되었으며, 이미지 삭제 아이콘 벡터와 문자열 리소스가 새로 도입되었습니다. 일부 기존 레이아웃의 마진 속성이 일관되게 정리되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateFeedImageContainer
participant AdaptationImage
participant RemoveButton
User->>CreateFeedImageContainer: 이미지 목록 전달
CreateFeedImageContainer->>AdaptationImage: 각 이미지 URL로 이미지 렌더링
User->>RemoveButton: 삭제 아이콘 클릭
RemoveButton->>CreateFeedImageContainer: onRemoveClick(index) 콜백 호출
Assessment against linked issues
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
app/src/main/res/layout/activity_create_feed.xml (1)
221-228: ComposeView 연결 로직 검증
cv_create_feed_imageComposeView를 추가하셨습니다. Activity/Fragment에서ComposeView.setContent { CreateFeedImageContainer(...) }를 호출해주셨는지 확인하고, 메모리 누수를 방지하기 위해ViewCompositionStrategy설정도 검토해 주세요.app/src/main/java/com/into/websoso/ui/createFeed/component/CreateFeedImageContainer.kt (2)
27-34: 기본값으로 하드코딩된 이미지 URL들은 제거 필요.
CreateFeedImageContainer함수에 기본값으로 하드코딩된 이미지 URL들이 있습니다. 실제 프로덕션 코드에서는 이러한 하드코딩된 값들은 제거하거나 테스트용 상수로 별도 분리하는 것이 좋습니다.다음과 같이 수정하세요:
@Composable fun CreateFeedImageContainer( - imageUrls: List<String> = listOf( - "https://github.com/user-attachments/assets/e89a02bb-549f-414d-809f-0ab1e8f72c5f", - "https://github.com/user-attachments/assets/e89a02bb-549f-414d-809f-0ab1e8f72c5f", - "https://github.com/user-attachments/assets/e89a02bb-549f-414d-809f-0ab1e8f72c5f", - ), + imageUrls: List<String>, onRemoveClick: (Int) -> Unit, ) {
79-92: 미리보기 컴포넌트의 다양한 상태 테스트 필요.현재 미리보기 컴포넌트는 단일 상태(이미지 3개)만 표시하고 있습니다. 다양한 상태(빈 목록, 1개 이미지, 많은 이미지 등)를 테스트할 수 있는 추가적인 미리보기 함수를 제공하는 것이 좋습니다.
다음과 같이 다양한 상태의 미리보기를 추가하세요:
@Preview(name = "Empty Image List") @Composable private fun CreateFeedImageContainerEmptyPreview() { WebsosoTheme { CreateFeedImageContainer( imageUrls = emptyList(), onRemoveClick = {}, ) } } @Preview(name = "Single Image") @Composable private fun CreateFeedImageContainerSinglePreview() { WebsosoTheme { CreateFeedImageContainer( imageUrls = listOf( "https://github.com/user-attachments/assets/e89a02bb-549f-414d-809f-0ab1e8f72c5f", ), onRemoveClick = {}, ) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/src/main/java/com/into/websoso/ui/createFeed/component/CreateFeedImageContainer.kt(1 hunks)app/src/main/res/layout/activity_create_feed.xml(4 hunks)app/src/main/res/layout/item_feed.xml(3 hunks)app/src/main/res/layout/item_feed_detail_header.xml(1 hunks)app/src/main/res/layout/item_my_activity.xml(2 hunks)core/resource/src/main/res/drawable/ic_feed_remove_image.xml(1 hunks)core/resource/src/main/res/values/strings.xml(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/main/java/com/into/websoso/ui/createFeed/component/CreateFeedImageContainer.kt (1)
app/src/main/java/com/into/websoso/core/common/ui/component/AdaptationImage.kt (1)
AdaptationImage(11-32)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (12)
core/resource/src/main/res/values/strings.xml (2)
112-112: 문자열 리소스 업데이트 적절성 확인
feed_private_label의 텍스트를 “비공개 기록이에요.”에서 “나만 보는 기록이에요.”로 수정하신 것은 사용자 맥락 상 이해하기 쉽고 자연스럽습니다. 하지만 기존에 해당 리소스를 참조하는 모든 화면에서 의도한 문구가 잘 반영되는지 UI 검증을 추천드립니다.
403-403: 새로운 문자열 리소스 네이밍 및 위치 확인
create_feed_private리소스를 추가하여 “나만 보는 기록”을 분리하신 것은 재사용성과 다국어 지원 관점에서 올바른 접근입니다. 네이밍 규칙(create_feed_*)이 다른 리소스와 일관되는지, 오타나 중복이 없는지 한 번 더 점검해 주세요.core/resource/src/main/res/drawable/ic_feed_remove_image.xml (1)
1-21: 뷰포트와 dp 크기 불일치 검토 요청
android:width="19dp"vsandroid:height="18dp"로 설정되어 있습니다. 일반적으로 원형 아이콘은 정사각 뷰포트를 사용해 왜곡 없이 렌더링하는 것이 좋습니다. 디자인 사양상 의도된 값인지 확인 부탁드립니다.app/src/main/res/layout/item_feed_detail_header.xml (1)
189-190: 마진 통일성 및 UI 확인
cl_feed_like의layout_marginStart를 20dp→12dp로,layout_marginVertical="10dp"로 통합하여 다른 피드 레이아웃과 일관성을 맞추셨습니다. 변경된 마진이 실제 화면에서 의도한 대로 적용되는지 UI 테스트를 권장드립니다.app/src/main/res/layout/item_my_activity.xml (2)
227-227: 마진 통합 적용 점검
cl_my_activity_like에android:layout_marginVertical="10dp"를 추가해 top/bottom 마진을 통일하셨습니다. 피드 레이아웃과 마찬가지로 사용자 경험에 지장이 없는지 간략한 UI 검증을 요청드립니다.
306-306: 레이아웃 마진 일관성 확인
비공개 상태 뷰의LinearLayout에도 동일하게layout_marginVertical="10dp"를 적용하셨습니다. 다른 컴포넌트와의 간격이 예상대로 유지되는지 확인해주세요.app/src/main/res/layout/activity_create_feed.xml (2)
75-75: 하드코딩 문자열 리소스화
tv_create_feed_lock_label에서 하드코딩된 “비공개 기록”을@string/create_feed_private로 변경하여 다국어 대응성과 유지보수성을 높이신 점 좋습니다.
240-240: 제약 조건 업데이트 적절
tv_create_feed_connect_novel의app:layout_constraintTop_toBottomOf를cv_create_feed_image로 변경하여 레이아웃 순서가 자연스럽습니다. ComposeView 추가 후 겹침이나 간격 이슈가 없는지 UI 테스트를 권장드립니다.app/src/main/res/layout/item_feed.xml (2)
209-210: 마진 선언 방식 개선 및 일관성 있는 UI 제공.레이아웃 마진을 단순화하여
layout_marginTop과layout_marginBottom대신 단일layout_marginVertical속성을 사용한 것은 좋은 개선입니다. 또한 시작 마진을 20dp에서 12dp로 줄임으로써 디자인 일관성을 높였습니다.
289-289: 레이아웃 마진 선언 방식 통일 적용.이 LinearLayout에도 동일하게
layout_marginVertical="10dp"를 적용하여 코드 일관성을 유지한 것이 좋습니다. 이런 일관된 마진 처리는 유지보수성을 높이고 레이아웃 코드를 더 읽기 쉽게 만듭니다.app/src/main/java/com/into/websoso/ui/createFeed/component/CreateFeedImageContainer.kt (2)
26-50: 피드 이미지 컨테이너 구현이 잘 되었습니다.
CreateFeedImageContainer컴포넌트가 수평 스크롤 가능한 이미지 목록을 효과적으로 표시합니다.LazyRow의 사용과 적절한 간격 및 패딩 설정이 UI 디자인 가이드라인을 잘 따르고 있습니다.
72-75:Details
❌ Incorrect review comment
이미지 로딩 실패 처리 부재.
AdaptationImage컴포넌트를 사용하여 이미지를 로드하고 있지만, 이미지 로딩 실패 시의 처리가 없습니다. 네트워크 이미지 로딩은 항상 실패 가능성이 있으므로 오류 처리 및 대체 이미지를 제공하는 방안을 고려해야 합니다.아래 스크립트를 통해
AdaptationImage컴포넌트가 오류 처리를 지원하는지 확인해보세요:
🏁 Script executed:
#!/bin/bash # Check if AdaptationImage has error handling echo "Checking AdaptationImage implementation for error handling..." rg -A 15 "fun AdaptationImage" --type kotlinLength of output: 1950
🏁 Script executed:
#!/bin/bash echo "Checking NetworkImage implementation for error handling..." rg -A 30 "fun NetworkImage" --type kotlinLength of output: 4177
AdaptationImage는 기본적으로 오류 처리 및 대체 이미지 로딩을 지원합니다
NetworkImage 구현에서 AsyncImagePainter의 상태를 감지해 실패 시 기본 placeholder(
img_my_library_empty_cat)를 사용하고, 로딩 중에는 스피너를 표시하므로 별도의 에러 처리 로직 추가는 불필요합니다.
- 확인 위치
- AdaptationImage:
app/src/main/java/com/into/websoso/core/common/ui/component/AdaptationImage.kt- NetworkImage:
core/designsystem/src/main/java/com/into/websoso/core/designsystem/component/NetworkImage.kt
onState콜백에서isError관리- 오류 시
placeholder페인터로 대체 이미지 표시필요할 경우 AdaptationImage에 placeholder 파라미터를 추가해 커스텀 플레이스홀더를 지정하는 확장만 고려해 주세요.
Likely an incorrect or invalid review comment.
| android:checked="@={!viewModel.isPublic}" | ||
| android:thumb="@drawable/bg_create_feed_toggle_thumb" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
양방향 바인딩 표현식 오류
android:checked="@={!viewModel.isPublic}"은 DataBinding의 양방향 바인딩(@={})이 직접 변수 참조에만 지원됩니다. !viewModel.isPublic 같은 표현식에는 적용되지 않아 상태 변화가 뷰모델에 반영되지 않을 수 있습니다.
해결책:
- android:checked="@={!viewModel.isPublic}"
+ android:checked="@{!viewModel.isPublic}"그리고 상태 변경 시 뷰모델 업데이트는 setOnCheckedChangeListener나 커스텀 BindingAdapter로 처리하세요.
🤖 Prompt for AI Agents
In app/src/main/res/layout/activity_create_feed.xml at lines 89-90, the two-way
data binding expression uses a negation operator inside the binding which is not
supported. To fix this, remove the negation from the binding expression and bind
directly to the variable without operators. Then handle the inversion logic and
update the ViewModel state by implementing a setOnCheckedChangeListener in the
code or create a custom BindingAdapter to manage the toggle state and
synchronize changes properly.
| painter = painterResource(ic_feed_remove_image), | ||
| contentDescription = null, | ||
| modifier = Modifier | ||
| .size(38.dp) | ||
| .padding(10.dp) | ||
| .align(Alignment.TopEnd) | ||
| .clickableWithoutRipple { onRemoveClick() }, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
삭제 버튼의 접근성 개선 필요.
삭제 버튼의 contentDescription이 null로 설정되어 있어 접근성 도구(예: 화면 리더)를 사용하는 사용자가 이 버튼의 기능을 인식하기 어렵습니다. 삭제 버튼임을 나타내는 적절한 설명을 추가해야 합니다.
Image(
painter = painterResource(ic_feed_remove_image),
- contentDescription = null,
+ contentDescription = "이미지 삭제 버튼",
modifier = Modifier
.size(38.dp)
.padding(10.dp)
.align(Alignment.TopEnd)
.clickableWithoutRipple { onRemoveClick() },
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| painter = painterResource(ic_feed_remove_image), | |
| contentDescription = null, | |
| modifier = Modifier | |
| .size(38.dp) | |
| .padding(10.dp) | |
| .align(Alignment.TopEnd) | |
| .clickableWithoutRipple { onRemoveClick() }, | |
| ) | |
| Image( | |
| painter = painterResource(ic_feed_remove_image), | |
| contentDescription = "이미지 삭제 버튼", | |
| modifier = Modifier | |
| .size(38.dp) | |
| .padding(10.dp) | |
| .align(Alignment.TopEnd) | |
| .clickableWithoutRipple { onRemoveClick() }, | |
| ) |
🤖 Prompt for AI Agents
In
app/src/main/java/com/into/websoso/ui/createFeed/component/CreateFeedImageContainer.kt
around lines 64 to 71, the delete button's contentDescription is set to null,
which hinders accessibility tools from conveying its purpose. Replace the null
contentDescription with a meaningful string that clearly describes the button's
function, such as "Delete image" or an equivalent localized description, to
improve accessibility for screen reader users.
| @Composable | ||
| private fun CreateFeedImageBox( | ||
| imageUrl: String, | ||
| onRemoveClick: () -> Unit, | ||
| ) { | ||
| Box( | ||
| modifier = Modifier | ||
| .size(100.dp) | ||
| .aspectRatio(1f) | ||
| .clip(RoundedCornerShape(8.dp)), | ||
| ) { | ||
| Image( | ||
| painter = painterResource(ic_feed_remove_image), | ||
| contentDescription = null, | ||
| modifier = Modifier | ||
| .size(38.dp) | ||
| .padding(10.dp) | ||
| .align(Alignment.TopEnd) | ||
| .clickableWithoutRipple { onRemoveClick() }, | ||
| ) | ||
| AdaptationImage( | ||
| imageUrl = imageUrl, | ||
| contentScale = ContentScale.Crop, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이미지 박스 컴포넌트의 레이어 순서 이슈가 있습니다.
CreateFeedImageBox 컴포넌트에서 삭제 버튼 이미지가 실제 이미지보다 먼저 선언되어 있어 뒤에 배치될 수 있는 문제가 있습니다. Box 내에서의 렌더링 순서는 선언 순서에 따라 결정되므로, AdaptationImage를 먼저 선언하고 그 위에 삭제 버튼을 배치하는 것이 더 안전합니다.
다음과 같이 수정하세요:
@Composable
private fun CreateFeedImageBox(
imageUrl: String,
onRemoveClick: () -> Unit,
) {
Box(
modifier = Modifier
.size(100.dp)
.aspectRatio(1f)
.clip(RoundedCornerShape(8.dp)),
) {
+ AdaptationImage(
+ imageUrl = imageUrl,
+ contentScale = ContentScale.Crop,
+ )
Image(
painter = painterResource(ic_feed_remove_image),
contentDescription = null,
modifier = Modifier
.size(38.dp)
.padding(10.dp)
.align(Alignment.TopEnd)
.clickableWithoutRipple { onRemoveClick() },
)
- AdaptationImage(
- imageUrl = imageUrl,
- contentScale = ContentScale.Crop,
- )
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Composable | |
| private fun CreateFeedImageBox( | |
| imageUrl: String, | |
| onRemoveClick: () -> Unit, | |
| ) { | |
| Box( | |
| modifier = Modifier | |
| .size(100.dp) | |
| .aspectRatio(1f) | |
| .clip(RoundedCornerShape(8.dp)), | |
| ) { | |
| Image( | |
| painter = painterResource(ic_feed_remove_image), | |
| contentDescription = null, | |
| modifier = Modifier | |
| .size(38.dp) | |
| .padding(10.dp) | |
| .align(Alignment.TopEnd) | |
| .clickableWithoutRipple { onRemoveClick() }, | |
| ) | |
| AdaptationImage( | |
| imageUrl = imageUrl, | |
| contentScale = ContentScale.Crop, | |
| ) | |
| } | |
| } | |
| @Composable | |
| private fun CreateFeedImageBox( | |
| imageUrl: String, | |
| onRemoveClick: () -> Unit, | |
| ) { | |
| Box( | |
| modifier = Modifier | |
| .size(100.dp) | |
| .aspectRatio(1f) | |
| .clip(RoundedCornerShape(8.dp)), | |
| ) { | |
| AdaptationImage( | |
| imageUrl = imageUrl, | |
| contentScale = ContentScale.Crop, | |
| ) | |
| Image( | |
| painter = painterResource(ic_feed_remove_image), | |
| contentDescription = null, | |
| modifier = Modifier | |
| .size(38.dp) | |
| .padding(10.dp) | |
| .align(Alignment.TopEnd) | |
| .clickableWithoutRipple { onRemoveClick() }, | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/into/websoso/ui/createFeed/component/CreateFeedImageContainer.kt
between lines 52 and 77, the delete button image is declared before the main
image inside the Box, causing it to be rendered underneath the main image. To
fix this, reorder the composables by declaring AdaptationImage first and then
the delete button Image, ensuring the delete button is rendered on top.
| .align(Alignment.TopEnd) | ||
| .clickableWithoutRipple { onRemoveClick() }, | ||
| ) | ||
| AdaptationImage( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c: 이거 지금 core모듈에 있는 NetworkImage로 바꿔주삼요 !
m6z1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굿굿입니다
| @Composable | ||
| fun CreateFeedImageContainer( | ||
| imageUrls: List<String>, | ||
| onRemoveClick: (Int) -> Unit, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c: 여기도 어떤 값인지 작성 부탁드려욥
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
📷𝘚𝘤𝘳𝘦𝘦𝘯𝘴𝘩𝘰𝘵
또 카카오 로그인이 안돼서 일단 날리겠삼
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴
포맷은 API 작업때 보고 변경해둘게요
Summary by CodeRabbit
신규 기능
버그 수정
스타일
문서화