Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,13 @@ class FeedDetailActivity : BaseActivity<ActivityFeedDetailBinding>(activity_feed
override fun onFeedDetailClick(view: View) {
view.hideKeyboard()
}

override fun onFeedImageClick(
imageUrls: List<String>,
position: Int,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a: 피드 이미지 클릭이란 함수명과 어색한 파라미터같아요! 하나의 이미지 url만 넘겨받도록 구현하는 것은 어떤가요?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a: 피드 이미지 클릭이란 함수명과 어색한 파라미터같아요! 하나의 이미지 url만 넘겨받도록 구현하는 것은 어떤가요?

좌우로 넘어가는 페이지네이션 화면이 필요해서 이동시 전부 받아야 할 것 같습니다!

) {
navigateToExpandedImage(imageUrls, position)
}
}

private fun navigateToNovelDetail(novelId: Long) {
Expand Down Expand Up @@ -177,6 +184,13 @@ class FeedDetailActivity : BaseActivity<ActivityFeedDetailBinding>(activity_feed
)
}

private fun navigateToExpandedImage(
imageUrls: List<String>,
position: Int,
) {
// TODO("Not yet implemented")
}

private fun bindMenuByIsMine(
id: Long,
isMine: Boolean,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,9 @@ interface FeedDetailClickListener {
)

fun onFeedDetailClick(view: View)

fun onFeedImageClick(
imageUrls: List<String>,
position: Int,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import androidx.recyclerview.widget.RecyclerView
import com.into.websoso.core.common.util.getS3ImageUrl
import com.into.websoso.databinding.ItemFeedDetailHeaderBinding
import com.into.websoso.ui.feedDetail.FeedDetailClickListener
import com.into.websoso.ui.feedDetail.component.AdaptationFeedImageContainer
import com.into.websoso.ui.main.feed.model.FeedModel

class FeedDetailContentViewHolder(
feedDetailClickListener: FeedDetailClickListener,
private val feedDetailClickListener: FeedDetailClickListener,
private val binding: ItemFeedDetailHeaderBinding,
) : RecyclerView.ViewHolder(binding.root) {
init {
Expand All @@ -21,6 +22,15 @@ class FeedDetailContentViewHolder(
user = feed.user.copy(avatarImage = itemView.getS3ImageUrl(feed.user.avatarImage)),
)
binding.clFeedLike.isSelected = feed.isLiked
binding.cvFeedImage.setContent {
// TODO: 이미지 존재 여부에 따른 가시성 설정
val imageUrls = List<String>(5) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a: 당장은 테스트용이죠?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a: 당장은 테스트용이죠?

넵! 추후 정리를 위해 TODO 주석을 달아놓았습니다 :)

"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)
}
}
Comment on lines +25 to +33
Copy link
Copy Markdown

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
)

}

