-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 피드 상세 이미지 컨테이너 UI 구현 #685
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
|
Caution Review failedThe pull request is closed. """ Walkthrough피드 상세 화면에서 이미지 관련 UI가 전면적으로 도입되었습니다. 이미지 개수에 따라 적절한 이미지 컨테이너(1~3개, 다수) 컴포저블이 동적으로 렌더링되며, 이미지를 클릭하면 콜백이 트리거됩니다. 관련 인터페이스 및 레이아웃, 뷰홀더, Enum 등이 추가 및 수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FeedDetailContentViewHolder
participant AdaptationFeedImageContainer
participant ImageContainer (Single/Double/Triple/Multiple)
participant FeedDetailClickListener
participant FeedDetailActivity
User->>FeedDetailContentViewHolder: 이미지 클릭
FeedDetailContentViewHolder->>AdaptationFeedImageContainer: 이미지 클릭 이벤트 위임
AdaptationFeedImageContainer->>ImageContainer: 클릭 이벤트 위임 (with index)
ImageContainer->>FeedDetailClickListener: onFeedImageClick(imageUrls, position)
FeedDetailClickListener->>FeedDetailActivity: onFeedImageClick 호출
FeedDetailActivity->>FeedDetailActivity: navigateToExpandedImage (미구현)
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 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: 1
🧹 Nitpick comments (6)
app/src/main/java/com/into/websoso/ui/feedDetail/component/DoubleImageContainer.kt (1)
17-42: 인덱싱 방식 개선 필요현재 구현에서는
imageUrls.indexOf(imageUrl)를 사용하여 클릭된 이미지의 인덱스를 찾고 있습니다. 이 방식은 중복된 URL이 있거나 목록이 큰 경우 비효율적일 수 있습니다. 인덱스를 직접 사용하는 방식으로 개선하면 더 효율적일 것입니다.- imageUrls.take(2).forEach { imageUrl -> - AdaptationImage( - imageUrl = imageUrl, - contentScale = ContentScale.Crop, - modifier = Modifier - .weight(1f) - .aspectRatio(1f) - .clip(RoundedCornerShape(8.dp)) - .clickableWithoutRipple { - onImageClick(imageUrls.indexOf(imageUrl)) - }, - ) - } + imageUrls.take(2).forEachIndexed { index, imageUrl -> + AdaptationImage( + imageUrl = imageUrl, + contentScale = ContentScale.Crop, + modifier = Modifier + .weight(1f) + .aspectRatio(1f) + .clip(RoundedCornerShape(8.dp)) + .clickableWithoutRipple { + onImageClick(index) + }, + ) + }그외에는 UI 구현이 깔끔하고 Compose 모범 사례를 잘 따르고 있습니다. 두 이미지를 동일한 너비로 표시하고 적절한 간격과 모서리 처리를 통해 일관된 디자인을 제공합니다.
app/src/main/java/com/into/websoso/ui/feedDetail/component/MultipleImageContainer.kt (1)
18-43: 반응형 UI 및 성능 최적화 개선 필요현재 구현에서는 이미지 크기가 고정값(100.dp)으로 설정되어 있어 다양한 화면 크기에서 최적화되지 않을 수 있습니다. 또한 이미지 수가 많을 경우 성능 문제가 발생할 수 있습니다.
다음과 같은 개선 사항을 고려해보세요:
- 반응형 이미지 크기 사용:
- .size(100.dp) + .size(dimensionResource(R.dimen.feed_image_thumbnail_size))
- 표시할 최대 이미지 수 제한:
-items(imageUrls.size) { position -> +items(min(imageUrls.size, 10)) { position ->
- 마지막 이미지에 더 많은 이미지가 있음을 표시하는 오버레이 추가:
if (position == 9 && imageUrls.size > 10) { Box( modifier = Modifier.matchParentSize(), contentAlignment = Alignment.Center ) { Text( text = "+${imageUrls.size - 10}", color = Color.White, fontWeight = FontWeight.Bold ) } }이미지를 가로 스크롤로 표시하는 LazyRow 사용은 적절하며, 이미지 간 간격과 패딩 설정도 UI 디자인에 일관성을 제공합니다.
app/src/main/java/com/into/websoso/ui/feedDetail/model/ImageContainerType.kt (1)
1-15: Enum 정의 명확화 필요Enum 클래스 구현은 깔끔하나,
MULTIPLE값의 정의와 사용 방식에 불일치가 있습니다.MULTIPLE은 size가 4로 정의되어 있지만,from메서드에서는 일치하는 값이 없을 경우 기본값으로 사용됩니다. 이는 4개 이상의 이미지를 모두MULTIPLE로 처리하겠다는 의도로 보이나, 코드만으로는 이해하기 어려울 수 있습니다.개선 방안:
- 문서화를 통한 명확화:
enum class ImageContainerType( private val size: Int, ) { SINGLE(1), DOUBLE(2), TRIPLE(3), - MULTIPLE(4), + MULTIPLE(4), // 4개 이상의 이미지를 위한 컨테이너 타입 ; companion object { - fun from(size: Int): ImageContainerType = ImageContainerType.entries.find { it.size == size } ?: MULTIPLE + /** + * 이미지 개수에 따라 적절한 컨테이너 타입을 반환합니다. + * 1~3개의 이미지는 각각 SINGLE, DOUBLE, TRIPLE 타입을 반환하고, + * 4개 이상의 이미지는 MULTIPLE 타입을 반환합니다. + */ + fun from(size: Int): ImageContainerType = when { + size <= 0 -> throw IllegalArgumentException("이미지 개수는 1개 이상이어야 합니다.") + size <= 3 -> ImageContainerType.entries.find { it.size == size }!! + else -> MULTIPLE + } } }이 enum 클래스는 이미지 개수에 따라 적절한 컨테이너 타입을 결정하는 역할을 잘 수행하고 있습니다.
app/src/main/java/com/into/websoso/ui/feedDetail/component/TripleImageContainer.kt (2)
28-40: 이미지 인덱스 검색 방식 개선 제안
imageUrls.indexOf(imageUrl)는 리스트에서 중복된 이미지가 있을 경우 항상 첫 번째 발견된 이미지의 인덱스를 반환합니다. 이는 의도하지 않은 동작을 유발할 수 있습니다.다음과 같이 수정하여 인덱스를 직접 사용하는 것이 더 효율적입니다:
- imageUrls.take(3).forEach { imageUrl -> + imageUrls.take(3).forEachIndexed { index, imageUrl -> AdaptationImage( imageUrl = imageUrl, contentScale = ContentScale.Crop, modifier = Modifier .weight(1f) .aspectRatio(1f) .clip(RoundedCornerShape(8.dp)) .clickableWithoutRipple { - onImageClick(imageUrls.indexOf(imageUrl)) + onImageClick(index) }, ) }
28-40: 코드 중복 검토 필요
DoubleImageContainer와TripleImageContainer의 구현이 매우 유사합니다. 이미지 수만 다를 뿐 대부분의 코드가 중복됩니다.다음과 같이 공통 컴포넌트를 추출하여 재사용할 수 있습니다:
@Composable private fun RowImagesContainer( imageUrls: List<String>, maxImages: Int, onImageClick: (Int) -> Unit, ) { Row( modifier = Modifier .fillMaxWidth() .padding(horizontal = 20.dp), horizontalArrangement = Arrangement.spacedBy(8.dp), ) { imageUrls.take(maxImages).forEachIndexed { index, imageUrl -> AdaptationImage( imageUrl = imageUrl, contentScale = ContentScale.Crop, modifier = Modifier .weight(1f) .aspectRatio(1f) .clip(RoundedCornerShape(8.dp)) .clickableWithoutRipple { onImageClick(index) }, ) } } } @Composable fun DoubleImageContainer( imageUrls: List<String>, onImageClick: (Int) -> Unit, ) { RowImagesContainer(imageUrls, 2, onImageClick) } @Composable fun TripleImageContainer( imageUrls: List<String>, onImageClick: (Int) -> Unit, ) { RowImagesContainer(imageUrls, 3, onImageClick) }app/src/main/java/com/into/websoso/ui/feedDetail/component/AdaptationFeedImageContainer.kt (1)
16-16: 첫 번째 이미지 처리 방식 개선단일 이미지 케이스에서
imageUrls.first()를 사용하고 있습니다. 람다 표현식 내에서 인덱스를 하드코딩하는 것보다 더 명확한 방식이 있을 수 있습니다.다음과 같이 수정을 고려해 보세요:
- SINGLE -> SingleImageContainer(imageUrls.first()) { onImageClick(0) } + SINGLE -> { + val imageIndex = 0 + SingleImageContainer(imageUrls[imageIndex]) { onImageClick(imageIndex) } + }또는 간결하게:
- SINGLE -> SingleImageContainer(imageUrls.first()) { onImageClick(0) } + SINGLE -> SingleImageContainer(imageUrls[0]) { onImageClick(0) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/src/main/java/com/into/websoso/ui/feedDetail/FeedDetailActivity.kt(2 hunks)app/src/main/java/com/into/websoso/ui/feedDetail/FeedDetailClickListener.kt(1 hunks)app/src/main/java/com/into/websoso/ui/feedDetail/adapter/FeedDetailContentViewHolder.kt(2 hunks)app/src/main/java/com/into/websoso/ui/feedDetail/component/AdaptationFeedImageContainer.kt(1 hunks)app/src/main/java/com/into/websoso/ui/feedDetail/component/DoubleImageContainer.kt(1 hunks)app/src/main/java/com/into/websoso/ui/feedDetail/component/MultipleImageContainer.kt(1 hunks)app/src/main/java/com/into/websoso/ui/feedDetail/component/SingleImageContainer.kt(1 hunks)app/src/main/java/com/into/websoso/ui/feedDetail/component/TripleImageContainer.kt(1 hunks)app/src/main/java/com/into/websoso/ui/feedDetail/model/ImageContainerType.kt(1 hunks)app/src/main/res/layout/item_feed_detail_header.xml(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
app/src/main/java/com/into/websoso/ui/feedDetail/component/DoubleImageContainer.kt (1)
app/src/main/java/com/into/websoso/core/common/ui/component/AdaptationImage.kt (1)
AdaptationImage(11-32)
app/src/main/java/com/into/websoso/ui/feedDetail/component/SingleImageContainer.kt (1)
app/src/main/java/com/into/websoso/core/common/ui/component/AdaptationImage.kt (1)
AdaptationImage(11-32)
app/src/main/java/com/into/websoso/ui/feedDetail/adapter/FeedDetailContentViewHolder.kt (1)
app/src/main/java/com/into/websoso/ui/feedDetail/component/AdaptationFeedImageContainer.kt (1)
AdaptationFeedImageContainer(10-21)
app/src/main/java/com/into/websoso/ui/feedDetail/component/AdaptationFeedImageContainer.kt (4)
app/src/main/java/com/into/websoso/ui/feedDetail/component/SingleImageContainer.kt (1)
SingleImageContainer(15-32)app/src/main/java/com/into/websoso/ui/feedDetail/component/DoubleImageContainer.kt (1)
DoubleImageContainer(17-42)app/src/main/java/com/into/websoso/ui/feedDetail/component/TripleImageContainer.kt (1)
TripleImageContainer(17-42)app/src/main/java/com/into/websoso/ui/feedDetail/component/MultipleImageContainer.kt (1)
MultipleImageContainer(18-43)
app/src/main/java/com/into/websoso/ui/feedDetail/component/MultipleImageContainer.kt (1)
app/src/main/java/com/into/websoso/core/common/ui/component/AdaptationImage.kt (1)
AdaptationImage(11-32)
app/src/main/java/com/into/websoso/ui/feedDetail/component/TripleImageContainer.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 (10)
app/src/main/java/com/into/websoso/ui/feedDetail/FeedDetailClickListener.kt (1)
19-23: 이미지 클릭 이벤트 처리를 위한 인터페이스 메서드가 잘 추가되었습니다.이미지 목록과 클릭 위치를 매개변수로 받는 방식으로 설계되어 확장 이미지 뷰로의 네비게이션을 위한 충분한 정보를 제공합니다.
app/src/main/res/layout/item_feed_detail_header.xml (3)
106-113: 이미지 컨테이너를 위한 ComposeView가 적절하게 추가되었습니다.ComposeView의 배치와 제약 조건이 적절하게 설정되어 있어 UI 레이아웃에 자연스럽게 통합됩니다. 상위 TextView와 하위 요소들 사이에 올바르게 배치되었습니다.
127-127: 제약 조건 업데이트가 정확히 이루어졌습니다.새로 추가된 ComposeView를 참조하도록 제약 조건이 올바르게 업데이트되었습니다.
198-199: 마진 값이 일관성 있게 조정되었습니다.세로 마진은 통일되고 좌측 마진이 감소되어 UI 레이아웃이 개선되었습니다.
app/src/main/java/com/into/websoso/ui/feedDetail/component/SingleImageContainer.kt (1)
15-32: 이미지 컨테이너 컴포저블이 잘 구현되었습니다.단일 이미지 컨테이너 구현이 깔끔하고 현대적인 Compose 패턴을 따르고 있습니다. 적절한 스타일링과 사용자 상호작용을 위한 클릭 처리가 잘 구현되어 있습니다.
1:1 비율과 균일한 패딩, 모서리 둥글기 등의 디자인 요소가 일관성 있게 적용되었습니다.
app/src/main/java/com/into/websoso/ui/feedDetail/FeedDetailActivity.kt (2)
143-148: 이미지 클릭 이벤트 처리가 적절히 구현되었습니다.인터페이스의 새 메서드가 적절히 구현되어 확장 이미지 뷰로의 네비게이션 메서드를 호출합니다.
187-192: 확장 이미지 뷰 네비게이션 기능 구현이 필요합니다.현재는 TODO 주석으로 구현되지 않았음을 표시하고 있습니다. 이는 향후 작업 항목으로 적절히 식별되어 있습니다.
이미지 확장 기능을 위한 후속 작업 일정이 있나요? 관련 이슈 번호를 주석에 추가하면 추적이 용이할 것 같습니다.
app/src/main/java/com/into/websoso/ui/feedDetail/component/TripleImageContainer.kt (1)
17-42: TripleImageContainer 구현이 잘 되었습니다.Row 레이아웃을 사용하여 세 개의 이미지를 동일한 너비로 표시하는 방식이 적절합니다. 각 이미지가 1:1 비율을 유지하고 둥근 모서리 처리가 되어 있어 UI가 깔끔해 보입니다.
app/src/main/java/com/into/websoso/ui/feedDetail/component/AdaptationFeedImageContainer.kt (2)
10-21: 이미지 컨테이너 선택 로직이 명확하게 구현되었습니다.이미지 개수에 따라 적절한 컨테이너를 선택하는 로직이 깔끔하게 구현되어 있습니다. 각 케이스별로 적합한 컴포넌트를 호출하고 있어 가독성이 좋습니다.
15-16:Details
❌ Incorrect review comment
빈 이미지 리스트 처리 필요
현재 구현에서는
imageUrls가 비어있을 경우imageUrls.first()에서NoSuchElementException이 발생할 수 있습니다.빈 리스트 상황을 처리하기 위해 다음과 같이 수정을 고려해 보세요:
- when (ImageContainerType.from(imageUrls.size)) { - SINGLE -> SingleImageContainer(imageUrls.first()) { onImageClick(0) } + when { + imageUrls.isEmpty() -> { /* 빈 이미지 처리 - 아무것도 표시하지 않거나 기본 이미지 표시 */ } + else -> when (ImageContainerType.from(imageUrls.size)) { + SINGLE -> SingleImageContainer(imageUrls.first()) { onImageClick(0) } + DOUBLE -> DoubleImageContainer(imageUrls, onImageClick) + TRIPLE -> TripleImageContainer(imageUrls, onImageClick) + MULTIPLE -> MultipleImageContainer(imageUrls, onImageClick) + }다음 검증 스크립트로 빈 리스트 처리 방식을 확인해보세요:
🏁 Script executed:
#!/bin/bash # ImageContainerType에서 빈 리스트 처리 방식 확인 rg "from\(" --include="*.kt" -A 5 -B 5Length of output: 482
아래 스크립트로
ImageContainerType.from()구현부가 어떻게 처리되고 있는지 확인해 주세요.
🏁 Script executed:
#!/bin/bash # ImageContainerType 선언 위치 확인 rg -n "class ImageContainerType" -g "*.kt" # 'from' 함수 구현부 확인 rg -n "fun from" -g "*.kt" -A5 -B5Length of output: 31945
빈 리스트 처리 로직 확인을 위해
AdaptationFeedImageContainer.kt전체 구조를 확인해 주세요.
🏁 Script executed:
#!/bin/bash # 파일 경로 확인 FILE=$(rg -l "class AdaptationFeedImageContainer" -g "*.kt") echo "Found: $FILE" # 앞쪽 100줄 출력 sed -n '1,100p' "$FILE"Length of output: 151
아래 스크립트로
AdaptationFeedImageContainer.kt파일 위치와 내용을 확인해주세요.#!/bin/bash # 파일 경로 찾기 FILE_PATH=$(fd -e kt AdaptationFeedImageContainer.kt) echo "Found: $FILE_PATH" # 파일 앞 200줄 출력 sed -n '1,200p' "$FILE_PATH"
빈 리스트 처리 확인을 위해
AdaptationFeedImageContainer.kt파일 경로와 핵심 코드를 출력해 주세요.#!/bin/bash # 파일 경로 검색 FILE=$(rg -l "AdaptationFeedImageContainer" -g "*.kt" | head -n1) echo "Found: $FILE" # 파일 상위 50줄 및 10~30라인 출력 sed -n '1,50p' "$FILE" echo "---- lines 10–30 ----" sed -n '10,30p' "$FILE"
빈 리스트 처리 로직 확인을 위해
imageUrls.first()호출 위치를 검색해 주세요.#!/bin/bash # imageUrls.first() 호출 위치 찾기 rg -n "imageUrls.first" -g "*.kt"
imageUrls.first()는 빈 리스트에서 호출되지 않습니다
현재ImageContainerType.from(0)은MULTIPLE을 반환하므로,imageUrls.size == 0일 때는SINGLE분기에 진입하지 않아imageUrls.first()가 실행되지 않습니다.
결과적으로 빈 리스트에서NoSuchElementException이 발생하지 않으므로 별도의 수정이 필요 없습니다.Likely an incorrect or invalid review comment.
| binding.cvFeedImage.setContent { | ||
| // TODO: 이미지 존재 여부에 따른 가시성 설정 | ||
| val imageUrls = List<String>(5) { | ||
| "https://upload.wikimedia.org/wikipedia/ko/9/9e/%EB%A0%88%EB%94%94_%ED%94%8C%EB%A0%88%EC%9D%B4%EC%96%B4_%EC%9B%90_%EC%98%81%ED%99%94.jpg" | ||
| } | ||
| AdaptationFeedImageContainer(imageUrls) { position -> | ||
| feedDetailClickListener.onFeedImageClick(imageUrls, position) | ||
| } | ||
| } |
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
이미지 데이터 연동 및 가시성 처리 필요
현재 구현에서는 하드코딩된 이미지 URL을 사용하고 있습니다. 실제 피드 데이터의 이미지를 사용하도록 연동이 필요합니다. 또한 TODO 주석에 언급된 대로 이미지 존재 여부에 따른 가시성 처리가 필요합니다.
- binding.cvFeedImage.setContent {
- // TODO: 이미지 존재 여부에 따른 가시성 설정
- val imageUrls = List<String>(5) {
- "https://upload.wikimedia.org/wikipedia/ko/9/9e/%EB%A0%88%EB%94%94_%ED%94%8C%EB%A0%88%EC%9D%B4%EC%96%B4_%EC%9B%90_%EC%98%81%ED%99%94.jpg"
- }
- AdaptationFeedImageContainer(imageUrls) { position ->
- feedDetailClickListener.onFeedImageClick(imageUrls, position)
- }
+ // 피드에 이미지가 있는 경우에만 이미지 컨테이너 표시
+ binding.cvFeedImage.setContent {
+ feed.imageUrls?.let { imageUrls ->
+ if (imageUrls.isNotEmpty()) {
+ AdaptationFeedImageContainer(imageUrls) { position ->
+ feedDetailClickListener.onFeedImageClick(imageUrls, position)
+ }
+ }
+ }
}또한 Compose의 라이프사이클을 관리하기 위해 setViewCompositionStrategy를 설정하는 것이 좋습니다:
binding.cvFeedImage.setViewCompositionStrategy(
ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed
)| imageUrls: List<String>, | ||
| position: Int, |
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.
a: 피드 이미지 클릭이란 함수명과 어색한 파라미터같아요! 하나의 이미지 url만 넘겨받도록 구현하는 것은 어떤가요?
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.
a: 피드 이미지 클릭이란 함수명과 어색한 파라미터같아요! 하나의 이미지 url만 넘겨받도록 구현하는 것은 어떤가요?
좌우로 넘어가는 페이지네이션 화면이 필요해서 이동시 전부 받아야 할 것 같습니다!
| binding.clFeedLike.isSelected = feed.isLiked | ||
| binding.cvFeedImage.setContent { | ||
| // TODO: 이미지 존재 여부에 따른 가시성 설정 | ||
| val imageUrls = List<String>(5) { |
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.
a: 당장은 테스트용이죠?
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.
a: 당장은 테스트용이죠?
넵! 추후 정리를 위해 TODO 주석을 달아놓았습니다 :)
| .aspectRatio(1f) | ||
| .clip(RoundedCornerShape(8.dp)) | ||
| .clickableWithoutRipple { | ||
| onImageClick(imageUrls.indexOf(imageUrl)) |
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.
a: 인덱스를 넘겨주는 이유가 있나요?
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.
a: 인덱스를 넘겨주는 이유가 있나요?
페이지네이션 적용시 초기에 보여줄 페이지를 확인시켜주기 위함입니다!
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.
forEachIndexed를 쓰면 바로 index를 넘겨줄 수 있어보여요!
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/res/drawable/bg_feed_gray_toast_radius_24dp.xml(1 hunks)app/src/main/res/layout/item_feed.xml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/src/main/res/drawable/bg_feed_gray_toast_radius_24dp.xml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
app/src/main/res/layout/item_feed.xml (1)
165-165:cl_feed_novel_info의 제약 검토
cl_feed_novel_info의app:layout_constraintTop_toBottomOf="@+id/iv_feed_image"로 변경된 것은 의도에 부합하지만, 기존android:layout_marginTop="20dp"와 결합 시 여백이 과도해질 수 있습니다.
레이아웃 Preview 를 통해 실제 간격을 확인하고, 필요 시marginTop값을 조정하세요.
| <TextView | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginEnd="12dp" | ||
| android:layout_marginBottom="10dp" | ||
| android:background="@drawable/bg_feed_gray_toast_radius_24dp" | ||
| android:paddingHorizontal="10dp" | ||
| android:paddingVertical="4dp" | ||
| android:textAppearance="@style/body3" | ||
| android:textColor="@color/white" | ||
| app:layout_constraintBottom_toBottomOf="@id/iv_feed_image" | ||
| app:layout_constraintEnd_toEndOf="@id/iv_feed_image" | ||
| tools:text="0" /> | ||
|
|
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
오버레이 텍스트뷰에 id·바인딩·조건부 노출 추가 필요
현재 오버레이 TextView 에는 ID가 없고, tools:text 만 있어 런타임 바인딩이 불가하며 항상 노출됩니다.
@+id/tv_feed_image_count와 같은 ID 를 부여하세요.feed.images.size() > 1일 때만 보이도록app:isVisible을 설정하세요.- 남은 이미지 수(예:
feed.images.size() - 1) 를 표시할 수 있도록android:text="@{String.valueOf(feed.images.size() - 1)}"바인딩을 추가하세요.
예시 제안 diff:
-<TextView
+<TextView
+ android:id="@+id/tv_feed_image_count"
+ app:isVisible="@{feed.images.size() > 1}"
+ android:text="@{String.valueOf(feed.images.size() - 1)}"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginEnd="12dp"
android:layout_marginBottom="10dp"
android:background="@drawable/bg_feed_gray_toast_radius_24dp"
android:paddingHorizontal="10dp"
android:paddingVertical="4dp"
android:textAppearance="@style/body3"
android:textColor="@color/white"
app:layout_constraintBottom_toBottomOf="@id/iv_feed_image"
app:layout_constraintEnd_toEndOf="@id/iv_feed_image"
- tools:text="0" />
+ tools:text="0" />🤖 Prompt for AI Agents
In app/src/main/res/layout/item_feed.xml around lines 139 to 152, the overlay
TextView lacks an ID and runtime binding, causing it to always be visible and
unbindable. Add an ID attribute such as @+id/tv_feed_image_count, set
app:isVisible to true only when feed.images.size() > 1, and bind android:text to
display the remaining image count using @{String.valueOf(feed.images.size() -
1)}. This will enable conditional visibility and dynamic text based on the image
count.
| <ImageView | ||
| android:id="@+id/iv_feed_image" | ||
| android:layout_width="0dp" | ||
| android:layout_height="0dp" | ||
| android:layout_marginHorizontal="12dp" | ||
| android:layout_marginTop="20dp" | ||
| app:layout_constraintBottom_toTopOf="@id/cl_feed_novel_info" | ||
| app:layout_constraintDimensionRatio="334:236" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toBottomOf="@id/tv_feed_content" | ||
| tools:src="@color/black" /> |
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
이미지 로드를 위한 바인딩 누락 및 접근성 이슈
현재 iv_feed_image 에는 tools:src 만 있어 실제로 이미지를 불러오지 않습니다.
loadImageUrl(또는 프로젝트의 커스텀 BindingAdapter) 를 이용해feed모델의 이미지 URL 을 바인딩하세요.feed.images가 비어 있을 때isVisible을 통해 뷰를 GONE 처리해야 합니다.- 접근성 향상을 위해
contentDescription속성도 추가가 필요합니다.
예시 제안 diff:
<ImageView
android:id="@+id/iv_feed_image"
android:layout_width="0dp"
android:layout_height="0dp"
android:layout_marginHorizontal="12dp"
android:layout_marginTop="20dp"
- tools:src="@color/black" />
+ app:loadImageUrl="@{feed.images.size() > 0 ? feed.images.get(0) : null}"
+ app:cornerRadius="@{8f}"
+ android:contentDescription="@{feed.content}"
+ app:isVisible="@{feed.images.size() > 0}" />📝 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.
| <ImageView | |
| android:id="@+id/iv_feed_image" | |
| android:layout_width="0dp" | |
| android:layout_height="0dp" | |
| android:layout_marginHorizontal="12dp" | |
| android:layout_marginTop="20dp" | |
| app:layout_constraintBottom_toTopOf="@id/cl_feed_novel_info" | |
| app:layout_constraintDimensionRatio="334:236" | |
| app:layout_constraintEnd_toEndOf="parent" | |
| app:layout_constraintStart_toStartOf="parent" | |
| app:layout_constraintTop_toBottomOf="@id/tv_feed_content" | |
| tools:src="@color/black" /> | |
| <ImageView | |
| android:id="@+id/iv_feed_image" | |
| android:layout_width="0dp" | |
| android:layout_height="0dp" | |
| android:layout_marginHorizontal="12dp" | |
| android:layout_marginTop="20dp" | |
| app:layout_constraintBottom_toTopOf="@id/cl_feed_novel_info" | |
| app:layout_constraintDimensionRatio="334:236" | |
| app:layout_constraintEnd_toEndOf="parent" | |
| app:layout_constraintStart_toStartOf="parent" | |
| app:layout_constraintTop_toBottomOf="@id/tv_feed_content" | |
| app:loadImageUrl="@{feed.images.size() > 0 ? feed.images.get(0) : null}" | |
| app:cornerRadius="@{8f}" | |
| android:contentDescription="@{feed.content}" | |
| app:isVisible="@{feed.images.size() > 0}" /> |
🤖 Prompt for AI Agents
In app/src/main/res/layout/item_feed.xml around lines 126 to 137, the ImageView
with id iv_feed_image only has a tools:src attribute and does not bind the
actual image URL from the feed model. To fix this, add a data binding expression
using the project's loadImageUrl BindingAdapter to bind feed.images URL to the
ImageView. Also, use isVisible binding to set the view's visibility to GONE when
feed.images is empty. Finally, add a contentDescription attribute bound to a
descriptive text from the feed model to improve accessibility.
| @Composable | ||
| fun AdaptationFeedImageContainer( | ||
| imageUrls: List<String>, | ||
| onImageClick: (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: 이 int 값이 어떤 값인지 명시해주면 좋을 것 같아요
| @Composable | ||
| fun DoubleImageContainer( | ||
| imageUrls: List<String>, | ||
| onImageClick: (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: 여기도 마찬가지
| .padding(horizontal = 20.dp), | ||
| horizontalArrangement = Arrangement.spacedBy(8.dp), | ||
| ) { | ||
| imageUrls.take(2).forEach { imageUrl -> |
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: 이 take 하는 값들 상수화 해주면 좋을 것 같아요
| @Composable | ||
| fun MultipleImageContainer( | ||
| imageUrls: List<String>, | ||
| onImageClick: (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: 여기도 명시
| package com.into.websoso.ui.feedDetail.model | ||
|
|
||
| enum class ImageContainerType( | ||
| private val size: Int, |
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.
a: 아니면 차라리 얘를 public 으로 열어서 take 값으로 사용하면 안 되나요 ?
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.
a: 아니면 차라리 얘를 public 으로 열어서 take 값으로 사용하면 안 되나요 ?
아니 맞네 왜 private인거죠
…o feat/684 # Conflicts: # app/src/main/res/layout/item_feed_detail_header.xml
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
📷𝘚𝘤𝘳𝘦𝘦𝘯𝘴𝘩𝘰𝘵
Screen_recording_20250515_005949.mp4
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴
오랜만에 컴포즈하니 재밌네요
좀 커질까봐 한턴 끊어가요~
Summary by CodeRabbit