-
Notifications
You must be signed in to change notification settings - Fork 0
[FEAT/#419] 이미지 로딩 뷰를 추가했어요 #420
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
WalkthroughSpoonyImage에 로딩 인디케이터 옵션이 추가되어 내부적으로 SubcomposeAsyncImage를 사용해 로딩 상태에서 CircularProgressIndicator를 표시하도록 변경되었고, 여러 호출부에서 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as 호출 Composable
participant SI as SpoonyImage
participant Coil as SubcomposeAsyncImage
participant L as Loading UI (CircularProgressIndicator)
participant C as 이미지 컨텐츠
UI->>SI: SpoonyImage(model, showLoadingIndicator)
SI->>Coil: 이미지 요청
alt 로딩 상태 (showLoadingIndicator == true)
SI-->>L: 로딩 인디케이터 표시
end
Coil-->>SI: 로드 성공
SI-->>C: SubcomposeAsyncImageContent 렌더링
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 주의 검토 포인트:
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/spoony/spoony/presentation/placeDetail/component/PlaceDetailImageLazyRow.kt (1)
33-41: 로딩 인디케이터 대비 문제PlaceDetailImageLazyRow.kt에서 지정한 배경색(SpoonyAndroidTheme.colors.gray300)과 SpoonyImage.kt의 인디케이터 색상(SpoonyAndroidTheme.colors.gray300)이 동일해 로딩 중 스피너가 보이지 않습니다.
- 배경색 제거/변경: PlaceDetailImageLazyRow.kt에서
.background(...)삭제 또는 대비 높은 색으로 교체- 인디케이터 색 조정: SpoonyImage.kt의
CircularProgressIndicator(color = …)를 SpoonyAndroidTheme.colors.gray500 이상으로 변경다크모드 대비도 함께 확인 바랍니다.
🧹 Nitpick comments (4)
app/src/main/java/com/spoony/spoony/core/designsystem/component/image/SpoonyImage.kt (2)
39-59: 에러 상태 미처리 · 로딩 UX 보완 제안
- error 상태에서 현재 아무것도 표시되지 않습니다. 최소한의 placeholder를 보여주면 UX 공백을 줄일 수 있어요.
- crossfade를 켜면 빠르게 로드될 때의 깜빡임도 완화됩니다.
아래와 같이 보완을 제안드립니다.
- import 추가:
import androidx.compose.foundation.background import androidx.compose.ui.platform.LocalContext import coil3.request.ImageRequest
- SubcomposeAsyncImage 개선(diff):
- SubcomposeAsyncImage( - model = model, + SubcomposeAsyncImage( + model = ImageRequest.Builder(LocalContext.current) + .data(model) + .crossfade(true) + .build(), contentDescription = contentDescription, contentScale = contentScale, modifier = modifier.clip(shape), loading = { if (showLoadingIndicator) { Box( modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center ) { CircularProgressIndicator( - color = SpoonyAndroidTheme.colors.gray300 + color = SpoonyAndroidTheme.colors.gray300 ) } } }, success = { SubcomposeAsyncImageContent() - } + }, + error = { + // 최소 placeholder + Box(modifier = Modifier.fillMaxSize().background(SpoonyAndroidTheme.colors.gray100)) + } )
51-52: 인디케이터 가시성(대비) 개선 제안여러 화면(예: PlaceDetailImageLazyRow)에서 배경이 gray300이면 인디케이터(gray300)와 색이 같아 거의 보이지 않을 수 있습니다. 기본 색을 한 단계 진한 gray500 등으로 올리거나, 색을 파라미터로 노출해 호출처에서 조절 가능하도록 하는 것을 권장합니다.
간단 수정 예:
- CircularProgressIndicator( - color = SpoonyAndroidTheme.colors.gray300 - ) + CircularProgressIndicator( + color = SpoonyAndroidTheme.colors.gray500 + )또는 API 추가(옵션):
showLoadingIndicator: Boolean = false, indicatorColor: Color = SpoonyAndroidTheme.colors.gray500app/src/main/java/com/spoony/spoony/presentation/exploreSearch/component/ExploreSearchUserItem.kt (1)
40-47: 아바타(48dp)에서 인디케이터 시각 균형 확인48dp 원형 아바타 내부에 기본 크기 인디케이터는 다소 커 보일 수 있습니다. 필요 시 인디케이터 크기/스트로크를 줄일 수 있도록 SpoonyImage에 옵션을 열어두는 것도 고려해주세요. 현재 구현으로 진행해도 기능상 문제는 없습니다.
app/src/main/java/com/spoony/spoony/presentation/gourmet/map/component/MapPlaceDetailCard.kt (1)
64-73: key 안정성 개선 권장리스트가 변경/재정렬될 수 있다면 key(index) 대신 key(url) 사용이 안정적입니다(중복 가능성 낮다면 권장).
- imageUrlList.forEachIndexed { index, url -> - key(index) { + imageUrlList.forEachIndexed { index, url -> + key(url) { SpoonyImage( model = url, modifier = Modifier .height(103.dp) .weight(1f), showLoadingIndicator = true ) } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/com/spoony/spoony/core/designsystem/component/card/ReviewCard.kt(1 hunks)app/src/main/java/com/spoony/spoony/core/designsystem/component/image/SpoonyImage.kt(3 hunks)app/src/main/java/com/spoony/spoony/presentation/exploreSearch/component/ExploreSearchUserItem.kt(1 hunks)app/src/main/java/com/spoony/spoony/presentation/gourmet/map/component/MapPlaceDetailCard.kt(1 hunks)app/src/main/java/com/spoony/spoony/presentation/placeDetail/component/PlaceDetailImageLazyRow.kt(1 hunks)app/src/main/java/com/spoony/spoony/presentation/register/component/PhotoPicker.kt(1 hunks)
⏰ 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). (1)
- GitHub Check: PR Builder
🔇 Additional comments (2)
app/src/main/java/com/spoony/spoony/core/designsystem/component/card/ReviewCard.kt (1)
168-179: 이미지 접근성 확인 요청리뷰 이미지가 정보 전달 목적이라면 contentDescription=null은 스크린리더에 빈 이미지로 노출됩니다. 장식적 이미지가 아니라면 적절한 설명을 넣는지 검토 부탁드립니다. 장식적 이미지라면 현 상태 유지 OK입니다.
app/src/main/java/com/spoony/spoony/presentation/register/component/PhotoPicker.kt (1)
162-163: 변경사항이 올바르게 적용되었습니다.이미지 로딩 인디케이터 추가라는 PR 목표에 부합하는 변경사항입니다.
ContentScale.Crop은 80dp 크기의 썸네일 이미지를 균일하게 채우기 위한 적절한 선택이며,showLoadingIndicator = true는 사용자가 선택한 이미지의 로딩 상태를 시각적으로 표시합니다.PR 설명에서 언급하신 대로, 로컬 이미지의 경우 빠르게 로드되어 인디케이터가 짧게 보일 수 있지만, 업로드 시나리오에서는 의도한 대로 명확하게 확인될 것입니다.
angryPodo
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.
굿이다 조라
| ) | ||
| } else { | ||
| AsyncImage( | ||
| SubcomposeAsyncImage( |
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.
오..이건 몰랐네...
기본 AsyncImage는 placeholder에 painter만 가능해서 요거로 한건가요???
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.
SubcomposeAsyncImage 넣야지 painter 뿐만 아니라 커스텀한 로딩 UI 를 넣을 수 있어서 저렇게 작업했습니다! 사용하진 않았지만loading 상태, success 상태, error 상태를 분기해서 뷰를 구성할 수도 있다고 합니다.
| Box( | ||
| modifier = Modifier.fillMaxSize(), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| CircularProgressIndicator( | ||
| color = SpoonyAndroidTheme.colors.gray300 | ||
| ) | ||
| } | ||
| } |
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.
이거 컴포넌트화 할래말래ㅋㅋ 그냥 private로 때려도 될거같아요
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.
아 그거도 좋을 듯!
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.
완완
Hyobeen-Park
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.
수고하셨습니다!! 혹시 민재오빠가 리뷰 달아준 인디케이터 분리 하실건가요??
| Box( | ||
| modifier = Modifier.fillMaxSize(), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| CircularProgressIndicator( | ||
| color = SpoonyAndroidTheme.colors.gray300 | ||
| ) | ||
| } |
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.
이거 Box로 꼭 감싸야 하나요?? 인디케이터에도 modifier 넣을 수 있는걸로 알고 있는데 fillMaxSize를 인디케이터에 넣고 box 빼면 원하는대로 안나오려나요?!??
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.
모양은 원하는대로 나올건데 인디케이터 크기를 유지하면서 정중앙에 하려면 저게 맞을거같다는??
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: 0
♻️ Duplicate comments (1)
app/src/main/java/com/spoony/spoony/core/designsystem/component/image/SpoonyImage.kt (1)
56-66: Box 없이 간소화 가능 (wrapContentSize로 중앙 정렬 대체)Box를 제거해도 동일한 결과를 더 간결하게 낼 수 있습니다. 이전 논의와 동일한 주제라 참고 코멘트로 남깁니다.
@Composable private fun LoadingIndicator() { - Box( - modifier = Modifier.fillMaxSize(), - contentAlignment = Alignment.Center - ) { - CircularProgressIndicator( - color = SpoonyAndroidTheme.colors.gray300 - ) - } + CircularProgressIndicator( + modifier = Modifier + .fillMaxSize() + .wrapContentSize(Alignment.Center), + color = SpoonyAndroidTheme.colors.gray300 + ) }wrapContentSize import 필요: androidx.compose.foundation.layout.wrapContentSize
🧹 Nitpick comments (4)
app/src/main/java/com/spoony/spoony/core/designsystem/component/image/SpoonyImage.kt (4)
23-30: API 유연성: Boolean 대신 슬롯 제공 고려호출처마다 다른 로딩 UI가 필요할 수 있어 불린보다 슬롯이 확장성이 좋습니다. 예: loading: (@composable () -> Unit)? = null를 추가하고, null이면 기본 인디케이터를 사용하도록 구성하면 테마/사이즈/콘텐츠 커스터마이징이 용이합니다. KDoc로 contentDescription 의미(장식 이미지면 null 권장)도 함께 명시해 주세요.
39-53: 에러 상태 미처리: 실패 시 화면이 비어 보일 수 있음SubcomposeAsyncImage에서 error 슬롯을 지정하지 않으면 실패 시 아무것도 렌더링되지 않을 수 있습니다. 간단한 실패 플레이스홀더(예: 런처 배경 아이콘)라도 노출해 주세요.
SubcomposeAsyncImage( model = model, contentDescription = contentDescription, contentScale = contentScale, modifier = modifier.clip(shape), loading = { if (showLoadingIndicator) { LoadingIndicator() } }, + error = { + Image( + imageVector = ImageVector.vectorResource(R.drawable.ic_spoony_launcher_background), + contentDescription = contentDescription, + contentScale = contentScale, + modifier = Modifier.fillMaxSize() + ) + }, success = { SubcomposeAsyncImageContent() } )
39-53: 인디케이터 미사용 시 Subcompose 오버헤드 회피showLoadingIndicator = false인 경우에는 AsyncImage로 분기하면 컴포지션/측정 오버헤드를 줄일 수 있습니다.
- SubcomposeAsyncImage( - model = model, - contentDescription = contentDescription, - contentScale = contentScale, - modifier = modifier.clip(shape), - loading = { - if (showLoadingIndicator) { - LoadingIndicator() - } - }, - success = { - SubcomposeAsyncImageContent() - } - ) + if (showLoadingIndicator) { + SubcomposeAsyncImage( + model = model, + contentDescription = contentDescription, + contentScale = contentScale, + modifier = modifier.clip(shape), + loading = { LoadingIndicator() }, + success = { SubcomposeAsyncImageContent() } + ) + } else { + coil3.compose.AsyncImage( + model = model, + contentDescription = contentDescription, + contentScale = contentScale, + modifier = modifier.clip(shape) + ) + }추가: AsyncImage import 필요.
44-48: 빠른 로드 깜빡임(flicker) 방지: 지연 표시 적용설명에 적어주신 “빠르게 로드되면 확인 어려움”을 줄이려면 인디케이터를 120–200ms 지연 후 표시하세요.
- if (showLoadingIndicator) { - LoadingIndicator() - } + if (showLoadingIndicator && showSpinner) { + LoadingIndicator() + }함수 상단에 상태/효과 추가:
// SubcomposeAsyncImage 호출부 위 var showSpinner by remember(model) { mutableStateOf(false) } LaunchedEffect(model) { showSpinner = false kotlinx.coroutines.delay(150) // 필요 시 파라미터화 showSpinner = true }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/spoony/spoony/core/designsystem/component/image/SpoonyImage.kt(3 hunks)
⏰ 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). (1)
- GitHub Check: PR Builder
🔇 Additional comments (1)
app/src/main/java/com/spoony/spoony/core/designsystem/component/image/SpoonyImage.kt (1)
63-64: 로딩 인디케이터 색상 대비 확인 요청gray300이 라이트/다크 테마에서 배경 대비가 충분한지 확인 부탁드립니다. 접근성 요구사항이 있다면 토큰(예: onSurfaceVariant) 기반으로 재정의하는 옵션도 고려해 주세요.
Related issue 🛠
Work Description ✏️
Screenshot 📸
2025-10-10.7.41.26.mov
To Reviewers 📢
따로 디자인 없고 자체적으로 하기로 했다고 디자인쪽에서 전달 받아서 색상만 회색으로 변경해서 작업했습니다. 다른 곳은 이미지가 빠르게 들어와서 확인하기 어려우실 수 있는데 이미지 업로드 할때는 확인하기 편하더라구요. 위 영상 한번 보시고 좋은 의견 주시면 수정하겠습니다.
Summary by CodeRabbit