companion object {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package com.into.websoso.ui.feedDetail.component

import androidx.compose.runtime.Composable
import com.into.websoso.ui.feedDetail.model.ImageContainerType
import com.into.websoso.ui.feedDetail.model.ImageContainerType.DOUBLE
import com.into.websoso.ui.feedDetail.model.ImageContainerType.MULTIPLE
import com.into.websoso.ui.feedDetail.model.ImageContainerType.SINGLE
import com.into.websoso.ui.feedDetail.model.ImageContainerType.TRIPLE

@Composable
fun AdaptationFeedImageContainer(
imageUrls: List<String>,
onImageClick: (Int) -> Unit,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c: 이 int 값이 어떤 값인지 명시해주면 좋을 것 같아요

) {
when (ImageContainerType.from(imageUrls.size)) {
SINGLE -> SingleImageContainer(imageUrls.first()) { onImageClick(0) }
DOUBLE -> DoubleImageContainer(imageUrls, onImageClick)
TRIPLE -> TripleImageContainer(imageUrls, onImageClick)
MULTIPLE -> MultipleImageContainer(imageUrls, onImageClick)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.into.websoso.ui.feedDetail.component

import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.aspectRatio
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.layout.ContentScale
import androidx.compose.ui.unit.dp
import com.into.websoso.core.common.ui.component.AdaptationImage
import com.into.websoso.core.common.util.clickableWithoutRipple

@Composable
fun DoubleImageContainer(
imageUrls: List<String>,
onImageClick: (Int) -> Unit,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c: 여기도 마찬가지

) {
Row(
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 20.dp),
horizontalArrangement = Arrangement.spacedBy(8.dp),
) {
imageUrls.take(2).forEach { imageUrl ->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c: 이 take 하는 값들 상수화 해주면 좋을 것 같아요

AdaptationImage(
imageUrl = imageUrl,
contentScale = ContentScale.Crop,
modifier = Modifier
.weight(1f)
.aspectRatio(1f)
.clip(RoundedCornerShape(8.dp))
.clickableWithoutRipple {
onImageClick(imageUrls.indexOf(imageUrl))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a: 인덱스를 넘겨주는 이유가 있나요?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a: 인덱스를 넘겨주는 이유가 있나요?

페이지네이션 적용시 초기에 보여줄 페이지를 확인시켜주기 위함입니다!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forEachIndexed를 쓰면 바로 index를 넘겨줄 수 있어보여요!

},
)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package com.into.websoso.ui.feedDetail.component

import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.aspectRatio
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.lazy.LazyRow
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.layout.ContentScale
import androidx.compose.ui.unit.dp
import com.into.websoso.core.common.ui.component.AdaptationImage
import com.into.websoso.core.common.util.clickableWithoutRipple

@Composable
fun MultipleImageContainer(
imageUrls: List<String>,
onImageClick: (Int) -> Unit,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c: 여기도 명시

) {
LazyRow(
modifier = Modifier
.fillMaxWidth(),
horizontalArrangement = Arrangement.spacedBy(6.dp),
contentPadding = PaddingValues(horizontal = 20.dp),
) {
items(imageUrls.size) { position ->
AdaptationImage(
imageUrl = imageUrls[position],
contentScale = ContentScale.Crop,
modifier = Modifier
.size(100.dp)
.aspectRatio(1f)
.clip(RoundedCornerShape(8.dp))
.clickableWithoutRipple {
onImageClick(position)
},
)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.into.websoso.ui.feedDetail.component

import androidx.compose.foundation.layout.aspectRatio
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.layout.ContentScale
import androidx.compose.ui.unit.dp
import com.into.websoso.core.common.ui.component.AdaptationImage
import com.into.websoso.core.common.util.clickableWithoutRipple

@Composable
fun SingleImageContainer(
imageUrl: String,
onImageClick: () -> Unit,
) {
AdaptationImage(
imageUrl = imageUrl,
contentScale = ContentScale.Crop,
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 20.dp)
.aspectRatio(1f)
.clip(RoundedCornerShape(8.dp))
.clickableWithoutRipple {
onImageClick()
},
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.into.websoso.ui.feedDetail.component

import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.aspectRatio
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.layout.ContentScale
import androidx.compose.ui.unit.dp
import com.into.websoso.core.common.ui.component.AdaptationImage
import com.into.websoso.core.common.util.clickableWithoutRipple

@Composable
fun TripleImageContainer(
imageUrls: List<String>,
onImageClick: (Int) -> Unit,
) {
Row(
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 20.dp),
horizontalArrangement = Arrangement.spacedBy(8.dp),
) {
imageUrls.take(3).forEach { imageUrl ->
AdaptationImage(
imageUrl = imageUrl,
contentScale = ContentScale.Crop,
modifier = Modifier
.weight(1f)
.aspectRatio(1f)
.clip(RoundedCornerShape(8.dp))
.clickableWithoutRipple {
onImageClick(imageUrls.indexOf(imageUrl))
},
)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.into.websoso.ui.feedDetail.model

enum class ImageContainerType(
private val size: Int,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a: 아니면 차라리 얘를 public 으로 열어서 take 값으로 사용하면 안 되나요 ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a: 아니면 차라리 얘를 public 으로 열어서 take 값으로 사용하면 안 되나요 ?

아니 맞네 왜 private인거죠

) {
SINGLE(1),
DOUBLE(2),
TRIPLE(3),
MULTIPLE(4),
;

companion object {
fun from(size: Int): ImageContainerType = ImageContainerType.entries.find { it.size == size } ?: MULTIPLE
}
}
16 changes: 12 additions & 4 deletions app/src/main/res/layout/item_feed_detail_header.xml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@
app:layout_constraintTop_toBottomOf="@+id/cl_feed_profile"
tools:text="최근 잼께 봤던 무협 소설 목록 진짜들만 골라봤어요 애매한 건 빼고 주로 무협 중에서도 회귀물로 골라봤어요 왜냐면 내가 회귀물을 좋아하기 때문에 최근 잼께 봤던 무협 소설 목록 진짜들만 골라봤어요최근 잼께 봤던 무협 소설 목록 진짜들을 가 회귀물을 좋아하기 때문에 최근 잼께 봤습니다 몇글자 까지 될지 모르겠네요" />

<androidx.compose.ui.platform.ComposeView
android:id="@+id/cv_feed_image"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="20dp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/tv_feed_content" />

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/cl_feed_novel_info"
isVisible="@{!feed.novel.nothing}"
Expand All @@ -115,7 +124,7 @@
android:onClick="@{() -> onClick.onNovelInfoClick(feed.novel.id)}"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/tv_feed_content">
app:layout_constraintTop_toBottomOf="@+id/cv_feed_image">

<ImageView
android:id="@+id/iv_feed_link"
Expand Down Expand Up @@ -186,9 +195,8 @@
android:id="@+id/cl_feed_like"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="20dp"
android:layout_marginTop="10dp"
android:layout_marginBottom="28dp"
android:layout_marginVertical="10dp"
android:layout_marginStart="12dp"
android:onClick="@{(cl_feed_like) -> onClick.onLikeButtonClick(cl_feed_like, feed.id)}"
app:layout_constraintBottom_toTopOf="@+id/view_feed_indicator"
app:layout_constraintStart_toStartOf="parent"
Expand Down
Loading