Conversation
📝 WalkthroughWalkthrough저장된 콘텐츠 리스트 아이템 UI 컴포넌트를 구현하기 위해 ContentModel, OttModel 데이터 클래스와 OttType 열거형을 추가했고, SavedContentItem과 OttHorizontalList 컴포저블을 작성하여 컨텐츠 포스터 이미지, OTT 리스트 오버레이, 제목 및 연도 정보를 표시합니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
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
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: 3
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/flint/core/designsystem/component/content/OttHorizontalList.kt:
- Around line 28-32: The code computes firstOttIcon from ottList and then always
renders an Image with rememberAsyncImagePainter(null), causing an
empty/incorrect UI when ottList is empty; update the logic in OttHorizontalList
so that you use ottList.firstOrNull()?.iconRes (or equivalent) to get
firstOttIcon and render the Image only when firstOttIcon is non-null (or render
a defined placeholder/fallback drawable), i.e., guard the Image composable with
a null check on firstOttIcon (or replace the painter with a proper placeholder)
to avoid drawing an empty image.
In
@app/src/main/java/com/flint/core/designsystem/component/content/SavedContentItem.kt:
- Around line 27-29: The code uses OttType.valueOf(...) when mapping
contentModel.ottSimpleList to ottList, which throws IllegalArgumentException for
unknown names; change this to a safe conversion by either adding a companion
helper like OttType.fromNameOrNull(name) that returns null for non-matching
names or by using a safe lookup (e.g., OttType.values().find { it.name == name
}) when building ottList, and filter or provide a default for nulls so
unexpected server values no longer crash the app.
- Around line 40-43: NetworkImage is missing a size modifier so it may not fill
its parent Box; update the NetworkImage invocation to include
Modifier.fillMaxSize() (e.g., pass modifier = Modifier.fillMaxSize()) so the
image fills the container, keeping the existing shape =
RoundedCornerShape(12.dp) argument intact.
🧹 Nitpick comments (5)
app/src/main/java/com/flint/domain/type/OttType.kt (1)
1-11: 도메인 레이어에서 Android 리소스 참조는 클린 아키텍처 위반입니다.
com.flint.domain.type패키지는 도메인 레이어에 위치하지만com.flint.R을 임포트하여 Android 리소스에 의존하고 있습니다. 도메인 레이어는 프레임워크에 독립적이어야 합니다.다음 두 가지 방안을 고려해 주세요:
- 이 enum을
core.designsystem또는presentation레이어로 이동- 도메인에는 순수 enum만 두고, 리소스 매핑은 별도 mapper를 통해 처리
app/src/main/java/com/flint/core/designsystem/component/content/OttHorizontalList.kt (3)
49-62: 로컬 drawable 리소스에는painterResource사용을 권장합니다.
rememberAsyncImagePainter는 네트워크 이미지 로딩을 위한 Coil API입니다. 번들된 drawable 리소스(iconRes)에는painterResource(iconRes)가 더 효율적이고 적합합니다.♻️ 제안하는 수정
+import androidx.compose.ui.res.painterResource ... Image( - painter = rememberAsyncImagePainter(firstOttIcon), + painter = painterResource(firstOttIcon!!), contentDescription = null,마찬가지로
secondOttIcon에도 동일하게 적용해 주세요.
54-61: Shadow 설정이 중복되어 있습니다.동일한 shadow 설정이 3번 반복됩니다. Modifier extension이나 상수로 추출하면 유지보수성이 향상됩니다.
♻️ 제안하는 수정
private fun Modifier.ottIconShadow() = this.dropShadow( shape = CircleShape, color = Color(0xFF000000).copy(alpha = 0.35f), offsetX = (-4).dp, offsetY = 0.dp, blur = 6.dp, spread = 0.dp )
40-44: 문자열 비교 대신 정수 비교를 권장합니다.
etcOttText를"0"과 비교하는 것보다 정수 값으로 관리하면 더 명확합니다.♻️ 제안하는 수정
- val etcOttText = if (ottList.size > 2) { - "+${ottList.size - 2}" - } else { - "0" - } + val etcCount = ottList.size - 2 ... - if (etcOttText != "0") { + if (etcCount > 0) { ... - text = etcOttText, + text = "+$etcCount",app/src/main/java/com/flint/core/designsystem/component/content/SavedContentItem.kt (1)
62-66: 하드코딩된 한국어 문자열은 string resource로 분리를 권장합니다.다국어 지원을 고려한다면
"${contentModel.year}년도"를 string resource로 분리하는 것이 좋습니다.<!-- strings.xml --> <string name="year_format">%d년도</string>text = stringResource(R.string.year_format, contentModel.year)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/com/flint/core/designsystem/component/content/OttHorizontalList.ktapp/src/main/java/com/flint/core/designsystem/component/content/SavedContentItem.ktapp/src/main/java/com/flint/domain/model/ContentModel.ktapp/src/main/java/com/flint/domain/model/OttModel.ktapp/src/main/java/com/flint/domain/type/OttType.kt
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/main/java/com/flint/core/designsystem/component/content/SavedContentItem.kt (3)
app/src/main/java/com/flint/core/designsystem/component/image/NetworkImage.kt (1)
NetworkImage(22-46)app/src/main/java/com/flint/core/designsystem/component/content/OttHorizontalList.kt (1)
OttHorizontalList(23-108)app/src/main/java/com/flint/core/designsystem/theme/Theme.kt (1)
FlintTheme(8-16)
app/src/main/java/com/flint/core/designsystem/component/content/OttHorizontalList.kt (1)
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 (4)
app/src/main/java/com/flint/core/designsystem/component/content/OttHorizontalList.kt (1)
110-126: LGTM!Preview 함수가
FlintTheme으로 올바르게 감싸져 있고, 다양한 OttType 샘플 데이터로 테스트하고 있습니다.app/src/main/java/com/flint/domain/model/OttModel.kt (1)
3-7: LGTM!순수 Kotlin 데이터 클래스로 도메인 모델이 깔끔하게 정의되어 있습니다.
ottName이OttType.valueOf()와 함께 사용되는 부분은SavedContentItem.kt리뷰에서 다루겠습니다.app/src/main/java/com/flint/domain/model/ContentModel.kt (1)
3-9: LGTM!콘텐츠 정보를 담는 도메인 모델이 적절하게 정의되어 있습니다.
app/src/main/java/com/flint/core/designsystem/component/content/SavedContentItem.kt (1)
70-112: LGTM!Preview가
FlintTheme으로 올바르게 감싸져 있고, 샘플 데이터가 적절하게 구성되어 있습니다.
app/src/main/java/com/flint/core/designsystem/component/content/OttHorizontalList.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/flint/core/designsystem/component/content/SavedContentItem.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/flint/core/designsystem/component/listItem/SavedContentItem.kt
Show resolved
Hide resolved
nahy-512
left a comment
nahy-512
left a comment
There was a problem hiding this comment.
작업 고생하셨습니당~!
동작에는 문제 없는데, 고민해보면 좋을 것 같아 코멘트 몇개 달아봤어요
| val firstOttIcon = ottList[0].iconRes | ||
|
|
||
| val secondOttIcon = if (ottList.size > 1) { | ||
| ottList[1].iconRes | ||
| } else { | ||
| null | ||
| } | ||
|
|
||
| val etcOttText = if (ottList.size > 2) { | ||
| "+${ottList.size - 2}" | ||
| } else { | ||
| "0" | ||
| } |
There was a problem hiding this comment.
p2
이거,, 최대 표시 개수(2) 기준으로 로직 좀 다듬어보면 좋을 것 같긴 한데ㅎㅎ
지금 너무 하드코딩 느낌..이지만 꼭 하지는 않아도 괜찮을 것 같아용
|
|
||
| import com.flint.R | ||
|
|
||
| enum class OttType(val imageRes: Int, val iconRes: Int) { |
There was a problem hiding this comment.
p1
이거 이름 구분 이유가 있나요??
image랑 icon이 네이밍 구분이 조금 애매한 것 같은,, 차라리 shape로 구분해보면 어떨까요?
There was a problem hiding this comment.
피그마상으로
원형 아이콘 -> icon
네모 아이콘 -> img
라고 구분되어 있길래 저희쪽에서도 똑같이 구분하면 어떨까 싶어서 나누어 보았습니다
There was a problem hiding this comment.
아이콘이랑 이미지라는 이름만으로는 내부 컴포넌트 모습이 상상이 잘 안되는 것 같아서, 차라리 용도나 모습을 이름에서 나타내보면 조금 더 좋을 것 같다는 생각입니다
| import com.flint.R | ||
|
|
||
| enum class OttType(val imageRes: Int, val iconRes: Int) { | ||
| Netflix(R.drawable.img_netflix, R.drawable.ic_small_netflix), |
There was a problem hiding this comment.
바텀시트 구현하려면 Ott 이름도 추가해보면 좋을 것 같아요!
| data class OttModel( | ||
| val ottId: Long, | ||
| val ottName: String, | ||
| val logoUrl: String | ||
| ) |
There was a problem hiding this comment.
p2
이거 서버에서 오는 응답 형태인가요??
enum으로 구분해봐도 충분하지 않을까 싶어서요..!!
logoUrl 말고도 아마 OTT 웹뷰로 이동하는 용도로 링크 추가로 받아봐야 할 것 같은데, 나중에라도 추가해주시면 좋을 것 같아요
There was a problem hiding this comment.
일단 서버에서 정해놓은 DTO를 기반으로 Model 만들었고 swagger에 데이터 추가되면 넣을 생각입니다
| val ottList = contentModel.ottSimpleList.mapNotNull { | ||
| OttType.entries.find { type -> type.name == it.ottName } | ||
| } |
There was a problem hiding this comment.
p2
ContentModel에서 List<OttModel>이 아니라 List<OttType>를 가지고 있으면 코드가 더 간결해질 것 같습니다~~
enum 내부 함수로 이름으로 type 반환할 수도 있고??
| Wave(R.drawable.img_wave, R.drawable.ic_small_wave), | ||
| Coupang(R.drawable.img_coupang, R.drawable.ic_small_coupang), | ||
| Disney(R.drawable.img_disney, R.drawable.ic_small_disney) | ||
| enum class OttType(val imageRes: Int, val iconRes: Int, val ottName: String) { |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/src/main/java/com/flint/domain/type/OttType.kt (1)
3-12: 도메인 레이어에서 Android 리소스 참조 분리 고려도메인 레이어(
com.flint.domain.type)에서R.drawable을 직접 참조하고 있습니다. 클린 아키텍처 관점에서 도메인 레이어는 플랫폼에 독립적이어야 합니다.리소스 매핑을 presentation/designsystem 레이어로 분리하거나, 별도의 mapper를 통해
OttType을 UI 리소스로 변환하는 방식을 고려해 보세요.♻️ 제안: 리소스 매핑 분리
// domain/type/OttType.kt - 플랫폼 독립적 enum class OttType(val ottName: String) { Netflix("넷플릭스"), Tving("티빙"), // ... } // designsystem/mapper/OttResourceMapper.kt - UI 레이어 fun OttType.toImageRes(): Int = when (this) { OttType.Netflix -> R.drawable.img_netflix // ... } fun OttType.toIconRes(): Int = when (this) { OttType.Netflix -> R.drawable.ic_small_netflix // ... }app/src/main/java/com/flint/core/designsystem/component/content/OttHorizontalList.kt (2)
40-44:etcOttText로직 개선 고려
"0"을 sentinel 값으로 사용하는 것은 가독성이 떨어집니다. nullable 또는 boolean 플래그 사용을 고려해 보세요.♻️ 개선 제안
- val etcOttText = if (ottList.size > maxOttShowingCount) { - "+${ottList.size - maxOttShowingCount}" - } else { - "0" - } + val remainingCount = ottList.size - maxOttShowingCount + val etcOttText = if (remainingCount > 0) "+$remainingCount" else null- if (etcOttText != "0") { + if (etcOttText != null) {
54-61: 중복된dropShadow설정 추출 고려동일한 shadow 설정이 세 곳에서 반복됩니다. 공통 Modifier로 추출하면 유지보수성이 향상됩니다.
♻️ 개선 제안
private val ottIconShadowModifier = Modifier .size(28.dp) .dropShadow( shape = CircleShape, color = Color(0xFF000000).copy(alpha = 0.35f), offsetX = (-4).dp, offsetY = 0.dp, blur = 6.dp, spread = 0.dp )Also applies to: 71-78, 87-94
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/com/flint/core/designsystem/component/content/OttHorizontalList.ktapp/src/main/java/com/flint/core/designsystem/component/content/SavedContentItem.ktapp/src/main/java/com/flint/domain/model/ContentModel.ktapp/src/main/java/com/flint/domain/type/OttType.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/flint/core/designsystem/component/content/SavedContentItem.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/flint/core/designsystem/component/content/OttHorizontalList.kt (1)
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 (2)
app/src/main/java/com/flint/domain/model/ContentModel.kt (1)
5-11: LGTM!도메인 모델이 깔끔하게 정의되어 있습니다.
OttType열거형을 사용하여 타입 안전성을 확보한 점이 좋습니다.app/src/main/java/com/flint/core/designsystem/component/content/OttHorizontalList.kt (1)
110-126: Preview 구현 적절함
FlintTheme으로 래핑하고 다양한 OTT 타입으로 테스트하는 Preview가 잘 구성되어 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/flint/core/designsystem/component/listView/OttHorizontalList.kt:
- Around line 49-62: The Image composable is using rememberAsyncImagePainter
with firstOttIcon/secondOttIcon (values from OttType.iconRes which are drawable
resource Ints); replace rememberAsyncImagePainter with painterResource when
rendering local drawable IDs: locate the Image calls that reference firstOttIcon
and secondOttIcon and swap rememberAsyncImagePainter(...) for painterResource(id
= ...) so the Image uses painterResource for local drawable resources instead of
the network/URI async painter.
🧹 Nitpick comments (4)
app/src/main/java/com/flint/core/designsystem/component/listView/OttHorizontalList.kt (3)
40-44:etcOttText로직 개선 제안매직 스트링
"0"을 센티넬 값으로 사용하는 것보다 nullable 타입을 활용하면 의도가 더 명확해집니다.♻️ 제안하는 수정
- val etcOttText = if (ottList.size > maxOttShowingCount) { - "+${ottList.size - maxOttShowingCount}" - } else { - "0" - } + val etcOttText: String? = if (ottList.size > maxOttShowingCount) { + "+${ottList.size - maxOttShowingCount}" + } else { + null + }그리고 Line 82의 조건문도 변경:
- if (etcOttText != "0") { + if (etcOttText != null) {
51-51: 접근성:contentDescription제공 권장스크린 리더 사용자를 위해 OTT 서비스 이름을
contentDescription으로 제공하면 접근성이 향상됩니다.♻️ 제안하는 수정
Image( painter = rememberAsyncImagePainter(firstOttIcon), - contentDescription = null, + contentDescription = ottList[0].ottName,Also applies to: 67-67
54-61: 중복된dropShadow설정 추출 고려동일한
dropShadow파라미터가 3회 반복됩니다. 공통 Modifier 또는 상수로 추출하면 유지보수성이 향상됩니다.♻️ 제안하는 수정
private fun Modifier.ottIconShadow() = this.dropShadow( shape = CircleShape, color = Color(0xFF000000).copy(alpha = 0.35f), offsetX = (-4).dp, offsetY = 0.dp, blur = 6.dp, spread = 0.dp )Also applies to: 71-78, 87-94
app/src/main/java/com/flint/core/designsystem/component/listItem/SavedContentItem.kt (1)
60-64: 하드코딩된 문자열 "년도" - i18n 고려향후 다국어 지원을 고려한다면 문자열 리소스로 분리하는 것이 좋습니다. 현재 스코프에서는 큰 문제가 아니므로 나중에 처리해도 됩니다.
♻️ 제안하는 수정
strings.xml:<string name="content_year_format">%d년도</string>Text( - text = "${contentModel.year}년도", + text = stringResource(R.string.content_year_format, contentModel.year), color = FlintTheme.colors.gray300, style = FlintTheme.typography.caption1R12 )
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/flint/core/designsystem/component/listItem/CollectionItem.ktapp/src/main/java/com/flint/core/designsystem/component/listItem/SavedContentItem.ktapp/src/main/java/com/flint/core/designsystem/component/listView/OttHorizontalList.kt
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/main/java/com/flint/core/designsystem/component/listItem/SavedContentItem.kt (3)
app/src/main/java/com/flint/core/designsystem/component/image/NetworkImage.kt (1)
NetworkImage(22-46)app/src/main/java/com/flint/core/designsystem/component/listView/OttHorizontalList.kt (1)
OttHorizontalList(25-108)app/src/main/java/com/flint/core/designsystem/theme/Theme.kt (1)
FlintTheme(8-16)
app/src/main/java/com/flint/core/designsystem/component/listView/OttHorizontalList.kt (1)
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 (5)
app/src/main/java/com/flint/core/designsystem/component/listItem/CollectionItem.kt (1)
1-1:listItem패키지로의 이동 승인, 모든 import 경로 정상 업데이트됨
collection패키지에서listItem패키지로 이동하여 리스트 아이템 관련 컴포넌트를 일관되게 구성한 좋은 리팩토링입니다. 모든 기존 import 경로가 정상적으로 업데이트되었으며, 파일도 올바른 위치에 있습니다.app/src/main/java/com/flint/core/designsystem/component/listView/OttHorizontalList.kt (2)
1-23: LGTM!패키지 구조, import, 상수 정의가 적절합니다.
110-126: LGTM!Preview가 FlintTheme으로 감싸져 있고, overflow 케이스를 잘 보여주는 샘플 데이터를 사용합니다.
app/src/main/java/com/flint/core/designsystem/component/listItem/SavedContentItem.kt (2)
23-66: LGTM!
SavedContentItem컴포넌트의 구조가 잘 설계되었습니다:
Box를 활용한 OTT 리스트 오버레이NetworkImage재사용- FlintTheme을 통한 일관된 스타일링
68-91: LGTM!Preview가 적절한 샘플 데이터와 함께 FlintTheme으로 잘 구성되어 있습니다. 빈
posterImage는NetworkImage의 에러 플레이스홀더로 처리됩니다.
| Image( | ||
| painter = rememberAsyncImagePainter(firstOttIcon), | ||
| contentDescription = null, | ||
| modifier = Modifier | ||
| .size(28.dp) | ||
| .dropShadow( | ||
| shape = CircleShape, | ||
| color = Color(0xFF000000).copy(alpha = 0.35f), | ||
| offsetX = (-4).dp, | ||
| offsetY = 0.dp, | ||
| blur = 6.dp, | ||
| spread = 0.dp | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for OttType enum definition
fd OttType.kt --type fRepository: imflint/Flint-Android
Length of output: 115
🏁 Script executed:
cat -n app/src/main/java/com/flint/domain/type/OttType.ktRepository: imflint/Flint-Android
Length of output: 703
🏁 Script executed:
cat -n app/src/main/java/com/flint/core/designsystem/component/listView/OttHorizontalList.kt | head -80Repository: imflint/Flint-Android
Length of output: 3147
로컬 drawable 리소스에는 painterResource() 사용 필수
firstOttIcon과 secondOttIcon은 OttType.iconRes로부터 추출된 drawable 리소스 ID(Int)입니다. 이는 rememberAsyncImagePainter()가 아닌 painterResource()를 사용해야 합니다. rememberAsyncImagePainter는 네트워크 이미지나 URI용으로 설계되었으며, 로컬 drawable 리소스에는 부적절합니다.
♻️ 수정 방안
+import androidx.compose.ui.res.painterResource
-import coil3.compose.rememberAsyncImagePainter Image(
- painter = rememberAsyncImagePainter(firstOttIcon),
+ painter = painterResource(firstOttIcon), Image(
- painter = rememberAsyncImagePainter(secondOttIcon),
+ painter = painterResource(secondOttIcon),🤖 Prompt for AI Agents
In
@app/src/main/java/com/flint/core/designsystem/component/listView/OttHorizontalList.kt
around lines 49 - 62, The Image composable is using rememberAsyncImagePainter
with firstOttIcon/secondOttIcon (values from OttType.iconRes which are drawable
resource Ints); replace rememberAsyncImagePainter with painterResource when
rendering local drawable IDs: locate the Image calls that reference firstOttIcon
and secondOttIcon and swap rememberAsyncImagePainter(...) for painterResource(id
= ...) so the Image uses painterResource for local drawable resources instead of
the network/URI async painter.
2 participants
📮 관련 이슈
📌 작업 내용
📸 스크린샷
|
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.