Conversation
Walkthrough하단 탭 재선택 트리거(Feed 리프레시/스크롤-투-탑) 추가, 이미지 업로드를 S3 presigned URL 방식으로 전환, 여러 화면/뷰모델에 커서 기반 페이징·무한 스크롤 도입, 폼 컴포넌트 리팩터(Outlined→Basic) 및 대문자 차단 옵션 추가 등 광범위한 API·UI·리포지토리 변경이 적용됨. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BottomBar
participant MainScreen
participant NavHost
participant FeedScreen
participant FeedVM
User->>BottomBar: Feed 탭 재선택
BottomBar-->>MainScreen: onTabReselected(MainTabRoutes.Feed)
MainScreen-->>NavHost: onFeedTabReselected++ (Int)
NavHost-->>FeedScreen: onFeedTabReselected (Int)
FeedScreen->>FeedVM: refreshOnBottomNavReselect()
FeedScreen->>FeedScreen: 스크롤-투-탑 실행
FeedVM-->>FeedScreen: 업데이트된 피드 데이터
sequenceDiagram
participant UI as FeedWriteUI
participant VM as FeedWriteViewModel
participant Repo as FeedRepository
participant Svc as FeedService
participant Helper as ImageUploadHelper
UI->>VM: createFeed(..., imageUris)
VM->>Repo: createFeed 요청 전달
alt imageUris 존재
Repo->>Helper: getImageMetadata(uri) for each
Repo->>Svc: getPresignedUrls(metadata list)
loop for each image
Repo->>Helper: uploadImageToS3(uri, presignedUrl)
end
Repo->>Svc: createFeed(@Body CreateFeedRequest(imageUrls))
else 이미지 없음
Repo->>Svc: createFeed(@Body CreateFeedRequest(imageUrls = []))
end
Svc-->>Repo: CreateFeedResponse
Repo-->>VM: 결과 반환
VM-->>UI: 성공/실패 처리
sequenceDiagram
participant UI as ListUI
participant VM as AnyViewModel
participant Repo as Repo
participant Svc as Service
UI->>VM: 스크롤 근접(onScrollNearEnd)
alt VM.canLoadMore && not VM.isLast
VM->>Repo: loadMore(cursor = nextCursor)
Repo->>Svc: API(cursor)
Svc-->>Repo: items + nextCursor + isLast
Repo-->>VM: append items, update cursor/isLast
VM-->>UI: 추가된 아이템 렌더
else
UI-->>UI: noop
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (4)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (1)
304-310: 경합 버그:refreshData()가isRefreshing을 설정하지 않아 무한스크롤(fetchMore)와 동시 실행 위험
canLoadMoreCurrentTab은isRefreshing을 체크하지만,refreshData()는 이를 토글하지 않습니다. 결과적으로 reselection에 따른 전체 갱신과 페이징 로드가 동시에 실행되어 중복/뒤섞인 목록 갱신이 발생할 수 있습니다.최소 수정으로 경합을 차단하려면 아래처럼 UI 상태의
isRefreshing을 토글하세요(오버레이가 보이는 것이 부담이면 별도 플래그를 도입해canLoadMore*계산식에만 반영하는 대안도 가능).fun refreshData() { viewModelScope.launch { - refreshAllFeeds() - refreshMyFeeds() - fetchRecentWriters() + updateState { it.copy(isRefreshing = true) } + try { + refreshAllFeeds() + refreshMyFeeds() + fetchRecentWriters() + } finally { + updateState { it.copy(isRefreshing = false) } + } } }원하시면
isGlobalRefreshing같은 별도 플래그로 오버레이 표시는 막고, 로드모어 차단에만 쓰도록 리팩터 제안 드리겠습니다.app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (3)
125-134: 메모리 누수 위험:observeForever사용
SavedStateHandle.getLiveData(...).observeForever { ... }는 해제되지 않아 수명 누수 소지가 큽니다. Flow로 전환하여 수명 안전하게 수집하세요.권장 패치:
LaunchedEffect(Unit) { navController.currentBackStackEntry?.savedStateHandle?.let { handle -> - handle.getLiveData<Long>("deleted_feed_id").observeForever { deletedId -> - if (deletedId != null) { - feedViewModel.removeDeletedFeed(deletedId) - handle.remove<Long>("deleted_feed_id") - } - } + handle.getStateFlow<Long?>("deleted_feed_id", null).collect { deletedId -> + if (deletedId != null) { + feedViewModel.removeDeletedFeed(deletedId) + handle.remove<Long>("deleted_feed_id") + } + } } }
145-169: 초기 진입 시 이중 새로고침 가능성같은
LaunchedEffect(Unit)내에서 (1) 목록이 비면refreshData()호출 후, (2)hasUpdatedFeedData/fromProfile가 false면 다시refreshData()를 호출합니다. 초기 네트워크 호출이 중복될 수 있습니다. 한 번만 호출되도록 조건을 통합하세요.간단 대안:
- if (feedUiState.allFeeds.isEmpty() && feedUiState.myFeeds.isEmpty()) { - feedViewModel.refreshData() - } ... - if (!hasUpdatedFeedData && !fromProfile) { - feedViewModel.refreshData() - allFeedListState.scrollToItem(0) - } else { + val needInitialRefresh = (feedUiState.allFeeds.isEmpty() && feedUiState.myFeeds.isEmpty()) && + !hasUpdatedFeedData && !fromProfile + if (needInitialRefresh) { + feedViewModel.refreshData() + allFeedListState.scrollToItem(0) + } else { // 댓글 화면 또는 프로필에서 돌아온 경우: recentWriters만 업데이트 feedViewModel.fetchRecentWriters() }
225-252: 메모리 누수 위험(2):updated_feed_id도observeForever사용위와 동일한 이유로 Flow로 전환하세요.
권장 패치:
LaunchedEffect(Unit) { //커스텀객체 타입 인식오류 -> 직렬화가 아닌 잘게 쪼개어 전달 navController.currentBackStackEntry?.savedStateHandle?.let { handle -> - handle.getLiveData<Long>("updated_feed_id").observeForever { feedId -> - if (feedId != null) { + handle.getStateFlow<Long?>("updated_feed_id", null).collect { feedId -> + if (feedId != null) { val isLiked = handle.get<Boolean>("updated_feed_isLiked") ?: false val likeCount = handle.get<Int>("updated_feed_likeCount") ?: 0 val isSaved = handle.get<Boolean>("updated_feed_isSaved") ?: false val commentCount = handle.get<Int>("updated_feed_commentCount") ?: 0 val result = FeedStateUpdateResult( feedId = feedId, isLiked = isLiked, likeCount = likeCount, isSaved = isSaved, commentCount = commentCount ) feedViewModel.updateFeedStateFromResult(result) handle.remove<Long>("updated_feed_id") handle.remove<Boolean>("updated_feed_isLiked") handle.remove<Int>("updated_feed_likeCount") handle.remove<Boolean>("updated_feed_isSaved") handle.remove<Int>("updated_feed_commentCount") - } - } + } + } } }
🧹 Nitpick comments (8)
app/src/main/java/com/texthip/thip/ui/navigator/BottomNavigationBar.kt (1)
126-128: 재선택 시 콜백 호출 처리 적절 + 경미한 디바운스/스테일 참조 보호 제안빠른 다중 탭(더블탭) 방어와 스테일 람다 캡처 방지를 위해
rememberUpdatedState로 최신 콜백을 참조하는 것을 권장합니다.적용 예시(요지):
@Composable fun BottomNavigationBar( navController: NavHostController, - onTabReselected: ((MainTabRoutes) -> Unit)? = null + onTabReselected: ((MainTabRoutes) -> Unit)? = null ) { + val updatedOnTabReselected by rememberUpdatedState(onTabReselected) ... - if (isSelected) { - onTabReselected?.invoke(item.route) + if (isSelected) { + updatedOnTabReselected?.invoke(item.route) } else { navController.navigateToTab(item.route) }app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (2)
77-105: 탭 재선택 분기 로직이 Screen과 ViewModel에 중복됨
FeedScreen의onTabSelected와 본 메서드가 모두 “같은 탭 재클릭 vs 탭 전환”을 판단합니다. 이벤트 중복 처리로 불필요한 새로고침이 2회 발생할 수 있습니다. 한 곳(가능하면 ViewModel)으로 단일화하세요.
107-115: reselect 처리 메서드 분리 자체는 좋으나, 실제 스크롤은 Screen에서만 수행역할 분리가 명확해 가독성은 좋습니다. 다만 데이터 새로고침은 전체 탭(전체/내피드)을 모두 갱신하는
refreshData()호출이라 비용이 큽니다. 재선택 시 현재 탭만 갱신하는 경량 경로(예:refreshCurrentTab()+ writers 업데이트)도 고려해보세요.app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
285-296: 탭 컴포넌트의 재클릭 처리와 ViewModel의 재선택 처리 중복여기서도 “같은 탭 재클릭”을 판별하고, ViewModel도 동일 판별을 수행합니다. 한 곳으로 책임을 모으면(권장: ViewModel) 이벤트 중복·불필요 새로고침을 줄일 수 있습니다.
app/src/main/java/com/texthip/thip/MainScreen.kt (4)
8-10: primitive State 최적화: mutableIntStateOf로 교체 권장Int 상태에 boxed Integer를 쓰는 오버헤드를 줄일 수 있습니다.
-import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.mutableIntStateOf
27-27: 이벤트 키 상태를 primitive로 관리recomposition 비용을 줄이기 위해 Int 전용 state 사용을 권장합니다.
- var feedReselectionTrigger by remember { mutableStateOf(0) } + var feedReselectionTrigger by remember { mutableIntStateOf(0) }
33-47: 탭 재선택 트리거 로직은 적절함 + 람다 안정화/연타 완화 고려현재 구현은 OK. 다만 람다를 remember로 한 번만 생성하면 할당을 줄일 수 있습니다. 또한 빠른 연타로 과도한 트리거가 발생할 수 있으니(스크롤/리프레시 중복) ViewModel 레벨에서 debounce/throttle 고려 바랍니다.
- onTabReselected = { route -> + onTabReselected = remember { + { route -> when (route) { MainTabRoutes.Feed -> { feedReselectionTrigger += 1 } else -> { // 다른 탭들은 향후 확장 가능 } } - } - } + } + }
54-55: 파라미터 명 변경:onFeedTabReselected→feedTabReselectKeyMainNavHost부터 FeedNavigation, FeedScreen, 그리고 MainScreen 호출부까지 일관되게 이름을 바꿔 키 의미가 명확해집니다. 아래 위치들을 리팩토링해 주세요:
app/src/main/java/com/texthip/thip/MainScreen.kt
MainNavHost( navController = navController, onNavigateToLogin = onNavigateToLogin, - onFeedTabReselected = feedReselectionTrigger + feedTabReselectKey = feedReselectionTrigger )app/src/main/java/com/texthip/thip/ui/navigator/MainNavHost.kt
@Composable fun MainNavHost( navController: NavHostController, onNavigateToLogin: () -> Unit, - onFeedTabReselected: Int = 0 + feedTabReselectKey : Int = 0 ) { NavHost( /* … */ - composable(...) { _, backStackEntry -> - FeedNavigation( - navController = navController, - navigateBack = navController::popBackStack, - onFeedTabReselected = onFeedTabReselected, + composable(...) { _, backStackEntry -> + FeedNavigation( + navController = navController, + navigateBack = navController::popBackStack, + feedTabReselectKey = feedTabReselectKey, /* … */app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt
@Composable fun FeedNavigation( navController: NavHostController, navigateBack: () -> Unit, - onFeedTabReselected: Int = 0 + feedTabReselectKey : Int = 0 ) { composable(MainTabRoutes.Feed) { backStackEntry -> FeedScreen( /* … */, - onFeedTabReselected = onFeedTabReselected, + feedTabReselectKey = feedTabReselectKey, onResultConsumed = { /* … */ } ) }app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt
@Composable fun FeedScreen( onNavigateToNotification: () -> Unit = {}, refreshFeed: Boolean? = null, - onFeedTabReselected: Int = 0, + feedTabReselectKey: Int = 0, // 바텀 네비게이션 재선택 트리거 onNavigateToOthersSubscription: (Long) -> Unit = {}, onResultConsumed: () -> Unit = {}, ) { val currentListState = rememberLazyListState() - LaunchedEffect(onFeedTabReselected) { - if (onFeedTabReselected > 0) { + LaunchedEffect(feedTabReselectKey) { + if (feedTabReselectKey > 0) { feedViewModel.refreshOnBottomNavReselect() currentListState.scrollToItem(0) } }이렇게 변경하면 파라미터가 콜백이 아닌 “키” 값임이 명확해지고,
LaunchedEffect(feedTabReselectKey)로도 일관되게 반응합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
app/src/main/java/com/texthip/thip/MainScreen.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/BottomNavigationBar.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/navigator/MainNavHost.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
app/src/main/java/com/texthip/thip/ui/navigator/MainNavHost.kt (1)
app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt (1)
feedNavigation(33-253)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (3)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedDetailViewModel.kt (1)
updateState(40-42)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(33-35)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt (1)
updateState(35-37)
app/src/main/java/com/texthip/thip/MainScreen.kt (2)
app/src/main/java/com/texthip/thip/ui/navigator/BottomNavigationBar.kt (1)
BottomNavigationBar(39-143)app/src/main/java/com/texthip/thip/ui/navigator/MainNavHost.kt (1)
MainNavHost(14-43)
🔇 Additional comments (7)
app/src/main/java/com/texthip/thip/ui/navigator/BottomNavigationBar.kt (1)
40-43: 탭 재선택 콜백 도입: API 확장 안전합니다기본값을
null로 두어 기존 호출처는 영향 없고, 콜백만 선택적으로 제공하면 됩니다. 전체 구조 👍app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
219-224: 바텀 탭 재선택 처리 흐름 OK (스크롤 상단 + 데이터 갱신)동작은 적절합니다. 다만 ViewModel의
refreshData()경합 이슈(무한스크롤 동시 실행)는 앞선 제안 반영이 필요합니다.재선택 직후 목록 하단 근접 상태에서 로드모어가 트리거되지 않는지 실제 기기에서 확인 부탁드립니다.
app/src/main/java/com/texthip/thip/ui/navigator/MainNavHost.kt (2)
26-28: feedNavigation로의 전달 정확
navigateBack = navController::popBackStack와 함께onFeedTabReselected가 정상 전달됩니다.
17-19: MainNavHost 호출 검증 완료
onFeedTabReselected: Int = 0기본값 덕분에 기존 호출에 영향이 없습니다.
검토 결과, app/src/main/java/com/texthip/thip/MainScreen.kt에서만 호출되며 추가 인자를 전달하지 않아도 정상 동작합니다.app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt (2)
33-37: feedNavigation 시그니처에 reselection 트리거 추가: OK그래프 수준에서 신호를 주입하는 방식이 단순하고 Compose 백스택 스코프와도 잘 맞습니다.
61-61: FeedScreen으로의 트리거 전달 적절
FeedScreen이LaunchedEffect(onFeedTabReselected)로 반응하므로 의도대로 동작합니다.app/src/main/java/com/texthip/thip/MainScreen.kt (1)
18-18: Route 의존성 추가 적절함탭 식별을 MainTabRoutes로 일관되게 가져오는 방향 좋습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (2)
128-137: observeForever 사용으로 인한 메모리 누수 위험.SavedStateHandle LiveData를 observeForever로 구독하고 해제하지 않아, 화면 재생성 시 누수/중복 처리 가능성이 큽니다. 수명 주기 인지 관찰로 교체하세요.
적용 diff(간단 교체안):
- LaunchedEffect(Unit) { - navController.currentBackStackEntry?.savedStateHandle?.let { handle -> - handle.getLiveData<Long>("deleted_feed_id").observeForever { deletedId -> - if (deletedId != null) { - feedViewModel.removeDeletedFeed(deletedId) - handle.remove<Long>("deleted_feed_id") - } - } - } - } + val lifecycleOwner = androidx.compose.ui.platform.LocalLifecycleOwner.current + androidx.compose.runtime.DisposableEffect(navController, lifecycleOwner) { + val handle = navController.currentBackStackEntry?.savedStateHandle + val liveData = handle?.getLiveData<Long>("deleted_feed_id") + val observer = androidx.lifecycle.Observer<Long> { deletedId -> + if (deletedId != null) { + feedViewModel.removeDeletedFeed(deletedId) + handle?.remove<Long>("deleted_feed_id") + } + } + liveData?.observe(lifecycleOwner, observer) + onDispose { liveData?.removeObserver(observer) } + }추가로 아래 블록(업데이트 결과 수신)에도 동일 적용이 필요합니다(라인 228-255).
228-255: observeForever 제거(업데이트 결과 수신 블록).위와 동일한 누수 위험. lifecycleOwner 기반으로 교체하십시오.
- LaunchedEffect(Unit) { //커스텀객체 타입 인식오류 -> 직렬화가 아닌 잘게 쪼개어 전달 - navController.currentBackStackEntry?.savedStateHandle?.let { handle -> - handle.getLiveData<Long>("updated_feed_id").observeForever { feedId -> - if (feedId != null) { - val isLiked = handle.get<Boolean>("updated_feed_isLiked") ?: false - val likeCount = handle.get<Int>("updated_feed_likeCount") ?: 0 - val isSaved = handle.get<Boolean>("updated_feed_isSaved") ?: false - val commentCount = handle.get<Int>("updated_feed_commentCount") ?: 0 - val result = FeedStateUpdateResult( - feedId = feedId, isLiked = isLiked, likeCount = likeCount, - isSaved = isSaved, commentCount = commentCount - ) - feedViewModel.updateFeedStateFromResult(result) - handle.remove<Long>("updated_feed_id") - handle.remove<Boolean>("updated_feed_isLiked") - handle.remove<Int>("updated_feed_likeCount") - handle.remove<Boolean>("updated_feed_isSaved") - handle.remove<Int>("updated_feed_commentCount") - } - } - } - } + val lifecycleOwner = androidx.compose.ui.platform.LocalLifecycleOwner.current + androidx.compose.runtime.DisposableEffect(navController, lifecycleOwner) { + val handle = navController.currentBackStackEntry?.savedStateHandle + val liveData = handle?.getLiveData<Long>("updated_feed_id") + val observer = androidx.lifecycle.Observer<Long> { feedId -> + if (feedId != null && handle != null) { + val isLiked = handle.get<Boolean>("updated_feed_isLiked") ?: false + val likeCount = handle.get<Int>("updated_feed_likeCount") ?: 0 + val isSaved = handle.get<Boolean>("updated_feed_isSaved") ?: false + val commentCount = handle.get<Int>("updated_feed_commentCount") ?: 0 + val result = FeedStateUpdateResult(feedId, isLiked, likeCount, isSaved, commentCount) + feedViewModel.updateFeedStateFromResult(result) + handle.remove<Long>("updated_feed_id") + handle.remove<Boolean>("updated_feed_isLiked") + handle.remove<Int>("updated_feed_likeCount") + handle.remove<Boolean>("updated_feed_isSaved") + handle.remove<Int>("updated_feed_commentCount") + } + } + liveData?.observe(lifecycleOwner, observer) + onDispose { liveData?.removeObserver(observer) } + }app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (2)
421-431: 댓글 롱프레스에서 non-null 단정(!!)로 NPE/크래시 위험임시/작성중 댓글 등으로 commentId가 null일 수 있습니다. 안전하게 처리해주세요.
- onCommentLongPress = { comment -> - selectedCommentForMenu = CommentActionTarget(comment.commentId!!, comment.isWriter) - isCommentMenuVisible = true - }, + onCommentLongPress = { comment -> + comment.commentId?.let { id -> + selectedCommentForMenu = CommentActionTarget(id, comment.isWriter) + isCommentMenuVisible = true + } + },
563-568: Dialog 오버레이가 터치를 소모하지 않음 → 배경 클릭 전파 가능clickable(enabled = false)는 포인터를 소비하지 않습니다. 배경 클릭이 아래 UI로 전파될 수 있어 치명적입니다. 스크림에서 터치를 소비하도록 바꿔주세요.
- Box( - Modifier - .fillMaxSize() - .clickable(enabled = false, onClick = {}) - ) { + Box( + Modifier + .fillMaxSize() + .clickable( + indication = null, + interactionSource = remember { MutableInteractionSource() } + ) { /* consume to block underlying clicks */ } + ) {추가 import:
import androidx.compose.foundation.interaction.MutableInteractionSource
🧹 Nitpick comments (27)
app/src/main/java/com/texthip/thip/ui/signin/screen/SplashScreen.kt (3)
56-58: 중복 배경 제거로 오버드로우 방지.ThipTheme가 이미 전역 Black 배경을 깔고 있어 Column의
.background(colors.Black)는 중복입니다. 아래처럼 제거해 오버드로우를 줄이는 것을 권장합니다.- Modifier - .background(colors.Black) - .fillMaxSize(), + Modifier + .fillMaxSize(),
69-74: 하드코딩 색상(Purple) 대신 테마 토큰 사용 권장.브랜드 보랏빛을 직접 import해 쓰면 테마 전환(다크/하이콘트라스트) 대응이 어려워집니다. Material3 또는 ThipTheme의 컬러 토큰으로 치환을 제안합니다.
- withStyle(style = SpanStyle(color = Purple)) { + withStyle(style = SpanStyle(color = androidx.compose.material3.MaterialTheme.colorScheme.primary)) {추가로 상단 import에서
com.texthip.thip.ui.theme.Purple제거,import androidx.compose.material3.MaterialTheme추가를 고려해 주세요.
62-66: 스플래시 로고 접근성 확인.
contentDescription = null은 장식용일 때 OK입니다. 만약 앱 브랜드 소개 목적이라면 접근성 문자열 리소스를 제공해 주세요.예:
- Icon( + Icon( painter = painterResource(R.drawable.ic_logo), - contentDescription = null, + contentDescription = stringResource(R.string.splash_logo_desc), tint = Unspecified, )app/src/main/java/com/texthip/thip/ui/navigator/navigations/AuthNavigation.kt (1)
71-76: 중괄호 배치/여백 정리로 가독성 개선.동일 라인에
}가 중첩되어 읽기 어렵습니다. 아래처럼 정리하면 가독성이 좋아집니다(동작 동일).- TutorialScreen( - onFinish = { - navController.navigate(CommonRoutes.SignupScreenRoutes.Done) { - popUpTo<CommonRoutes.SignupScreenRoutes.Tutorial> { inclusive = true } - } } - ) + TutorialScreen( + onFinish = { + navController.navigate(CommonRoutes.SignupScreenRoutes.Done) { + popUpTo<CommonRoutes.SignupScreenRoutes.Tutorial> { inclusive = true } + } + } + )app/src/main/java/com/texthip/thip/ui/group/search/screen/GroupSearchScreen.kt (3)
119-127: onSearch 인자 활용으로 스테일 상태 방지.IME 검색 액션에서 전달되는 문자열을 바로 반영 후 검색을 트리거하면, 텍스트 동기화 지연에 의한 엣지 케이스를 줄일 수 있습니다.
- onSearch = { _ -> onSearchButtonClick() } + onSearch = { query -> + onUpdateSearchQuery(query) + onSearchButtonClick() + }
171-186: 선택 인덱스 중복 계산 제거.람다 내부에서 현재 선택 인덱스를 다시 계산하지 말고, 상단에서 계산한 selectedGenreIndex를 재사용하세요. 가독성과 미세 성능 개선에 도움됩니다.
- onGenreSelect = { index -> - val currentSelectedIndex = if (uiState.selectedGenre != null) { - uiState.genres.indexOf(uiState.selectedGenre) - } else -1 - val selectedGenre = if (index == currentSelectedIndex) { + onGenreSelect = { index -> + val selectedGenre = if (index == selectedGenreIndex) { null } else if (index >= 0 && index < uiState.genres.size) { uiState.genres[index] } else { null } onUpdateSelectedGenre(selectedGenre) }
199-215: 정렬 타입을 문자열 리터럴 대신 타입세이프하게."deadline"/"memberCount" 하드코딩은 오타에 취약합니다. enum/sealed class 또는 Map<String, SortType>으로 매핑하세요. 또한 VM의 updateSortType이 동일한 키를 기대하는지 확인이 필요합니다.
VM에서 기대하는 정렬 키가 "deadline"/"memberCount"가 맞는지 확인 부탁드립니다. 불일치 시 정렬이 적용되지 않을 수 있습니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
343-345: 불필요한 지역 변수 제거.isCurrentTab를 계산하지만 사용하지 않습니다. 제거하여 간결화하세요.
- onTabSelected = { index -> - val isCurrentTab = feedUiState.selectedTabIndex == index - onTabSelected(index) - } + onTabSelected = onTabSelectedapp/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomUnlockScreen.kt (2)
100-127: 백스페이스 UX 보완 제안.현재 onBackspace는 이전 칸으로만 이동합니다. 일반적으로 이전 칸 값도 함께 비워주는 편이 자연스럽습니다.
- onBackspace = { index -> - // 빈 박스에서 백스페이스 → 이전 박스로 이동 - if (index > 0) { - val prevIndex = index - 1 - focusRequesters[prevIndex].requestFocus() - } - } + onBackspace = { index -> + if (index > 0) { + val prevIndex = index - 1 + val newPassword = password.copyOf() + newPassword[prevIndex] = "" + password = newPassword + focusRequesters[prevIndex].requestFocus() + } + }
55-60: 입력 완료 조건 검증 적절.숫자 4자리 모두 1글자인지 확인 후 요청하는 로직이 명확합니다. 다만 회전 등 구성 변경에서 상태 유지를 원하면 rememberSaveable 도입을 고려하세요.
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageSaveScreen.kt (5)
36-37: mock 패키지 의존 노출 최소화
mock.BookItem,mock.FeedItem을 프로덕션 화면 컴포저블의 파라미터/타입으로 직접 노출하면 설계가 취약해집니다. UI 모델 전용 패키지(예:ui.mypage.model)로 이동하거나 네이밍에서mock을 제거해 주세요. 미리보기 전용이라면@Preview범위로만 한정하는 것도 방법입니다.
78-89: 뷰모델 nullable 설계 대신 액션 람다로 디커플링 검토미리보기 대응을 위해 VM을 nullable로 두고 본문에서 렌더링을 건너뛰면, 프리뷰가 비어 보이는 문제가 생깁니다.
MypageSaveContent는 리스트/콜백(북마크/좋아요/클릭)만 받는 “프레젠테이션”으로 두고, VM 의존은 상위에서 람다로 주입하는 방식으로 디커플링을 권장합니다. 이렇게 하면 프리뷰에서는 no-op 람다를 넘겨 실제 목록 미리보기가 가능합니다.
90-91: tabs 중복 정의 제거동일한 탭 문자열 리스트가
MypageSaveScreen(Line 53)과MypageSaveContent(여기)에서 중복 정의됩니다. 한 곳으로 일원화해 주세요. 권장: Content 내부 정의 유지 → Screen 쪽 선언 제거.
110-127: 테마 일관성: White 상수 대신 theme color 사용인디케이터 배경에
White상수를 직접 사용하고 있습니다. 테마 일관성과 다크모드 대응을 위해colors.White로 통일하세요.다음 패치 제안:
- .background(White) + .background(colors.White)
154-160: VM null 시 콘텐츠가 렌더링되지 않음 (프리뷰 가독성 저하)현재 VM이 null이면(프리뷰 케이스) 리스트가 전혀 표시되지 않습니다. 프리뷰/디자인 검증을 위해 VM이 없을 때는 읽기전용 리스트(토글 동작 없음)라도 렌더링하는 폴백을 권장합니다. 예: VM 존재 시
FeedContent/BookContent, 부재 시 간단한LazyColumn버전으로 대체.원하시면 폴백용
FeedListPreview/BookListPreview컴포저블 초안을 드리겠습니다.Also applies to: 162-168
app/src/main/java/com/texthip/thip/ui/common/forms/FormTextFieldDefault.kt (5)
63-66: 클리어 아이콘과 글자수 카운터가 텍스트와 겹칠 수 있음 — 끝쪽 패딩을 동적으로 확보해주세요.showIcon=true 또는 showLimit=true일 때 텍스트가 아이콘/카운터 아래로 흘러들어가 시각적 충돌 가능성이 있습니다. 텍스트 영역의 end 패딩을 상황에 따라 늘려 공간을 확보하는 편이 안전합니다.
아래와 같이 end 패딩을 조건부로 계산해 적용해보세요.
@@ - .padding(horizontal = 14.dp, vertical = 12.dp), + .padding( + start = 14.dp, + end = + when { + showIcon && showLimit -> 72.dp // 아이콘(약 40dp) + 카운터 여유 + showIcon || showLimit -> 40.dp + else -> 14.dp + }, + top = 12.dp, + bottom = 12.dp + ),Also applies to: 80-97, 102-115
25-27: 접근성/현지화: contentDescription 현지화 및 장식용 아이콘은 null 처리클릭 가능한 클리어 아이콘은 현지화된 라벨을 사용하고, 비활성(장식용) 아이콘은 contentDescription=null로 보조기기에 노출되지 않도록 하는 것이 좋습니다.
@@ -import androidx.compose.ui.res.painterResource +import androidx.compose.ui.res.painterResource +import androidx.compose.ui.res.stringResource @@ - contentDescription = "Clear text", + contentDescription = stringResource(R.string.a11y_clear_text), @@ - contentDescription = "Clear text", + contentDescription = null,참고: 문자열 리소스 R.string.a11y_clear_text 추가가 필요합니다.
Also applies to: 82-89, 91-96
42-47: 상태 상승(state hoisting) 필요 여부 확인이 컴포저블이 내부 상태(text)를 소유합니다. 폼 입력값을 화면/VM에서 관리해야 한다면 value/onValueChange 패턴으로 API를 확장하는 것이 바람직합니다.
원하시면 외부 제어형 API로의 리팩터를 제안드릴게요.
Also applies to: 50-56
45-47: 불필요한 displayText 재계산 최소화onValueChange에서 이미 길이 제한을 강제하므로 displayText는 항상 text와 동일합니다. 단순화를 위해 displayText 없이 text를 직접 value에 바인딩할 수 있습니다.
- val displayText = if (showLimit && text.length > limit) text.substring(0, limit) else text + val displayText = text @@ - value = displayText, + value = text,Also applies to: 51-55
82-89: Icon 대신 IconButton 사용 고려터치 타겟, 포커스, 접근성에서 IconButton이 더 안전합니다. 클릭 가능한 아이콘은 IconButton으로 감싸는 것을 권장합니다.
- Icon( - painter = painterResource(id = R.drawable.ic_x_circle_white), - contentDescription = stringResource(R.string.a11y_clear_text), - modifier = Modifier - .align(Alignment.CenterEnd) - .clickable { text = "" }, - tint = Color.Unspecified - ) + androidx.compose.material3.IconButton( + modifier = Modifier.align(Alignment.CenterEnd), + onClick = { text = "" } + ) { + Icon( + painter = painterResource(id = R.drawable.ic_x_circle_white), + contentDescription = stringResource(R.string.a11y_clear_text), + tint = Color.Unspecified + ) + }app/src/main/java/com/texthip/thip/ui/common/forms/WarningTextField.kt (3)
70-82: 아이콘/글자수 카운터와 텍스트 겹침 방지 — end 패딩 확보FormTextFieldDefault와 동일하게 끝쪽 패딩을 조건부로 늘려 겹침을 방지하세요.
@@ - .padding(horizontal = 14.dp, vertical = 12.dp), + .padding( + start = 14.dp, + end = + when { + showIcon && (showLimit && maxLength != Int.MAX_VALUE) -> 72.dp + showIcon || (showLimit && maxLength != Int.MAX_VALUE) -> 40.dp + else -> 14.dp + }, + top = 12.dp, + bottom = 12.dp + ),Also applies to: 113-124, 99-108
100-107: 접근성/현지화: 클리어 아이콘 contentDescription 현지화사용자 언어에 맞는 라벨을 사용해주세요.
- Icon( + Icon( painter = painterResource(id = R.drawable.ic_x_circle_grey), - contentDescription = "Clear text", + contentDescription = stringResource(R.string.a11y_clear_text), modifier = Modifier .align(Alignment.CenterEnd) .clickable { onValueChange("") }, tint = Color.Unspecified )strings.xml에 a11y 문자열 추가가 필요합니다.
60-61: IconButton 사용 고려클릭 타겟 확보 및 포커스/리플 효과를 위해 Icon 대신 IconButton을 권장합니다.
- if (showIcon) { - Icon( + if (showIcon) { + androidx.compose.material3.IconButton( + modifier = Modifier.align(Alignment.CenterEnd), + onClick = { onValueChange("") } + ) { + Icon( painter = painterResource(id = R.drawable.ic_x_circle_grey), - contentDescription = stringResource(R.string.a11y_clear_text), - modifier = Modifier - .align(Alignment.CenterEnd) - .clickable { onValueChange("") }, - tint = Color.Unspecified - ) + contentDescription = stringResource(R.string.a11y_clear_text), + tint = Color.Unspecified + ) + }Also applies to: 83-86
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (4)
147-160: 매개변수 타입의 FQCN 제거로 가독성 개선이미 같은 타입을 import하고 있으니 FQCN을 제거해 읽기 쉽게 합시다.
- feedDetailUiState: com.texthip.thip.ui.feed.viewmodel.FeedDetailUiState, - commentsUiState: com.texthip.thip.ui.group.note.viewmodel.CommentsUiState, + feedDetailUiState: FeedDetailUiState, + commentsUiState: CommentsUiState,
447-456: 댓글 전송 즉시 입력 필드 초기화 → 실패 시 입력 손실전송 실패(네트워크/서버 오류)에도 입력이 비워집니다. 성공 시점에만 비우도록 이동하는 게 안전합니다.
- commentInput = "" - replyingToCommentId = null - replyingToNickname = null - focusManager.clearFocus()그리고 성공 플래그에 반응해 클리어하는 이펙트를 추가하세요(아래 패치 참고).
@@ LaunchedEffect(showToast) { if (showToast) { delay(3000) showToast = false } } + // 댓글 생성 성공 시 입력 초기화 + LaunchedEffect(commentsUiState.isCommentCreated) { + if (commentsUiState.isCommentCreated) { + commentInput = "" + replyingToCommentId = null + replyingToNickname = null + focusManager.clearFocus() + } + }
121-122: 문자열 상수 postType = "FEED" → 타입 안전성 제고 제안매직 스트링 대신 enum/sealed class 사용을 권장합니다. (예: PostType.FEED.name)
515-519: 하드코딩된 토스트 메시지의 국제화(I18N) 제안문자열 리소스로 이전하면 번역과 일관성 관리가 쉬워집니다.
Also applies to: 549-552
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
app/src/main/java/com/texthip/thip/ui/common/forms/FormTextFieldDefault.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/common/forms/WarningTextField.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt(11 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(14 hunks)app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomUnlockScreen.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/group/search/screen/GroupSearchScreen.kt(8 hunks)app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageEditScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageSaveScreen.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/AuthNavigation.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/signin/screen/SignupNicknameScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/signin/screen/SplashScreen.kt(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
app/src/main/java/com/texthip/thip/ui/group/search/screen/GroupSearchScreen.kt (2)
app/src/main/java/com/texthip/thip/ui/group/search/viewmodel/GroupSearchViewModel.kt (1)
onSearchButtonClick(123-141)app/src/main/java/com/texthip/thip/ui/group/search/component/GroupRecentSearch.kt (1)
GroupRecentSearch(27-62)
app/src/main/java/com/texthip/thip/ui/signin/screen/SplashScreen.kt (1)
app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomUnlockScreen.kt (1)
app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (1)
onTabSelected(77-105)
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageSaveScreen.kt (2)
app/src/main/java/com/texthip/thip/ui/mypage/component/FeedContent.kt (1)
FeedContent(25-63)app/src/main/java/com/texthip/thip/ui/mypage/component/BookContent.kt (1)
BookContent(26-68)
🔇 Additional comments (21)
app/src/main/java/com/texthip/thip/ui/signin/screen/SplashScreen.kt (2)
50-55: UI 분리 방향 좋습니다 (SplashContent 추출).화면 전환 로직과 UI가 분리되어 테스트·재사용성이 좋아졌습니다.
81-86: 프리뷰 구성 적절합니다.ThipTheme로 감싸 실제 배경과 타이포 환경을 재현한 점 좋습니다.
app/src/main/java/com/texthip/thip/ui/navigator/navigations/AuthNavigation.kt (1)
3-3: 패키지 경로 정리 OK.SplashScreen의 이동된 경로로 import 정리된 점 확인했습니다.
app/src/main/java/com/texthip/thip/ui/group/search/screen/GroupSearchScreen.kt (2)
49-60: UI-VM 분리 잘 됨 (콜백 주입으로 결합도↓).화면 이벤트를 VM 메서드에 매핑하는 방식이 명확합니다. 재사용성/테스트성 향상에 긍정적입니다.
220-259: 미리보기 구성 적절.실데이터에 가까운 샘플로 UI 확인이 용이합니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (3)
221-227: 바텀 탭 재선택 처리 OK.리시브 트리거 증가 시 새로고침+상단 스크롤이 한 번만 실행됩니다. 의도대로 동작해 보입니다.
274-283: 재선택 처리 중복 가능성 점검.onTabSelected에서 reselect 시 shouldScrollToTop=true + viewModel.onTabSelected(index)를 호출하고, VM도 reselect 분기에서 refresh를 수행합니다. bottomNav 재선택(라인 221-227)까지 고려하면, 새로고침이 2회 트리거될 여지가 있습니다.
원치 않는 이중 새로고침이 없는지 로그로 확인 부탁드립니다. 필요 시 UI는 스크롤만 담당하고 데이터 새로고침은 단일 경로로 정리하는 것을 권장합니다.
351-361: progress 활용 방식 적절.0..1f 범위의 fraction으로 안전하게 사용되고, 텍스트 상태와 시각적 진행률이 일치합니다.
Also applies to: 381-387
app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomUnlockScreen.kt (3)
84-91: 초기화/포커스/키보드 제어 흐름 깔끔.진입 시 상태 초기화 및 키보드 호출, 포커스 이동 시점이 자연스럽습니다.
130-138: Content 추출로 책임 분리가 잘 되었어요.입력 로직 콜백화로 테스트/미리보기 용이성이 높아졌습니다.
140-201: IME 패딩 적용 OK, 에러 표현 명확.advancedImePadding과 테마 색상 사용이 일관적입니다.
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageSaveScreen.kt (3)
65-76: 상태/이벤트 호이스팅 깔끔합니다
selectedTabIndex와onTabSelected를 상위Screen에서 관리하고Content로 전달한 구조가 Compose 베스트프랙티스에 부합합니다.
129-145: 탭 재선택 콜백 호이스팅 OK
onClick = { onTabSelected(index) }로 이벤트를 올려보내는 방식 적절합니다. 테스트/재사용성에 유리합니다.
178-253: 현재 Preview에서 리스트가 보이지 않습니다프리뷰에서
feedViewModel = null,bookViewModel = null로 전달되어 본문 렌더링이 생략됩니다. 위 폴백 전략을 적용하거나, 프리뷰 전용 스텁 액션/VM을 주입해 실제 리스트가 보이도록 해주세요.미리보기에서 리스트가 의도대로 표시되는지 확인 부탁드립니다.
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageEditScreen.kt (2)
124-126: 닉네임 대문자 방지 적용 LGTMWarningTextField에 preventUppercase=true 적용이 일관적입니다.
114-126: 서버/검증 로직과의 정책 정합성 확인클라이언트에서 대문자를 차단할 경우 서버 검증/정규식도 동일 정책인지 확인 필요합니다. 서버가 대소문자 구분 없이 허용/정규화(lowercase)한다면 UX는 “자동 소문자 변환”이 더 자연스럽습니다.
원하시면 서버 스펙과 맞춰 클라이언트 입력 정책(차단 vs 소문자 변환) 비교표를 드리겠습니다.
app/src/main/java/com/texthip/thip/ui/signin/screen/SignupNicknameScreen.kt (2)
109-111: 닉네임 대문자 방지 적용 LGTM회원가입 화면에서도 preventUppercase=true가 적용되어 일관성이 유지됩니다.
100-111: 정합성 확인: 경고 메시지/서버 정책과의 UX경고 메시지가 “허용 문자”를 안내한다면 대문자 입력 시 어떤 피드백(자동 소문자 변환 vs 입력 무시)을 줄지 명확히 해주세요. 현재 WarningTextField 구현은 “대문자 제거(무시)”이므로 사용자가 혼란을 느낄 수 있습니다.
원하시면 “자동 소문자 변환” UX로의 전환 패치를 제안하겠습니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (3)
132-145: 콜백으로 하위 컴포저블 의존성 분리한 점 좋습니다FeedCommentContent로 이벤트를 위임해 UI-VM 결합도가 낮아졌습니다. 유지보수성 향상에 👍
362-364: 좋아요/북마크 중복 탭 처리 확인 필요빠른 연속 탭 시 중복 요청/상태 튀김 가능. 진행 중 디스에이블/디바운스/낙관적 업데이트 롤백 전략 존재 여부 확인 부탁드립니다.
595-652: 프리뷰 충실도 좋습니다샘플 데이터가 실제 UI 상태와 근접해 디자인/QA에 유용합니다.
| var filtered = input | ||
| if (isNumberOnly) filtered = filtered.filter { it.isDigit() } | ||
| if (preventUppercase) filtered = filtered.filter { !it.isUpperCase() } | ||
| if (filtered.length > maxLength) filtered = filtered.take(maxLength) | ||
| onValueChange(filtered) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
대문자 ‘제거’는 손실적 UX — ‘소문자 변환’으로 전환 제안
현재는 대문자를 입력하면 문자가 사라집니다. 자동 소문자 변환이 더 예측 가능하고 친화적입니다.
- if (isNumberOnly) filtered = filtered.filter { it.isDigit() }
- if (preventUppercase) filtered = filtered.filter { !it.isUpperCase() }
+ if (isNumberOnly) filtered = filtered.filter { it.isDigit() }
+ if (preventUppercase) filtered = filtered.lowercase()📝 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.
| var filtered = input | |
| if (isNumberOnly) filtered = filtered.filter { it.isDigit() } | |
| if (preventUppercase) filtered = filtered.filter { !it.isUpperCase() } | |
| if (filtered.length > maxLength) filtered = filtered.take(maxLength) | |
| onValueChange(filtered) | |
| var filtered = input | |
| if (isNumberOnly) filtered = filtered.filter { it.isDigit() } | |
| if (preventUppercase) filtered = filtered.lowercase() | |
| if (filtered.length > maxLength) filtered = filtered.take(maxLength) | |
| onValueChange(filtered) |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/common/forms/WarningTextField.kt around
lines 63 to 67, currently uppercase characters are removed which causes a
jarring UX; change the transform so uppercase letters are converted to lowercase
instead of filtered out. Implement the sequence: start from input, if
preventUppercase then map characters to lowercase (not drop them), then if
isNumberOnly filter to digits, then apply maxLength truncation, and finally call
onValueChange with the resulting string; ensure the order preserves expected
behavior (lowercase before digit filtering/truncation).
| text = feedDetailUiState.error, | ||
| style = typography.copy_r400_s14, | ||
| color = colors.Grey |
There was a problem hiding this comment.
nullable 문자열을 Text에 직접 전달 — 컴파일 오류 발생
조건문으로 null을 거른 뒤에도 스마트 캐스트가 적용되지 않아 String? → String 불일치가 납니다.
- Text(
- text = feedDetailUiState.error,
+ Text(
+ text = feedDetailUiState.error ?: "알 수 없는 오류가 발생했어요",
style = typography.copy_r400_s14,
color = colors.Grey
)📝 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.
| text = feedDetailUiState.error, | |
| style = typography.copy_r400_s14, | |
| color = colors.Grey | |
| Text( | |
| text = feedDetailUiState.error ?: "알 수 없는 오류가 발생했어요", | |
| style = typography.copy_r400_s14, | |
| color = colors.Grey | |
| ) |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt around
lines 189-191, a nullable String (feedDetailUiState.error) is being passed
directly to Text causing a compile error because the compiler won't smart-cast;
change the call to provide a non-null String (for example use the Elvis operator
feedDetailUiState.error ?: ""), or wrap the usage in a let block
(feedDetailUiState.error?.let { Text(text = it, ...) }) or use a safe non-null
assertion only if you are certain it's never null; ensure Text always receives a
non-null String.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (1)
420-433: 댓글 롱프레스에서 '!!' 사용 — NPE 크래시 위험commentId가 null일 수 있는 모델이면 즉시 크래시합니다. 가드 처리로 변경하세요.
- onCommentLongPress = { comment -> - selectedCommentForMenu = CommentActionTarget(comment.commentId!!, comment.isWriter) - isCommentMenuVisible = true - }, + onCommentLongPress = { comment -> + comment.commentId?.let { + selectedCommentForMenu = CommentActionTarget(it, comment.isWriter) + isCommentMenuVisible = true + } + },필요 시 null인 항목에는 메뉴를 노출하지 않거나 토스트로 안내하세요.
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (1)
376-390: FeedMyScreen에서 중복 호출 제거
FeedMyScreen.kt 51번째 줄의viewModel.fetchMyFeedInfo()호출을 삭제하고 ViewModel 내의 init/onTabSelected 흐름만 사용하세요.app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt (1)
22-22:import kotlin.onFailure삭제 (FeedWriteViewModel.kt:22)-import kotlin.onFailure
onFailure는Result의 확장 함수로 별도 import가 필요 없습니다.app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
157-166: async 내부에서 launch를 호출하는 함수들을 기다리지 못하고 있습니다.
loadUserName()/loadRoomSections()가 내부에서viewModelScope.launch를 사용하므로, 여기서async { ... }로 감싸도awaitAll()이 실제 작업 완료를 보장하지 않습니다. 이로 인해isRefreshing이 조기 해제되는 UI 불일치가 발생할 수 있습니다. 두 함수를suspend로 전환하거나, 이 지점에서 직접 repository 호출을 수행하도록 리팩터링하세요.리팩터링 예시:
- val jobs = listOf( - async { loadUserName() }, - async { - resetMyGroupsData() - loadMyGroupsSuspend(isInitial = true) - }, - async { loadRoomSections() }, - ) - jobs.awaitAll() + coroutineScope { + val jobs = listOf( + async { loadUserNameSuspend() }, + async { + resetMyGroupsData() + loadMyGroupsSuspend(isInitial = true) + }, + async { loadRoomSectionsSuspend() }, + ) + jobs.awaitAll() + }함수 본문 변경(파일 외부 수정 예시):
// 기존: viewModelScope.launch { ... } suspend fun loadUserNameSuspend() { repository.getUserName() .onSuccess { userName -> updateState { it.copy(userName = userName) } } } suspend fun loadRoomSectionsSuspend() { updateState { it.copy(roomSectionsError = null) } val genresResult = repository.getGenres() val selectedIndex = uiState.value.selectedGenreIndex val selectedGenre = if (genresResult.isSuccess) { val genres = genresResult.getOrThrow() if (selectedIndex in genres.indices) genres[selectedIndex] else genres.firstOrNull() ?: Genre.getDefault() } else { Genre.getDefault() } repository.getRoomSections(selectedGenre) .onSuccess { roomMainList -> updateState { it.copy(roomMainList = roomMainList) } } .onFailure { error -> updateState { it.copy(roomSectionsError = error.message) } } }
♻️ Duplicate comments (1)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (1)
189-193: nullable 문자열을 Text에 직접 전달 — 컴파일 오류 가능이전 코멘트와 동일 이슈가 남아 있습니다. Elvis로 non-null 보장해주세요.
- Text( - text = feedDetailUiState.error, + Text( + text = feedDetailUiState.error ?: "알 수 없는 오류가 발생했어요", style = typography.copy_r400_s14, color = colors.Grey )
🧹 Nitpick comments (41)
app/src/main/java/com/texthip/thip/ui/feed/component/ImageViewerModal.kt (1)
57-59: 문자열 하드코딩 제거contentDescription의 "닫기"는 리소스로 추출해 i18n/접근성 일관성을 확보하세요.
- contentDescription = "닫기", + contentDescription = stringResource(R.string.cd_close),strings.xml(예시):
<string name="cd_close">닫기</string>app/src/main/java/com/texthip/thip/ui/feed/component/LiveSearchPeopleResult.kt (3)
27-50: LazyColumn 항목에 key 추가로 재구성 안정성 향상리스트 스크롤/애니메이션 안정성과 성능을 위해 key 지정 권장합니다(특히 userId가 있음).
- itemsIndexed(peopleList) { index, user -> + itemsIndexed( + items = peopleList, + key = { _, u -> u.userId } + ) { index, user ->
41-47: Divider 컴포저블 사용 권장Spacer+background 대신 Divider(HorizontalDivider)를 사용하면 의미가 명확하고 접근성/테마 일관성이 좋아집니다.
- Spacer( - modifier = Modifier - .padding(vertical = 12.dp, horizontal = 20.dp) - .fillMaxWidth() - .height(1.dp) - .background(colors.DarkGrey02) - ) + androidx.compose.material3.HorizontalDivider( + modifier = Modifier.padding(vertical = 12.dp, horizontal = 20.dp), + color = colors.DarkGrey02, + thickness = 1.dp + )
64-64: Preview의 userId가 모두 동일 — key 도입 시 충돌 소지Preview 데이터의 userId가 전부 1L입니다. 위에서 key를 도입하면 키 충돌이 발생하니 미리 고쳐 두는 것을 권장합니다.
- userId = 1L, + userId = 1L, ... - userId = 1L, + userId = 2L, ... - userId = 1L, + userId = 3L,Also applies to: 74-74, 83-83
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (6)
148-161: FQCN 파라미터 타입 간소화(import 사용)이미 import된 타입을 사용해 시그니처를 간결히 하는 편이 가독성에 좋습니다.
- feedDetailUiState: com.texthip.thip.ui.feed.viewmodel.FeedDetailUiState, - commentsUiState: com.texthip.thip.ui.group.note.viewmodel.CommentsUiState, + feedDetailUiState: FeedDetailUiState, + commentsUiState: CommentsUiState,
316-329: 이미지 접근성: contentDescription 제공 권장화면낭독을 위해 적절한 contentDescription을 제공하세요(또는 클릭 액션 용도라도 “피드 이미지 n” 등).
- AsyncImage( - model = imageUrl, - contentDescription = null, + AsyncImage( + model = imageUrl, + contentDescription = stringResource(R.string.feed_image_content_description, index + 1),(리소스가 없다면 추가 필요)
449-455: 댓글 전송 시 공백 제거UX 측면에서 본문은 trim하여 전달하는 편이 좋습니다.
- CommentsEvent.CreateComment( - content = commentInput, + CommentsEvent.CreateComment( + content = commentInput.trim(),
316-316: 매직 넘버 3 상수화이미지 최대 미리보기 개수 3이 중복됩니다. 상수로 추출해 일관성/변경 용이성을 높이세요.
// 파일 상단에 추가 private const val MAX_PREVIEW_IMAGES = 3- itemsIndexed(images.take(3)) { index, imageUrl -> + itemsIndexed(images.take(MAX_PREVIEW_IMAGES)) { index, imageUrl ->- imageUrls = images.take(3), + imageUrls = images.take(MAX_PREVIEW_IMAGES),Also applies to: 589-589
284-284: aliasColor 빈 문자열 대비 처리
FeedDetailResponse.aliasColor는 non-null String이므로 null 체크 없이 빈 문자열만 처리해주세요.- bottomTextColor = hexToColor(feedDetail.aliasColor) + bottomTextColor = hexToColor(feedDetail.aliasColor.ifBlank { "#FFFFFF" })
121-123: postType 문자열 리터럴 대신 enum/Sealed class로 대체해 타입 안전성 강화
postId 타입(Long)은 feedId(Long)와 일치하므로 문제없습니다.app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedOthersViewModel.kt (1)
66-88: fetchData 재진입 가능성 — 단일 실행 가드 추가 권장공개 메서드로 바뀌면서 Screen 등에서 중복 호출될 수 있고, init에서도 이미 호출되어 초기에 중복 네트워크 요청이 발생할 수 있습니다. 단일 실행(single-flight) 가드를 두어 중복 호출을 무시하거나 이전 작업을 취소하세요.
아래처럼 Job 가드를 두는 방식을 제안합니다.
추가(파일 상단 멤버):
private var fetchJob: Job? = nullimport:
import kotlinx.coroutines.Job변경(이 범위 내 diff):
- fun fetchData() { - viewModelScope.launch { + fun fetchData() { + if (fetchJob?.isActive == true) return + fetchJob = viewModelScope.launch { _uiState.update { it.copy(isLoading = true) } val userInfoDeferred = async { feedRepository.getFeedUsersInfo(userId) } val feedsDeferred = async { feedRepository.getFeedUsers(userId) } val userInfoResult = userInfoDeferred.await() val feedsResult = feedsDeferred.await() val fetchedFeeds = feedsResult.getOrNull()?.feedList ?: emptyList() _uiState.update { it.copy( isLoading = false, userInfo = userInfoResult.getOrNull(), feeds = fetchedFeeds, errorMessage = userInfoResult.exceptionOrNull()?.message ?: feedsResult.exceptionOrNull()?.message ) } } }app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (4)
78-115: 피드 상태 반영 로직 중복 — updateFeedStateFromResult 재사용으로 DRY동일한 매핑 로직이 아래의 updateFeedStateFromResult와 중복됩니다. 구독부에서 해당 메서드만 호출하도록 단순화하세요.
- private fun observeFeedUpdates() { - viewModelScope.launch { - feedRepository.feedStateUpdateResult.collect { update -> - val updatedAllFeeds = _uiState.value.allFeeds.map { feed -> - if (feed.feedId.toLong() == update.feedId) { - feed.copy( - isLiked = update.isLiked, - likeCount = update.likeCount, - isSaved = update.isSaved, - commentCount = update.commentCount - ) - } else { - feed - } - } - - val updatedMyFeeds = _uiState.value.myFeeds.map { feed -> - if (feed.feedId.toLong() == update.feedId) { - feed.copy( - isLiked = update.isLiked, - likeCount = update.likeCount, - isSaved = update.isSaved, - commentCount = update.commentCount - ) - } else { - feed - } - } - - _uiState.update { - it.copy( - allFeeds = updatedAllFeeds, - myFeeds = updatedMyFeeds - ) - } - } - } - } + private fun observeFeedUpdates() { + viewModelScope.launch { + feedRepository.feedStateUpdateResult.collect(::updateFeedStateFromResult) + } + }추가로, 이 영역에서도 updateState와 _uiState.update가 혼용됩니다. 한 가지로 통일 권장.
118-145: 탭 재선택 시 전체 데이터 새로고침은 과도할 수 있음reselect에서 refreshDataAndScrollToTop()은 양 탭 모두를 새로고침합니다. 트래픽/지연을 줄이려면 현재 탭만 새로고침하도록 스코프를 좁히는 것이 일반적입니다.
- // 같은 탭 다시 클릭 시: 전체 새로고침 + 스크롤 상단 - refreshDataAndScrollToTop() + // 같은 탭 다시 클릭 시: 현재 탭만 새로고침 (스크롤은 Screen에서 처리) + refreshCurrentTab()동일 변경을 탭 1 분기에도 적용하세요.
147-155: 바텀 탭 재선택 연타 시 네트워크 스팸 가능 — 간단한 스로틀/잡 가드 권장연속 탭으로 refreshData()가 중첩 실행될 수 있습니다. 최근 실행을 취소/무시하도록 Job 가드를 두거나, 짧은 스로틀을 적용하세요.
예시(필드 추가):
private var reselectionRefreshJob: Job? = null예시(본문 변경):
- fun refreshOnBottomNavReselect() { - // 바텀 네비게이션에서 같은 탭 다시 클릭 시 (스크롤은 Screen에서 처리) - refreshData() - } + fun refreshOnBottomNavReselect() { + if (reselectionRefreshJob?.isActive == true) return + reselectionRefreshJob = viewModelScope.launch { refreshCurrentTab() } + }
501-529: updateFeedStateFromResult 도입 좋습니다단일 업데이트 적용 API로 외부/내부 이벤트 처리에 재사용 가능해졌습니다. LGTM. observeFeedUpdates에서도 본 메서드 재사용 권장(위 코멘트 참고).
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupUiState.kt (1)
16-18: 페이징 상태 변수·에러 필드 통합 권장
- ViewModel의 GroupUiState에 hasMoreMyGroups 와 isLast 를 동시에 정의·사용 중입니다. 둘 중 하나만 남기고, canLoadMore 도 해당 필드 기준만 참조하도록 단순화하세요.
- 화면 스코프에서 error 와 roomSectionsError 가 중복되어 사용됩니다. 책임 범위에 맞춰 단일 필드로 통일하거나 역할을 분리하세요.
- GroupUiState 생성부에 named 인자를 다수 사용 중이므로(Preview/호출부 다수) 필드 제거·이름 변경 시 호출부도 함께 수정해야 합니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt (1)
25-34: 로딩/페이징 상태 파생 프로퍼티 도입 제안 (호출 무결성 확인 완료)
- 로딩 더보기 상태를 묶어 계산하는 파생 프로퍼티 추가 제안:
val isAnyBookLoadingMore: Boolean get() = isLoadingMoreSavedBooks || isLoadingMoreGroupBooks || isLoadingMoreSearchResults
isLast*대신nextCursor == null기반으로hasMore*파생 패턴 고려GroupMakeRoomUiState호출은 화면(명명 인자)과 뷰모델(기본 생성자) 두 곳에서만 사용되며, 기본값 덕분에 순서 변경 없이 모두 정상 작동 확인됨app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt (7)
200-203: 초기 로딩 트리거 OK, 단isLoadingBooks와 하위 플래그 일관성 주의
isLoadingBooks=true후 각각의 finally에서만 false로 내립니다. 현재 플래그 조합은 동시 로딩 2건을 올바르게 종료시키지만, 향후 로직 추가 시 드리프트 위험이 있습니다. 파생값 전환을 고려하세요.
205-253: SAVED 리스트 로드: 타입 안정성과 오류 전달 개선 제안
- 매직 스트링
"SAVED"대신 enum/sealed class 사용 권장.- 초기 로드 실패 시 UI에
errorMessage설정이 없어서 사용자가 실패를 인지하기 어렵습니다.다음처럼 에러 메시지 반영을 고려하세요(개념 예시):
- .onFailure { exception -> - if (isInitial) { - updateState { it.copy(savedBooks = emptyList()) } - } - } + .onFailure { exception -> + if (isInitial) { + updateState { it.copy(savedBooks = emptyList(), errorMessage = exception.message) } + } + }또한 로직이
loadGroupBooks와 거의 동일하므로 공통화로 중복을 줄일 수 있습니다(제네릭 헬퍼로 추출).
255-303: GROUP 리스트 로드: 중복 제거 및 매직 스트링 제거
"JOINING"또한 enum/sealed class로 치환하고,loadSavedBooks와의 중복을 헬퍼로 추출하세요(커서·리스트·플래그를 파라미터화).
342-353: 검색 디바운스/상태 전이 양호, UX 미세개선 포인트새 검색 시 결과를 즉시 비워 스켈레톤을 보이게 하는 패턴은 합리적입니다. 필요 시 이전 결과를 유지하고 상단에 로딩 인디케이터만 갱신하는 옵션도 고려해 보세요(플리커 감소).
Also applies to: 360-367
410-418: 가드 조건 적절, 최신 상태 스냅샷 사용 권장람다 외부에서 캡처한
currentState는 코루틴 시작 시점 이후 변경될 수 있습니다. 내부에서 최신 상태를 다시 읽으면 안전합니다.- val currentState = _uiState.value - if (currentState.isLoadingMoreSearchResults || + val snapshot = _uiState.value + if (snapshot.isLoadingMoreSearchResults || ... - currentState.currentSearchQuery.isBlank()) { + snapshot.currentSearchQuery.isBlank()) { return } ... - val nextPage = currentState.searchPage + 1 + val nextPage = _uiState.value.searchPage + 1
456-459: 예외 삼킴(detekt 경고) — 최소 로깅 추가예외를 로그로 남겨 디버깅 가능성을 확보하세요.
- } catch (e: Exception) { - updateState { - it.copy(isLoadingMoreSearchResults = false) - } - } + } catch (e: Exception) { + android.util.Log.e("FeedWriteViewModel", "loadMoreSearchResults failed", e) + updateState { it.copy(isLoadingMoreSearchResults = false) } + }
322-329: BookUserSaveList.toBookDataFromSaved() 미사용 함수 삭제
FeedWriteViewModel.kt에 정의된 해당 확장 함수는 코드 어디에서도 호출되지 않습니다. 사용하지 않는다면 제거하세요.app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt (1)
75-75: onCardVisible 인자 미사용 — 시그니처와 구현의 의도 불일치cardIndex를 사용하지 않으므로 불필요 경고/오해를 막기 위해 명시적으로 무시하거나, 장기적으로는 파라미터 없는 콜백으로 계약을 정리하는 게 좋습니다.
적용 예(단기 수정):
- onCardVisible = { cardIndex -> viewModel.loadMoreGroups() }, + onCardVisible = { _ -> viewModel.loadMoreGroups() },장기 방향(별도 PR 권장): GroupContent/GroupPager의 콜백 타입을
(Int) -> Unit→() -> Unit으로 변경하고 명확히onLoadMore의미로 변경.app/src/main/java/com/texthip/thip/ui/mypage/component/BookContent.kt (1)
12-19: 무한 스크롤 트리거 경계값 보강 및 가독성 개선현재 lastVisible 계산에서
+ 1이 더해져 직관성이 떨어지고, 아이템 수가 적을 때(예: < 3) 초기 과도 호출 가능성이 있습니다. 임계값을 0으로 클램핑하고 인덱스를 그대로 비교하도록 정리하면 안전합니다. 또한 import에max가 필요합니다.제안 diff:
import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.remember +import kotlin.math.max @@ val shouldLoadMore = remember { derivedStateOf { val layoutInfo = listState.layoutInfo - val totalItemsNumber = layoutInfo.totalItemsCount - val lastVisibleItemIndex = (layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0) + 1 - - totalItemsNumber > 0 && lastVisibleItemIndex >= totalItemsNumber - 3 + val totalItemsNumber = layoutInfo.totalItemsCount + val lastVisibleIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: -1 + totalItemsNumber > 0 && lastVisibleIndex >= max(0, totalItemsNumber - 3) } } @@ - LazyColumn( - state = listState, + LazyColumn( + state = listState,추가 제안(선택): 중복 호출 방지를 더 확실히 하려면 snapshotFlow + distinctUntilChanged로 트리거를 스트림화하세요. (별도 PR 권장)
LaunchedEffect(listState) { snapshotFlow { shouldLoadMore.value } .distinctUntilChanged() .filter { it } .collect { viewModel.loadMoreBooks() } }필요 시
kotlinx.coroutines.flow.*import 추가.Also applies to: 39-55, 57-57
app/src/main/java/com/texthip/thip/data/model/book/response/BookUserSaveResponse.kt (1)
8-11: 응답 스키마 일관성: bookList 기본값/직렬화 명시화동일 계열 응답(BookListResponse)과 맞추어
bookList에 기본값과 SerialName을 부여하면 NPE/마이그레이션 리스크를 줄일 수 있습니다.제안 diff:
@Serializable data class BookUserSaveResponse( - val bookList: List<BookUserSaveList>, + @SerialName("bookList") val bookList: List<BookUserSaveList> = emptyList(), @SerialName("nextCursor") val nextCursor: String? = null, @SerialName("isLast") val isLast: Boolean = false )app/src/main/java/com/texthip/thip/data/repository/RoomsRepository.kt (1)
45-54: 닉네임 캐싱은 첫 페이지에서만 수행하도록 가드 권장커서 기반 무한 스크롤에서 다음 페이지 요청마다 캐시 쓰기가 반복될 수 있습니다. 첫 로드(커서가 null)일 때만 캐싱하도록 가드하면 불필요한 I/O를 줄일 수 있습니다.
- response?.let { joinedRoomsDto -> - userDataManager.cacheUserName(joinedRoomsDto.nickname) - } + response?.let { joinedRoomsDto -> + if (cursor == null) { + userDataManager.cacheUserName(joinedRoomsDto.nickname) + } + }app/src/main/java/com/texthip/thip/data/service/BookService.kt (1)
23-25: cursor 파라미터 추가 LGTM + 타입 안정성 제안구현은 적절합니다. 다만
type: String은 오입력 위험이 있어 컴파일 타임 제약을 권장합니다(예: @stringdef 또는 enum).- suspend fun getBooks( - @Query("type") type: String, + @StringDef("saved", "group") + @Retention(AnnotationRetention.SOURCE) + annotation class BookListType + + suspend fun getBooks( + @Query("type") @BookListType type: String, @Query("cursor") cursor: String? = null ): BaseResponse<BookListResponse>app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (1)
87-89: load-more 콜백 중복 트리거 방지 확인지금은 VM 메서드를 그대로 바인딩합니다. 스크롤 임계 도달 시 연속 호출을 막기 위해 VM 또는 하위 컴포넌트에서
isLoadingMore* || !hasMore*가드가 있는지 확인해주세요. 없다면 간단 가드 추가를 권장합니다.app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageSaveScreen.kt (2)
59-63: 탭 전환 시 초기 로드 중복 호출 가능성 최소화책 탭을 여러 번 진입할 때
isInitial = true호출이 반복될 수 있습니다. 로컬 플래그로 1회만 수행하도록 가드하면 불필요한 네트워크 호출을 줄일 수 있습니다.- LaunchedEffect(selectedTabIndex) { - when (selectedTabIndex) { - 0 -> feedViewModel.loadSavedFeeds() - 1 -> bookViewModel.loadSavedBooks(isInitial = true) - } - } + var hasLoadedBooks by rememberSaveable { mutableStateOf(false) } + LaunchedEffect(selectedTabIndex) { + when (selectedTabIndex) { + 0 -> feedViewModel.loadSavedFeeds() + 1 -> if (!hasLoadedBooks) { + bookViewModel.loadSavedBooks(isInitial = true) + hasLoadedBooks = true + } + } + }
80-90: Content 컴포저블의 VM 파라미터 nullable 처리 LGTM탭별 조건 렌더링과 궁합이 좋습니다. 추가로,
tabs리스트는 상수 성격이라remember로 감싸면 미세한 재조합을 줄일 수 있습니다.- val tabs = listOf(stringResource(R.string.feed), stringResource(R.string.book)) + val tabs = remember { + listOf(stringResource(R.string.feed), stringResource(R.string.book)) + }app/src/main/java/com/texthip/thip/ui/feed/screen/FeedWriteScreen.kt (1)
414-423: hasMore = !isLast*의 이중 부정은 가독성이 떨어집니다.UiState에
hasMoreSavedBooks/hasMoreGroupBooks/hasMoreSearchResults를 직접 노출하면 의미 충돌과 실수를 줄일 수 있습니다. 현 구조 유지 시에도, 변수명 주석으로 의도를 명시해 주세요.app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt (3)
50-55: 무한 스크롤 트리거 안정화 제안: snapshotFlow 기반으로 중복 호출을 줄이세요.
LaunchedEffect(shouldLoadMore.value, …)는 재조합 타이밍에 따라 연속 호출될 수 있습니다.snapshotFlow+distinctUntilChanged(필요 시)로 안정화하면 좋습니다.- LaunchedEffect(shouldLoadMore.value, hasMore, isLoadingMore) { - if (shouldLoadMore.value && hasMore && !isLoadingMore && books.isNotEmpty()) { - onLoadMore() - } - } + LaunchedEffect(hasMore, isLoadingMore, books) { + snapshotFlow { shouldLoadMore.value } + .filter { it && hasMore && !isLoadingMore && books.isNotEmpty() } + .collect { onLoadMore() } + }
62-77: 리스트 항목에 key를 부여해 재조합/이동 비용을 줄이세요.안정적인 key가 없으면 스크롤 시 아이템 재구성이 커집니다.
isbn혹은 고유 ID가 있으면 그 값을 key로 사용하세요.- items(books) { book -> + items(items = books, key = { it.isbn ?: it.title }) { book -> CardBookSearch( title = book.title, imageUrl = book.imageUrl, onClick = { onBookClick(book) } )
38-48: 임계값 상수화(가독성/튜닝 용이성).
totalItemsNumber - 3의 매직 넘버를 상수로 추출하면 향후 UX 튜닝이 쉬워집니다(예:private const val PREFETCH_THRESHOLD = 3).app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt (1)
43-52: 페이지네이션 props 기본값 재검토 (무해하지만 과호출 위험 존재)hasMore* 기본값이 true이고 onLoadMore*가 빈 람다일 경우, 아이템이 존재할 때 스크롤 하단 진입 시 빈 콜백이 반복 호출될 수 있습니다. 기능상 문제는 없지만 추후 실제 로더 연결을 누락했을 때 디버깅 난이도가 올라갑니다. 기본값을 보수적으로 false로 두거나 KDoc에 “반드시 연결 필요”를 명시하는 방안을 고려해 주세요.
가능한 수정 예:
- hasMoreSaved: Boolean = true, - hasMoreGroup: Boolean = true, - hasMoreSearch: Boolean = true, + hasMoreSaved: Boolean = false, + hasMoreGroup: Boolean = false, + hasMoreSearch: Boolean = false,app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (2)
146-194: Group 목록 로딩: 로깅/에러 신호 동일화Saved와 동일한 형태로 실패 로깅을 추가해 주세요. 공통화하면 더 좋습니다(예: private fun logError(tag, msg, e)).
제안 diff:
- .onFailure { exception -> - if (isInitial) { - updateState { it.copy(groupBooks = emptyList()) } - } - } + .onFailure { exception -> + if (isInitial) { + updateState { it.copy(groupBooks = emptyList()) } + } + android.util.Log.e("GroupMakeRoomVM", "loadGroupBooks failed", exception) + }
213-221: 미사용 확장 함수 제거
GroupMakeRoomViewModel.kt의 213221행에 정의된329행)에도 정의만 되어 있으며 사용되지 않으니, 데드 코드 제거를 권장합니다.BookUserSaveList.toBookDataFromSaved()가 코드베이스 내에서 호출되지 않습니다. 동일한 확장 함수가 FeedWriteViewModel.kt(322app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/SavedBookViewModel.kt (2)
73-76: printStackTrace 대신 표준 로깅 사용printStackTrace는 릴리즈에서 유실됩니다. Log 또는 Crashlytics로 전환해 주세요.
제안 diff:
- updateState { it.copy(error = exception.message) } - exception.printStackTrace() + updateState { it.copy(error = exception.message) } + android.util.Log.e("SavedBookVM", "loadSavedBooks failed", exception)
90-97: 토글 API 파라미터 의도 확인 필요saveBook(isbn, type = false)의 의미가 “저장 해제”인지 “저장”인지 명확하지 않습니다. UI 의도(목록에서 토글 후 다시 초기 로딩)와 일치하는지 확인해 주세요. 필요 시 도메인 메서드 분리(save/unsave) 또는 enum 도입을 권장합니다.
원하시면 저장/해제 API를 명시적으로 분리하는 리팩터링 패치를 제안드리겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (28)
app/src/main/java/com/texthip/thip/data/model/book/response/BookListResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookUserSaveResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/rooms/response/JoinedRoomListResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt(2 hunks)app/src/main/java/com/texthip/thip/data/repository/RoomsRepository.kt(1 hunks)app/src/main/java/com/texthip/thip/data/service/BookService.kt(2 hunks)app/src/main/java/com/texthip/thip/data/service/RoomsService.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/component/ImageViewerModal.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/component/LiveSearchPeopleResult.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt(12 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedMyScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedOthersScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedWriteScreen.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedOthersViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt(7 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt(7 hunks)app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/mypage/component/BookContent.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageSaveScreen.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/SavedBookViewModel.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt (1)
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (3)
updateState(47-49)loadSavedBooks(96-144)loadGroupBooks(146-194)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt (1)
app/src/main/java/com/texthip/thip/ui/common/cards/CardBookSearch.kt (1)
CardBookSearch(25-66)
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageSaveScreen.kt (2)
app/src/main/java/com/texthip/thip/ui/mypage/component/FeedContent.kt (1)
FeedContent(25-63)app/src/main/java/com/texthip/thip/ui/mypage/component/BookContent.kt (1)
BookContent(30-90)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (2)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedOthersViewModel.kt (1)
observeFeedUpdates(46-63)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedDetailViewModel.kt (2)
observeFeedUpdates(43-60)updateState(40-42)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt (1)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt (1)
GroupBookListWithScrollbar(30-92)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (2)
app/src/main/java/com/texthip/thip/utils/color/HexToColor.kt (1)
hexToColor(6-13)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
🪛 detekt (1.23.8)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt
[warning] 455-455: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
[warning] 346-346: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (36)
app/src/main/java/com/texthip/thip/ui/feed/component/LiveSearchPeopleResult.kt (1)
33-33: AuthorHeader 기본 파라미터 확인 및 roleColor 사용 검토
AuthorHeader시그니처에badgeTextColor: Color = colors.NeonGreen기본값이 이미 있어 호출부 호환성 문제 없음.LiveSearchPeopleResult만badgeTextColor = user.roleColor를 명시하며, 그 외 호출부는 기본값을 사용 중입니다.user.roleColor가 텍스트용 색상인지, 배지 배경색 컨셉인지 의도를 다시 확인하고, 대비(contrast) 요구사항을 충족하도록contentColorFor또는 MaterialTheme 기반 색상 유도도 고려하세요.app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (6)
48-48: 프리뷰/모델 import 추가 OK프리뷰와 미리보기용 모델 참조를 위한 import들이 적절합니다.
Also applies to: 55-56, 67-67, 71-71, 74-74, 77-77
133-146: 콜백으로 ViewModel 호출 위임: 👍UI → 콜백 → VM 흐름으로 단방향 데이터 흐름이 명확해졌습니다.
364-366: ActionBarButton 콜백 전달 OKUI와 상태 변경 분리가 잘 되었습니다.
538-542: 삭제 이벤트 위임 OK도메인 이벤트로 위임되어 테스트 용이성이 좋아졌습니다.
577-578: 삭제 확인 콜백 위임 OKUI 레이어가 비즈니스 로직에 의존하지 않도록 잘 분리됐습니다.
596-654: 프리뷰 구성 👍실데이터에 가까운 샘플로 UI 확인이 수월합니다.
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (2)
71-72: init에서 observeFeedUpdates() 등록 — 방향성 적절초기 구독으로 상태 동기화 보장됩니다. LGTM.
531-541: 삭제된 피드 제거 로직 명확 — LGTM양 리스트에서 동시 제거로 일관성 유지됩니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedOthersScreen.kt (1)
58-60: LaunchedEffect 블록에서 fetchData() 중복 호출 제거
FeedOthersScreen.kt (58–60): ViewModel init에서 이미 fetchData()를 호출하므로 Compose 레이어의 중복 네트워크 요청이 발생합니다. 해당 블록을 삭제하세요.- LaunchedEffect(Unit) { - viewModel.fetchData() - }app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteUiState.kt (1)
21-30: 페이징 상태 추가 안전 확인 및 파생값 제안
FeedWriteScreen/FeedWriteViewModel 모두 기본값 또는 명명 인자로만FeedWriteUiState를 호출하므로 신규 프로퍼티 추가에 따른 영향 없음 확인. 뷰 로직 단순화를 위해val hasMoreSavedBooks get() = !isLastSavedBooks등 파생값 추가 권장.
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt (4)
35-40: 검색/더보기 Job, 커서/로딩 플래그 도입 적절Job 취소 및 커서/로딩 상태 분리는 👍.
onCleared에서 모두 취소하는 처리도 적절합니다.
305-311: 더보기 래퍼 메서드 적절간결하고 명확합니다.
642-646: 리소스 정리 적절
onCleared에서 Job을 모두 취소하는 처리 👍
9-9: BookUserSaveList import 유지
FeedWriteViewModel.kt 322행의BookUserSaveList.toBookDataFromSaved()확장 함수에서 해당 타입을 참조하므로, import 제거 시 컴파일 에러가 발생합니다.Likely an incorrect or invalid review comment.
app/src/main/java/com/texthip/thip/data/model/book/response/BookListResponse.kt (1)
9-12: LGTM — 커서 기반 페이지네이션 필드 추가 적절
nextCursor/isLast도입과 기본값 지정이 일관되고 안전합니다. 호출부에서 널/종료 상태 처리만 보장되면 됩니다.app/src/main/java/com/texthip/thip/data/repository/RoomsRepository.kt (1)
45-46: 시그니처 변경 영향 범위 점검 완료
getMyJoinedRooms 호출부는 모두 cursor 기반으로 업데이트됐으며, 관련 page 파라미터 호출 흔적이 없습니다.app/src/main/java/com/texthip/thip/data/service/BookService.kt (1)
60-62: getSavedBooks의 커서 도입 LGTM서명 변경과 기본값(null) 전략이 Retrofit과 잘 맞습니다. 별도 이슈 없습니다.
app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt (2)
22-26: cursor 기반 조회 도입 LGTM서비스 계층과 일관되게 커서를 전달하고 예외를 Result로 감싼 처리 흐름이 명확합니다.
70-74: 저장 도서 커서 조회 도입 LGTM동일 패턴으로 문제 없습니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (2)
109-113: 컴포저블 API 확장 LGTM기본값을 제공해 호출부 부담 없이 점진 도입 가능해 보입니다.
267-276: GroupBookSearchBottomSheet에 isSearching 파라미터가 여전히 정의되어 있으므로 호출부 수정 불필요Likely an incorrect or invalid review comment.
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageSaveScreen.kt (3)
56-58: UI 상태 수집 방식 LGTMSavedBookViewModel의 uiState를 기준으로 리스트를 파생하는 구조가 명확합니다.
66-76: 레이아웃 분리(MypageSaveContent)로 책임 분담 명확화 LGTM호출부가 단순해지고 Preview에도 유리합니다.
155-161: 탭별 콘텐츠 렌더링 LGTMnull-safe 처리와 키 전달이 명확합니다.
Also applies to: 163-169
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedWriteScreen.kt (2)
97-99: onLoadMore 콜백 체인 연결은 적절합니다. 중복 호출 가드만 확인하세요.*BottomSheet 쪽 무한 스크롤 트리거와 ViewModel의
loadMore*가 동시에/연속으로 호출될 수 있으니, VM 내부에isLoading/isLast기반의 가드와 중복 요청 방지(예: debounce 또는 최근 호출 타임스탬프) 여부를 점검해 주세요.
120-124: FeedWriteContent API 확장(기본값 제공) 좋습니다.기존 호출부에 대한 이진/소스 호환성 유지가 잘 되어 있습니다.
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (3)
65-103: 커서 기반 페이지네이션 상태 처리 적절합니다.중복 로딩 가드와
isLast/hasMoreMyGroups동기화가 명확합니다.
105-111: loadMoreGroups 가드 로직 명확합니다.
hasMoreMyGroups와!isLoadingMyGroups체크로 불필요한 중복 호출을 잘 방지하고 있습니다.
173-182: 리셋 로직 적절합니다.페이지네이션 상태(
nextCursor,hasMoreMyGroups,isLast)와 목록 초기화가 일관됩니다.app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt (2)
171-171: 프리뷰: 더미 데이터 연결 확인dummySavedBooks 연결 변경 사항 문제없습니다. 프리뷰 동작에 도움 됩니다.
189-189: 프리뷰: 빈 리스트 케이스 확인빈 리스트 프리뷰 추가도 적절합니다. UI 회귀 방지에 유용합니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
459-463: ViewModel 정리 로직 적절Job 취소 처리 좋습니다.
app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/SavedBookViewModel.kt (3)
14-22: UI 상태 모델 구성 적절파생 프로퍼티(canLoadMore)로 로딩 가능 여부를 캡슐화한 점 좋습니다.
29-37: 상태 래퍼 및 업데이트 헬퍼 구성 LGTMMutableStateFlow + updateState 패턴 적절합니다.
84-87: loadMoreBooks 게이트 조건 정확canLoadMore 활용 적절합니다.
| @SerialName("nickname") val nickname: String, | ||
| @SerialName("page") val page: Int, | ||
| @SerialName("size") val size: Int, | ||
| @SerialName("nextCursor") val nextCursor: String? = null, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
커서 기반 필드 도입 OK, 사용처 점검 필요
nextCursor 추가는 적절합니다. 다만 last/first와 동시 노출 시 어떤 값을 진리 소스로 삼을지 명확히 하세요(권장: 한쪽만 UI 판단 근거).
서비스/레포지토리 시그니처가 커서 파라미터를 수용하는지 점검:
🏁 Script executed:
#!/bin/bash
rg -nP '\bgetJoinedRooms\s*\(' -C3
rg -nP 'RoomsService|RoomsRepository' -C3 | rg -nP '\bcursor\b' -nLength of output: 1528
커서 기반 페이징 UI 진리 소스 결정 필요
- nextCursor 추가 및 서비스( getJoinedRooms )·레포지토리( getMyJoinedRooms )의
cursor파라미터 수용은 이미 완료되어 있습니다. - lastCursor/firstCursor와 함께 노출될 경우 UI에서 사용할 진리 소스를 하나로 명확히 설정(권장: 하나만 사용)해주세요.
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/data/model/rooms/response/JoinedRoomListResponse.kt
around line 11, the model currently exposes nextCursor while the API/service
also supports lastCursor/firstCursor, causing ambiguity for the UI; pick a
single canonical cursor field (recommended: keep only nextCursor) and update the
response model to expose only that field (or mark others deprecated and remove
their serialization), then ensure getJoinedRooms/getMyJoinedRooms and any DTOs,
serializers, and UI consumers are aligned to use the chosen cursor name
consistently and update any docs/tests accordingly.
| Box( | ||
| modifier = Modifier | ||
| .align(Alignment.TopStart) | ||
| .padding(20.dp) | ||
| .size(24.dp) | ||
| .clickable { onDismiss() } | ||
| .padding(horizontal = 20.dp, vertical = 16.dp) | ||
| .zIndex(1f) | ||
| ) | ||
| ) { | ||
| Icon( | ||
| painter = painterResource(R.drawable.ic_arrow_back), | ||
| contentDescription = "닫기", | ||
| tint = colors.White, | ||
| modifier = Modifier | ||
| .size(24.dp) | ||
| .clickable { onDismiss() } | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
닫기 아이콘 터치 타깃(24dp) → 48dp로 확대 + IconButton로 전환 제안
접근성(최소 48x48dp) 미준수로 터치 미스 가능성이 큽니다. IconButton 사용 시 최소 터치 영역과 역할/리플이 자동 적용됩니다. 아래처럼 교체를 권장합니다. 상태바 영역과의 겹침을 피하려면 statusBarsPadding도 함께 적용해 주세요.
- Box(
- modifier = Modifier
- .align(Alignment.TopStart)
- .padding(horizontal = 20.dp, vertical = 16.dp)
- .zIndex(1f)
- ) {
- Icon(
- painter = painterResource(R.drawable.ic_arrow_back),
- contentDescription = "닫기",
- tint = colors.White,
- modifier = Modifier
- .size(24.dp)
- .clickable { onDismiss() }
- )
- }
+ Box(
+ modifier = Modifier
+ .align(Alignment.TopStart)
+ .statusBarsPadding()
+ .padding(horizontal = 16.dp, vertical = 12.dp)
+ .zIndex(1f)
+ ) {
+ IconButton(
+ onClick = onDismiss,
+ modifier = Modifier.size(48.dp)
+ ) {
+ Icon(
+ painter = painterResource(R.drawable.ic_arrow_back),
+ contentDescription = stringResource(R.string.cd_close),
+ tint = colors.White,
+ modifier = Modifier.size(24.dp)
+ )
+ }
+ }추가로 필요한 import:
import androidx.compose.material3.IconButton
import androidx.compose.foundation.layout.statusBarsPadding가독성(밝은 이미지 위) 개선이 필요하면 아이콘 뒤에 얇은 원형 스크림을 추가하는 것도 고려해 주세요(예: 배경 colors.Black.copy(alpha = 0.3f)).
| LazyColumn( | ||
| state = listState, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .drawVerticalScrollbar(rememberScrollState()) | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
스크롤바가 리스트 스크롤 상태와 동기화되지 않을 수 있습니다.
LazyColumn을 쓰면서 drawVerticalScrollbar(rememberScrollState())를 전달하면, 스크롤바가 별도 ScrollState에 묶여 실제 스크롤을 반영하지 못할 가능성이 큽니다. LazyListState를 넘기도록 수정하는 것을 권장합니다.
다음과 같이 교체를 제안합니다(유틸에 LazyListState 오버로드가 없다면 추가 필요):
- LazyColumn(
- state = listState,
- modifier = Modifier
- .fillMaxWidth()
- .drawVerticalScrollbar(rememberScrollState())
- ) {
+ LazyColumn(
+ state = listState,
+ modifier = Modifier
+ .fillMaxWidth()
+ .drawVerticalScrollbar(listState)
+ ) {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt
around lines 56-61, the LazyColumn is passing rememberScrollState() into
drawVerticalScrollbar which can desync the visible scrollbar from the
LazyColumn's LazyListState; replace the call to
drawVerticalScrollbar(rememberScrollState()) with
drawVerticalScrollbar(listState) so the scrollbar is driven by the same
LazyListState used by the LazyColumn, and if the drawVerticalScrollbar utility
lacks a LazyListState overload add one that accepts LazyListState and derives
the required scroll metrics from it.
| else -> { | ||
| Column(Modifier.padding(horizontal = 20.dp)) { | ||
| GroupBookListWithScrollbar( | ||
| books = displayBooks, | ||
| onBookClick = onBookSelect | ||
| ) | ||
| when { | ||
| searchText.isNotEmpty() -> { | ||
| GroupBookListWithScrollbar( | ||
| books = displayBooks, | ||
| onBookClick = onBookSelect, | ||
| isLoadingMore = isLoadingMoreSearch, | ||
| hasMore = hasMoreSearch, | ||
| onLoadMore = onLoadMoreSearch | ||
| ) | ||
| } | ||
| selectedTab == 0 -> { | ||
| GroupBookListWithScrollbar( | ||
| books = displayBooks, | ||
| onBookClick = onBookSelect, | ||
| isLoadingMore = isLoadingMoreSaved, | ||
| hasMore = hasMoreSaved, | ||
| onLoadMore = onLoadMoreSaved | ||
| ) | ||
| } | ||
| else -> { | ||
| GroupBookListWithScrollbar( | ||
| books = displayBooks, | ||
| onBookClick = onBookSelect, | ||
| isLoadingMore = isLoadingMoreGroup, | ||
| hasMore = hasMoreGroup, | ||
| onLoadMore = onLoadMoreGroup | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
검색 진행 중 로딩 상태 미노출 → 검색 스피너 분기 추가 제안
현재 isSearching이 true이고 검색어가 존재할 때도 리스트 컬럼만 렌더링되어, 사용자가 “검색 중” 피드백을 받지 못합니다. isLoading 분기 바로 다음에 “검색 중” 분기를 추가해 주세요.
제안 diff:
when {
isLoading -> {
Box(
modifier = Modifier
.fillMaxWidth()
.align(Alignment.CenterHorizontally),
contentAlignment = Alignment.Center
) {
CircularProgressIndicator()
}
}
+ searchText.isNotEmpty() && isSearching -> {
+ Box(
+ modifier = Modifier
+ .fillMaxWidth()
+ .align(Alignment.CenterHorizontally),
+ contentAlignment = Alignment.Center
+ ) {
+ CircularProgressIndicator()
+ }
+ }
else -> {
Column(Modifier.padding(horizontal = 20.dp)) {
when {
searchText.isNotEmpty() -> {
GroupBookListWithScrollbar(
books = displayBooks,
onBookClick = onBookSelect,
isLoadingMore = isLoadingMoreSearch,
hasMore = hasMoreSearch,
onLoadMore = onLoadMoreSearch
)
}
selectedTab == 0 -> {
GroupBookListWithScrollbar(
books = displayBooks,
onBookClick = onBookSelect,
isLoadingMore = isLoadingMoreSaved,
hasMore = hasMoreSaved,
onLoadMore = onLoadMoreSaved
)
}
else -> {
GroupBookListWithScrollbar(
books = displayBooks,
onBookClick = onBookSelect,
isLoadingMore = isLoadingMoreGroup,
hasMore = hasMoreGroup,
onLoadMore = onLoadMoreGroup
)
}
}
}
}
}추가로, 세 분기에서 동일 컴포저블을 반복 호출하므로, 분기별로 isLoadingMore/hasMore/onLoadMore만 추출해 단일 호출로 합치는 것도 가독성에 도움이 됩니다.
📝 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.
| else -> { | |
| Column(Modifier.padding(horizontal = 20.dp)) { | |
| GroupBookListWithScrollbar( | |
| books = displayBooks, | |
| onBookClick = onBookSelect | |
| ) | |
| when { | |
| searchText.isNotEmpty() -> { | |
| GroupBookListWithScrollbar( | |
| books = displayBooks, | |
| onBookClick = onBookSelect, | |
| isLoadingMore = isLoadingMoreSearch, | |
| hasMore = hasMoreSearch, | |
| onLoadMore = onLoadMoreSearch | |
| ) | |
| } | |
| selectedTab == 0 -> { | |
| GroupBookListWithScrollbar( | |
| books = displayBooks, | |
| onBookClick = onBookSelect, | |
| isLoadingMore = isLoadingMoreSaved, | |
| hasMore = hasMoreSaved, | |
| onLoadMore = onLoadMoreSaved | |
| ) | |
| } | |
| else -> { | |
| GroupBookListWithScrollbar( | |
| books = displayBooks, | |
| onBookClick = onBookSelect, | |
| isLoadingMore = isLoadingMoreGroup, | |
| hasMore = hasMoreGroup, | |
| onLoadMore = onLoadMoreGroup | |
| ) | |
| } | |
| } | |
| when { | |
| isLoading -> { | |
| Box( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .align(Alignment.CenterHorizontally), | |
| contentAlignment = Alignment.Center | |
| ) { | |
| CircularProgressIndicator() | |
| } | |
| } | |
| searchText.isNotEmpty() && isSearching -> { | |
| Box( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .align(Alignment.CenterHorizontally), | |
| contentAlignment = Alignment.Center | |
| ) { | |
| CircularProgressIndicator() | |
| } | |
| } | |
| else -> { | |
| Column(Modifier.padding(horizontal = 20.dp)) { | |
| when { | |
| searchText.isNotEmpty() -> { | |
| GroupBookListWithScrollbar( | |
| books = displayBooks, | |
| onBookClick = onBookSelect, | |
| isLoadingMore = isLoadingMoreSearch, | |
| hasMore = hasMoreSearch, | |
| onLoadMore = onLoadMoreSearch | |
| ) | |
| } | |
| selectedTab == 0 -> { | |
| GroupBookListWithScrollbar( | |
| books = displayBooks, | |
| onBookClick = onBookSelect, | |
| isLoadingMore = isLoadingMoreSaved, | |
| hasMore = hasMoreSaved, | |
| onLoadMore = onLoadMoreSaved | |
| ) | |
| } | |
| else -> { | |
| GroupBookListWithScrollbar( | |
| books = displayBooks, | |
| onBookClick = onBookSelect, | |
| isLoadingMore = isLoadingMoreGroup, | |
| hasMore = hasMoreGroup, | |
| onLoadMore = onLoadMoreGroup | |
| ) | |
| } | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt
around lines 121 to 151, add an explicit isSearching branch so that when
isSearching == true and searchText.isNotEmpty() a loading UI (e.g.,
spinner/ProgressIndicator) is shown instead of rendering the list; then refactor
the three current branches that call GroupBookListWithScrollbar into a single
call by first selecting the appropriate isLoadingMore, hasMore and onLoadMore
values based on which logical branch applies (search, saved tab, or group tab)
and pass those computed values to one GroupBookListWithScrollbar call so you
both display the search spinner during active searches and remove duplicated
composable invocations.
| fun loadSavedBooks(isInitial: Boolean = false) { | ||
| if (isLoadingSavedBooks) return | ||
| if (!isInitial && _uiState.value.isLastSavedBooks) return | ||
|
|
||
| viewModelScope.launch { | ||
| updateState { it.copy(isLoadingBooks = true) } | ||
| try { | ||
| val savedBooksResult = bookRepository.getBooks("SAVED") | ||
| savedBooksResult.onSuccess { response -> | ||
| updateState { | ||
| it.copy(savedBooks = response?.bookList?.map { dto -> dto.toBookData() } | ||
| ?: emptyList()) | ||
| } | ||
| }.onFailure { | ||
| updateState { it.copy(savedBooks = emptyList()) } | ||
| isLoadingSavedBooks = true | ||
|
|
||
| if (isInitial) { | ||
| updateState { it.copy(savedBooks = emptyList(), isLastSavedBooks = false) } | ||
| savedBooksCursor = null | ||
| } else { | ||
| updateState { it.copy(isLoadingMoreSavedBooks = true) } | ||
| } | ||
|
|
||
| val groupBooksResult = bookRepository.getBooks("JOINING") | ||
| groupBooksResult.onSuccess { response -> | ||
| updateState { | ||
| it.copy(groupBooks = response?.bookList?.map { dto -> dto.toBookData() } | ||
| ?: emptyList()) | ||
| val cursor = if (isInitial) null else savedBooksCursor | ||
|
|
||
| bookRepository.getBooks("SAVED", cursor) | ||
| .onSuccess { response -> | ||
| if (response != null) { | ||
| val currentList = if (isInitial) emptyList() else _uiState.value.savedBooks | ||
| val newBooks = response.bookList.map { it.toBookData() } | ||
| updateState { | ||
| it.copy( | ||
| savedBooks = currentList + newBooks, | ||
| isLastSavedBooks = response.isLast | ||
| ) | ||
| } | ||
| savedBooksCursor = response.nextCursor | ||
| } else { | ||
| updateState { it.copy(isLastSavedBooks = true) } | ||
| } | ||
| } | ||
| }.onFailure { | ||
| updateState { it.copy(groupBooks = emptyList()) } | ||
| .onFailure { exception -> | ||
| if (isInitial) { | ||
| updateState { it.copy(savedBooks = emptyList()) } | ||
| } | ||
| } | ||
| } finally { | ||
| isLoadingSavedBooks = false | ||
| updateState { | ||
| it.copy( | ||
| isLoadingBooks = if (isInitial && !isLoadingGroupBooks) false else it.isLoadingBooks, | ||
| isLoadingMoreSavedBooks = false | ||
| ) | ||
| } | ||
| } catch (e: Exception) { | ||
| updateState { it.copy(savedBooks = emptyList(), groupBooks = emptyList()) } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Saved 목록 로딩: 예외 처리 신호와 로깅 보강
초기/추가 로딩 분기와 커서 처리 모두 적절합니다. 다만 onFailure에서 예외를 무시하고 있어 문제 추적이 어렵습니다. 최소한 로그 남기기를 권장합니다.
제안 diff:
- .onFailure { exception ->
- if (isInitial) {
- updateState { it.copy(savedBooks = emptyList()) }
- }
- }
+ .onFailure { exception ->
+ if (isInitial) {
+ updateState { it.copy(savedBooks = emptyList()) }
+ }
+ android.util.Log.e("GroupMakeRoomVM", "loadSavedBooks failed", exception)
+ }📝 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.
| fun loadSavedBooks(isInitial: Boolean = false) { | |
| if (isLoadingSavedBooks) return | |
| if (!isInitial && _uiState.value.isLastSavedBooks) return | |
| viewModelScope.launch { | |
| updateState { it.copy(isLoadingBooks = true) } | |
| try { | |
| val savedBooksResult = bookRepository.getBooks("SAVED") | |
| savedBooksResult.onSuccess { response -> | |
| updateState { | |
| it.copy(savedBooks = response?.bookList?.map { dto -> dto.toBookData() } | |
| ?: emptyList()) | |
| } | |
| }.onFailure { | |
| updateState { it.copy(savedBooks = emptyList()) } | |
| isLoadingSavedBooks = true | |
| if (isInitial) { | |
| updateState { it.copy(savedBooks = emptyList(), isLastSavedBooks = false) } | |
| savedBooksCursor = null | |
| } else { | |
| updateState { it.copy(isLoadingMoreSavedBooks = true) } | |
| } | |
| val groupBooksResult = bookRepository.getBooks("JOINING") | |
| groupBooksResult.onSuccess { response -> | |
| updateState { | |
| it.copy(groupBooks = response?.bookList?.map { dto -> dto.toBookData() } | |
| ?: emptyList()) | |
| val cursor = if (isInitial) null else savedBooksCursor | |
| bookRepository.getBooks("SAVED", cursor) | |
| .onSuccess { response -> | |
| if (response != null) { | |
| val currentList = if (isInitial) emptyList() else _uiState.value.savedBooks | |
| val newBooks = response.bookList.map { it.toBookData() } | |
| updateState { | |
| it.copy( | |
| savedBooks = currentList + newBooks, | |
| isLastSavedBooks = response.isLast | |
| ) | |
| } | |
| savedBooksCursor = response.nextCursor | |
| } else { | |
| updateState { it.copy(isLastSavedBooks = true) } | |
| } | |
| } | |
| }.onFailure { | |
| updateState { it.copy(groupBooks = emptyList()) } | |
| .onFailure { exception -> | |
| if (isInitial) { | |
| updateState { it.copy(savedBooks = emptyList()) } | |
| } | |
| } | |
| } finally { | |
| isLoadingSavedBooks = false | |
| updateState { | |
| it.copy( | |
| isLoadingBooks = if (isInitial && !isLoadingGroupBooks) false else it.isLoadingBooks, | |
| isLoadingMoreSavedBooks = false | |
| ) | |
| } | |
| } catch (e: Exception) { | |
| updateState { it.copy(savedBooks = emptyList(), groupBooks = emptyList()) } | |
| } | |
| } | |
| fun loadSavedBooks(isInitial: Boolean = false) { | |
| if (isLoadingSavedBooks) return | |
| if (!isInitial && _uiState.value.isLastSavedBooks) return | |
| viewModelScope.launch { | |
| try { | |
| isLoadingSavedBooks = true | |
| if (isInitial) { | |
| updateState { it.copy(savedBooks = emptyList(), isLastSavedBooks = false) } | |
| savedBooksCursor = null | |
| } else { | |
| updateState { it.copy(isLoadingMoreSavedBooks = true) } | |
| } | |
| val cursor = if (isInitial) null else savedBooksCursor | |
| bookRepository.getBooks("SAVED", cursor) | |
| .onSuccess { response -> | |
| if (response != null) { | |
| val currentList = if (isInitial) emptyList() else _uiState.value.savedBooks | |
| val newBooks = response.bookList.map { it.toBookData() } | |
| updateState { | |
| it.copy( | |
| savedBooks = currentList + newBooks, | |
| isLastSavedBooks = response.isLast | |
| ) | |
| } | |
| savedBooksCursor = response.nextCursor | |
| } else { | |
| updateState { it.copy(isLastSavedBooks = true) } | |
| } | |
| } | |
| .onFailure { exception -> | |
| if (isInitial) { | |
| updateState { it.copy(savedBooks = emptyList()) } | |
| } | |
| android.util.Log.e("GroupMakeRoomVM", "loadSavedBooks failed", exception) | |
| } | |
| } finally { | |
| isLoadingSavedBooks = false | |
| updateState { | |
| it.copy( | |
| isLoadingBooks = if (isInitial && !isLoadingGroupBooks) false else it.isLoadingBooks, | |
| isLoadingMoreSavedBooks = false | |
| ) | |
| } | |
| } | |
| } | |
| } |
| fun loadMoreSearchResults() { | ||
| val currentState = _uiState.value | ||
| if (currentState.isLoadingMoreSearchResults || | ||
| currentState.isSearching || | ||
| currentState.isLastSearchPage || | ||
| currentState.searchResults.isEmpty() || | ||
| currentState.currentSearchQuery.isBlank()) { | ||
| return | ||
| } | ||
|
|
||
| loadMoreSearchJob?.cancel() | ||
| loadMoreSearchJob = viewModelScope.launch { | ||
| updateState { it.copy(isLoadingMoreSearchResults = true) } | ||
|
|
||
| try { | ||
| val nextPage = currentState.searchPage + 1 | ||
| val result = bookRepository.searchBooks( | ||
| currentState.currentSearchQuery, | ||
| page = nextPage, | ||
| isFinalized = false | ||
| ) | ||
| result.onSuccess { response -> | ||
| if (response != null) { | ||
| val newResults = response.searchResult.map { it.toBookData() } | ||
| updateState { | ||
| it.copy( | ||
| searchResults = currentState.searchResults + newResults, | ||
| searchPage = response.page, | ||
| isLastSearchPage = response.last, | ||
| isLoadingMoreSearchResults = false | ||
| ) | ||
| } | ||
| } else { | ||
| updateState { | ||
| it.copy( | ||
| isLoadingMoreSearchResults = false, | ||
| isLastSearchPage = true | ||
| ) | ||
| } | ||
| } | ||
| }.onFailure { | ||
| updateState { | ||
| it.copy(isLoadingMoreSearchResults = false) | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| updateState { | ||
| it.copy(isLoadingMoreSearchResults = false) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
loadMoreSearchResults: 캡처된 스냅샷 사용으로 인한 상태 경합 가능성
currentState를 함수 시작 시 캡처해 그 값을 기준으로 nextPage/append를 수행하고 있어, 코루틴 실행 시점에 상태가 변해도 반영되지 않습니다. updateState 람다의 it(최신 상태)를 기반으로 계산하도록 변경해 주세요. 또한 detekt 경고대로 예외 로깅을 추가하세요.
제안 diff:
- fun loadMoreSearchResults() {
- val currentState = _uiState.value
- if (currentState.isLoadingMoreSearchResults ||
- currentState.isSearching ||
- currentState.isLastSearchPage ||
- currentState.searchResults.isEmpty() ||
- currentState.currentSearchQuery.isBlank()) {
- return
- }
-
- loadMoreSearchJob?.cancel()
- loadMoreSearchJob = viewModelScope.launch {
- updateState { it.copy(isLoadingMoreSearchResults = true) }
-
- try {
- val nextPage = currentState.searchPage + 1
- val result = bookRepository.searchBooks(
- currentState.currentSearchQuery,
- page = nextPage,
- isFinalized = false
- )
- result.onSuccess { response ->
- if (response != null) {
- val newResults = response.searchResult.map { it.toBookData() }
- updateState {
- it.copy(
- searchResults = currentState.searchResults + newResults,
- searchPage = response.page,
- isLastSearchPage = response.last,
- isLoadingMoreSearchResults = false
- )
- }
- } else {
- updateState {
- it.copy(
- isLoadingMoreSearchResults = false,
- isLastSearchPage = true
- )
- }
- }
- }.onFailure {
- updateState {
- it.copy(isLoadingMoreSearchResults = false)
- }
- }
- } catch (e: Exception) {
- updateState {
- it.copy(isLoadingMoreSearchResults = false)
- }
- }
- }
- }
+ fun loadMoreSearchResults() {
+ val snapshot = _uiState.value
+ if (snapshot.isLoadingMoreSearchResults ||
+ snapshot.isSearching ||
+ snapshot.isLastSearchPage ||
+ snapshot.searchResults.isEmpty() ||
+ snapshot.currentSearchQuery.isBlank()
+ ) return
+
+ loadMoreSearchJob?.cancel()
+ loadMoreSearchJob = viewModelScope.launch {
+ updateState { it.copy(isLoadingMoreSearchResults = true) }
+ try {
+ val nextPage = _uiState.value.searchPage + 1
+ val query = _uiState.value.currentSearchQuery
+ val result = bookRepository.searchBooks(query, page = nextPage, isFinalized = false)
+ result.onSuccess { response ->
+ if (response != null) {
+ val newResults = response.searchResult.map { it.toBookData() }
+ updateState { prev ->
+ prev.copy(
+ searchResults = prev.searchResults + newResults,
+ searchPage = response.page,
+ isLastSearchPage = response.last,
+ isLoadingMoreSearchResults = false
+ )
+ }
+ } else {
+ updateState { it.copy(isLoadingMoreSearchResults = false, isLastSearchPage = true) }
+ }
+ }.onFailure { e ->
+ android.util.Log.e("GroupMakeRoomVM", "loadMoreSearchResults failed", e)
+ updateState { it.copy(isLoadingMoreSearchResults = false) }
+ }
+ } catch (e: Exception) {
+ android.util.Log.e("GroupMakeRoomVM", "loadMoreSearchResults exception", e)
+ updateState { it.copy(isLoadingMoreSearchResults = false) }
+ }
+ }
+ }📝 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.
| fun loadMoreSearchResults() { | |
| val currentState = _uiState.value | |
| if (currentState.isLoadingMoreSearchResults || | |
| currentState.isSearching || | |
| currentState.isLastSearchPage || | |
| currentState.searchResults.isEmpty() || | |
| currentState.currentSearchQuery.isBlank()) { | |
| return | |
| } | |
| loadMoreSearchJob?.cancel() | |
| loadMoreSearchJob = viewModelScope.launch { | |
| updateState { it.copy(isLoadingMoreSearchResults = true) } | |
| try { | |
| val nextPage = currentState.searchPage + 1 | |
| val result = bookRepository.searchBooks( | |
| currentState.currentSearchQuery, | |
| page = nextPage, | |
| isFinalized = false | |
| ) | |
| result.onSuccess { response -> | |
| if (response != null) { | |
| val newResults = response.searchResult.map { it.toBookData() } | |
| updateState { | |
| it.copy( | |
| searchResults = currentState.searchResults + newResults, | |
| searchPage = response.page, | |
| isLastSearchPage = response.last, | |
| isLoadingMoreSearchResults = false | |
| ) | |
| } | |
| } else { | |
| updateState { | |
| it.copy( | |
| isLoadingMoreSearchResults = false, | |
| isLastSearchPage = true | |
| ) | |
| } | |
| } | |
| }.onFailure { | |
| updateState { | |
| it.copy(isLoadingMoreSearchResults = false) | |
| } | |
| } | |
| } catch (e: Exception) { | |
| updateState { | |
| it.copy(isLoadingMoreSearchResults = false) | |
| } | |
| } | |
| } | |
| } | |
| fun loadMoreSearchResults() { | |
| val snapshot = _uiState.value | |
| if (snapshot.isLoadingMoreSearchResults || | |
| snapshot.isSearching || | |
| snapshot.isLastSearchPage || | |
| snapshot.searchResults.isEmpty() || | |
| snapshot.currentSearchQuery.isBlank() | |
| ) return | |
| loadMoreSearchJob?.cancel() | |
| loadMoreSearchJob = viewModelScope.launch { | |
| updateState { it.copy(isLoadingMoreSearchResults = true) } | |
| try { | |
| val nextPage = _uiState.value.searchPage + 1 | |
| val query = _uiState.value.currentSearchQuery | |
| val result = bookRepository.searchBooks(query, page = nextPage, isFinalized = false) | |
| result.onSuccess { response -> | |
| if (response != null) { | |
| val newResults = response.searchResult.map { it.toBookData() } | |
| updateState { prev -> | |
| prev.copy( | |
| searchResults = prev.searchResults + newResults, | |
| searchPage = response.page, | |
| isLastSearchPage = response.last, | |
| isLoadingMoreSearchResults = false | |
| ) | |
| } | |
| } else { | |
| updateState { | |
| it.copy( | |
| isLoadingMoreSearchResults = false, | |
| isLastSearchPage = true | |
| ) | |
| } | |
| } | |
| }.onFailure { e -> | |
| android.util.Log.e("GroupMakeRoomVM", "loadMoreSearchResults failed", e) | |
| updateState { it.copy(isLoadingMoreSearchResults = false) } | |
| } | |
| } catch (e: Exception) { | |
| android.util.Log.e("GroupMakeRoomVM", "loadMoreSearchResults exception", e) | |
| updateState { it.copy(isLoadingMoreSearchResults = false) } | |
| } | |
| } | |
| } |
🧰 Tools
🪛 detekt (1.23.8)
[warning] 346-346: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
| import com.texthip.thip.ui.mypage.mock.BookItem | ||
| import com.texthip.thip.ui.mypage.mock.FeedItem |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
mock 타입 의존 여부 확인
com.texthip.thip.ui.mypage.mock.* 타입을 프로덕션 컴포저블의 파라미터로 직접 사용하면 계층 오염/의존성이 생깁니다. 실제 UI 모델 패키지로 대체되었는지 확인하거나, mock은 Preview 전용으로 한정하는 구성이 안전합니다.
검증 스크립트:
🏁 Script executed:
#!/bin/bash
# FeedItem/BookItem의 실제 정의 위치 파악
rg -nP --type=kotlin 'data class\s+(FeedItem|BookItem)\b' -C2
rg -nP --type=kotlin '\bFeedItem\b|\bBookItem\b' app/src/main/java/com/texthip/thip/ui/mypage | sed -n '1,120p'Length of output: 4049
mock 타입 의존 제거 (MypageSaveScreen.kt:36-37)
- 프로덕션 컴포저블에서
com.texthip.thip.ui.mypage.mock.BookItem/FeedItem에 직접 의존하고 있습니다. 실제 UI 모델 패키지 타입으로 교체하거나, mock은@Preview전용으로 분리하세요.
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageSaveScreen.kt
around lines 36-37, the file imports and depends on mock types
com.texthip.thip.ui.mypage.mock.BookItem and FeedItem; replace those mock
imports with the real production UI model types (e.g., the actual
BookItem/FeedItem from the UI model package) or refactor the mock types into a
Preview-only source (move them into a @Preview file or a test-only package) and
update all usages in this composable to use the production models (or add
mapping from production models to preview mocks only in preview code). Ensure no
references to com.texthip.thip.ui.mypage.mock remain in production code and
update imports/usages accordingly.
| fun loadSavedBooks(isInitial: Boolean = true) { | ||
| if (isLoadingBooks && !isInitial) return | ||
| if (_uiState.value.isLast && !isInitial) return | ||
|
|
||
| fun loadSavedBooks() { | ||
| viewModelScope.launch { | ||
| bookRepository.getSavedBooks() | ||
| .onSuccess { response -> | ||
| _books.value = response?.bookList?.map { it.toBookItem() } ?: emptyList() | ||
| } | ||
| .onFailure { | ||
| it.printStackTrace() | ||
| try { | ||
| isLoadingBooks = true | ||
|
|
||
| if (isInitial) { | ||
| updateState { it.copy(isLoading = true, books = emptyList(), isLast = false) } | ||
| nextCursor = null | ||
| } else { | ||
| updateState { it.copy(isLoadingMore = true) } | ||
| } | ||
|
|
||
| val cursor = if (isInitial) null else nextCursor | ||
|
|
||
| bookRepository.getSavedBooks(cursor) | ||
| .onSuccess { response -> | ||
| if (response != null) { | ||
| val currentList = if (isInitial) emptyList() else _uiState.value.books | ||
| val newBooks = response.bookList.map { it.toBookItem() } | ||
| updateState { | ||
| it.copy( | ||
| books = currentList + newBooks, | ||
| error = null, | ||
| isLast = response.isLast | ||
| ) | ||
| } | ||
| nextCursor = response.nextCursor | ||
| } else { | ||
| updateState { it.copy(isLast = true) } | ||
| } | ||
| } | ||
| .onFailure { exception -> | ||
| updateState { it.copy(error = exception.message) } | ||
| exception.printStackTrace() | ||
| } | ||
| } finally { | ||
| isLoadingBooks = false | ||
| updateState { it.copy(isLoading = false, isLoadingMore = false) } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
로드 동시성 가드 보강: 초기/추가 로딩 중복 실행 가능성
현재 가드가 if (isLoadingBooks && !isInitial) return으 로직이라, 이미 추가 로딩 중일 때 초기 로딩 호출이 들어오면 중복 실행될 수 있습니다(역시 반대 케이스도). 단일 인플라이트만 허용하도록 가드를 단순화하거나, 기존 Job을 취소 후 재시작하세요.
제안 diff(단일 Job 관리):
+ private var loadJob: kotlinx.coroutines.Job? = null
@@
- fun loadSavedBooks(isInitial: Boolean = true) {
- if (isLoadingBooks && !isInitial) return
- if (_uiState.value.isLast && !isInitial) return
+ fun loadSavedBooks(isInitial: Boolean = true) {
+ if (isLoadingBooks) return
+ if (!isInitial && _uiState.value.isLast) return
@@
- viewModelScope.launch {
+ loadJob?.cancel()
+ loadJob = viewModelScope.launch {
try {
isLoadingBooks = true또는 보다 단순하게는 첫 줄을 if (isLoadingBooks) return으로 바꾸는 것만으로도 중복 호출을 방지할 수 있습니다.
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/SavedBookViewModel.kt
around lines 39 to 82, the current concurrency guard (if (isLoadingBooks &&
!isInitial) return) allows overlapping initial vs additional loads; change the
guard to enforce a single in-flight load by either replacing it with a simple if
(isLoadingBooks) return to block any new load while one is active, or manage a
single Job reference: cancel the existing Job before launching a new one and
store the new Job so only one coroutine runs at a time; ensure isLoadingBooks is
set/cleared consistently around job start/finish and retain the existing state
updates in the try/finally.
There was a problem hiding this comment.
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/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
153-169: 버그: awaitAll이 실작업을 기다리지 않음(내부에서 launch 사용으로 조기 종료)async { loadUserName() } / async { loadRoomSections() }는 각각 내부에서 launch를 다시 호출하므로 awaitAll이 실제 네트워크 작업 완료를 기다리지 않습니다. 그 결과 isRefreshing이 너무 빨리 false가 됩니다. 두 함수는 suspend로 바꾸고 refresh에서 직접 대기하도록 수정하세요.
fun refreshGroupData() { viewModelScope.launch { updateState { it.copy(isRefreshing = true) } try { - val jobs = listOf( - async { loadUserName() }, - async { - resetMyGroupsData() - loadMyGroupsSuspend(isInitial = true) - }, - async { loadRoomSections() }, - ) - - jobs.awaitAll() + kotlinx.coroutines.coroutineScope { + awaitAll( + async { loadUserNameSuspend() }, + async { + resetMyGroupsData() + loadMyGroupsSuspend(isInitial = true) + }, + async { loadRoomSectionsSuspend() }, + ) + } } finally { updateState { it.copy(isRefreshing = false) } } } }추가: loadUserName / loadRoomSections를 suspend로 변경(파일 내 다른 위치)
// 기존 loadUserName 대체 private suspend fun loadUserNameSuspend() { repository.getUserName() .onSuccess { userName -> updateState { it.copy(userName = userName) } } } // 기존 loadRoomSections 대체 private suspend fun loadRoomSectionsSuspend() { updateState { it.copy(roomSectionsError = null) } val genresResult = repository.getGenres() val selectedIndex = uiState.value.selectedGenreIndex val selectedGenre = if (genresResult.isSuccess) { val genres = genresResult.getOrThrow() if (selectedIndex >= 0 && selectedIndex < genres.size) genres[selectedIndex] else genres.firstOrNull() ?: Genre.getDefault() } else { Genre.getDefault() } repository.getRoomSections(selectedGenre) .onSuccess { roomMainList -> updateState { it.copy(roomMainList = roomMainList) } } .onFailure { error -> updateState { it.copy(roomSectionsError = error.message) } } }
♻️ Duplicate comments (1)
app/src/main/java/com/texthip/thip/data/model/rooms/response/JoinedRoomListResponse.kt (1)
11-12: 커서/종료 여부의 UI 진리 소스 일원화nextCursor와 isLast를 모두 노출하는 건 OK입니다만, UI 게이트(“더 불러오기 가능 여부”)는 하나로만 판단하도록 팀 기준을 고정해 주세요(권장: isLast만 사용). 이전 코멘트와 동일 맥락이라 재확인 차 남깁니다.
🧹 Nitpick comments (3)
app/src/main/java/com/texthip/thip/data/model/rooms/response/JoinedRoomListResponse.kt (1)
11-12: isLast 직렬화 호환성(서버 필드 누락 시) 안전장치 추가 제안서버/앱 배포 타이밍이 어긋날 때 isLast가 누락되면 kotlinx.serialization에서 디코딩 예외가 납니다. 기본값을 지정해 방어하는 편이 안전합니다.
- @SerialName("isLast") val isLast: Boolean + @SerialName("isLast") val isLast: Boolean = falseapp/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (2)
78-92: nextCursor 누락 대비 방어 로직 추가서버가 isLast=false인데 nextCursor=null을 반환하는 장애 케이스에 대비해 방어 로직을 추가하세요(로그 남기고 isLast=true로 전환 혹은 재시도).
.onSuccess { joinedRoomsResponse -> if (joinedRoomsResponse != null) { val currentList = if (isInitial) emptyList() else _uiState.value.myJoinedRooms updateState { it.copy( myJoinedRooms = currentList + joinedRoomsResponse.roomList, hasMoreMyGroups = !joinedRoomsResponse.isLast, isLast = joinedRoomsResponse.isLast ) } - nextCursor = joinedRoomsResponse.nextCursor + nextCursor = joinedRoomsResponse.nextCursor + if (!joinedRoomsResponse.isLast && joinedRoomsResponse.nextCursor == null) { + // 방어: 불일치 발견 시 더 로드 불가로 전환 + updateState { it.copy(isLast = true, hasMoreMyGroups = false) } + } } else {
105-111: 게이트 조건 단일화(권장: isLast만 사용)중복 상태(hasMoreMyGroups, isLast)를 함께 쓰면 드리프트 가능성이 있습니다. 게이트는 isLast 하나로만 판단하세요.
- if (_uiState.value.hasMoreMyGroups && !isLoadingMyGroups) { + if (!_uiState.value.isLast && !isLoadingMyGroups) { viewModelScope.launch { loadMyGroupsSuspend(isInitial = false) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/src/main/java/com/texthip/thip/data/model/rooms/response/JoinedRoomListResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt(4 hunks)
🔇 Additional comments (2)
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (2)
27-29: 중복 호출 방지 가드 추가 LGTMnextCursor와 isLoadingMyGroups로 재진입을 막는 접근 적절합니다.
65-77: 초기/추가 로딩 분기 깔끔isInitial 플래그로 초기화/추가 로딩을 분리한 구성 명확합니다.
There was a problem hiding this comment.
Actionable comments posted: 3
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/texthip/thip/data/di/NetworkModule.kt (1)
55-58: 로그에 Authorization/쿠키/프리사인 URL 노출 위험 — 헤더 마스킹 및 릴리즈 빌드 로깅 비활성화 권장
BODY레벨은 헤더/바디를 모두 출력하므로 토큰 및 프리사인 쿼리스트링이 노출될 수 있습니다. 릴리즈에서는NONE으로, 공통 민감 헤더는 마스킹하세요.@Provides @Singleton fun providesLoggingInterceptor(): HttpLoggingInterceptor = - HttpLoggingInterceptor().apply { - level = HttpLoggingInterceptor.Level.BODY - } + HttpLoggingInterceptor().apply { + level = if (BuildConfig.DEBUG) { + HttpLoggingInterceptor.Level.BODY + } else { + HttpLoggingInterceptor.Level.NONE + } + redactHeader("Authorization") + redactHeader("Cookie") + redactHeader("X-Api-Key") + }
🧹 Nitpick comments (7)
app/src/main/java/com/texthip/thip/data/model/feed/request/CreateFeedRequest.kt (1)
15-17: 빈 리스트 직렬화로 인한 서버 스펙 호환성 확인 + 속성 단위 제어 제안NetworkModule에서
encodeDefaults = true설정이므로,tagList/imageUrls가 비어도[]로 항상 전송됩니다. 서버가 “필드 생략(null/미포함)”과 “빈 배열([])”을 구분해 검증한다면 400이 날 수 있습니다. 필요 시 해당 필드만 기본값 인코딩을 비활성화하세요.import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable +import kotlinx.serialization.EncodeDefault @@ - @SerialName("tagList") - val tagList: List<String> = emptyList(), + @EncodeDefault(EncodeDefault.Mode.NEVER) + @SerialName("tagList") + val tagList: List<String> = emptyList(), - @SerialName("imageUrls") - val imageUrls: List<String> = emptyList() + @EncodeDefault(EncodeDefault.Mode.NEVER) + @SerialName("imageUrls") + val imageUrls: List<String> = emptyList()서버가 빈 배열을 허용한다면 현재 구현 유지로 충분합니다. 확인만 부탁드립니다.
app/src/main/java/com/texthip/thip/data/model/feed/response/PresignedUrlResponse.kt (1)
1-18: 응답 모델 정의 깔끔합니다 — 필드 의미를 KDoc으로 명시하면 추후 유지보수에 도움됩니다
presignedUrl(업로드에 사용하는 일회성 URL)과fileUrl(업로드 완료 후 컨텐츠에서 사용할 공개 URL)의 의미를 KDoc 주석으로 남겨두면 활용 맥락 파악이 빨라집니다. 기능에는 영향 없습니다.app/src/main/java/com/texthip/thip/data/di/NetworkModule.kt (2)
69-70:requireNotNull불필요
"application/json".toMediaType()는 null을 반환하지 않으므로requireNotNull래핑이 군더더기입니다.- json.asConverterFactory(requireNotNull("application/json".toMediaType())) + json.asConverterFactory("application/json".toMediaType())
45-51: 네트워크 지연 대비callTimeout추가 고려(선택)대부분의 API는 10초로 충분하지만, 간헐적 지연이나 서버 측 처리시간 증가에 대비해 전체 호출 상한을 명시해두면 안전합니다. (업로드는 별도 헬퍼를 쓰더라도, API 호출 측은 보호됨)
OkHttpClient.Builder().apply { connectTimeout(10, TimeUnit.SECONDS) writeTimeout(10, TimeUnit.SECONDS) readTimeout(10, TimeUnit.SECONDS) + callTimeout(15, TimeUnit.SECONDS) addInterceptor(loggingInterceptor) addInterceptor(authInterceptor)app/src/main/java/com/texthip/thip/data/model/feed/request/PresignedUrlRequest.kt (1)
1-14: typealias 기반 @Body 직렬화 호환성 점검
typealias PresignedUrlRequest = List<ImageMetadata>는 컴파일 타임에List<ImageMetadata>로 확장되어 일반적으로 kotlinx.serialization/Retrofit 컨버터가 문제없이 처리합니다. 다만 런타임에서 “Serializer for class ‘PresignedUrlRequest’ not found” 류 이슈가 있었다면, 명시적 래퍼 DTO로 전환을 검토해주세요. 또한size단위(바이트)와extension포맷(jpgvsimage/jpeg)을 팀 내로 명확히 합의해두는 것이 좋습니다.app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt (1)
68-76: HEIC 미지원 시 업로드 누락 가능성안드로이드 기본 카메라는 HEIC를 생성할 수 있습니다. 불가 정책이 아니라면 "image/heic" 처리 추가를 검토하세요.
val extension = when (mimeType) { "image/png" -> "png" "image/jpeg", "image/jpg" -> "jpg" "image/gif" -> "gif" + "image/heic", "image/heif" -> "heic" else -> return null }app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (1)
107-109: 예외 래핑 시 원인(cause) 보존 필요원인을 잃어버리면 디버깅이 어렵습니다. cause를 포함해 던지세요. (상단 제안 diff에 포함)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
app/src/main/java/com/texthip/thip/data/di/NetworkModule.kt(2 hunks)app/src/main/java/com/texthip/thip/data/model/feed/request/CreateFeedRequest.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/feed/request/PresignedUrlRequest.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/feed/response/PresignedUrlResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt(2 hunks)app/src/main/java/com/texthip/thip/data/service/FeedService.kt(3 hunks)app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt(1 hunks)
🔇 Additional comments (4)
app/src/main/java/com/texthip/thip/data/di/NetworkModule.kt (1)
74-78: ImageUploadHelper DI 바인딩 적절합니다ApplicationContext 주입으로 리소스 누수 우려도 없습니다. 그대로 가셔도 됩니다.
app/src/main/java/com/texthip/thip/data/service/FeedService.kt (2)
36-41: Presigned URL 발급 API 추가 — 업로드 시 요구 헤더/HTTP 메서드 정합성만 확인해주세요서버가 응답한
presignedUrl이 PUT 전용인지,Content-Type/Content-MD5등 특정 헤더를 요구하는지에 따라 업로드 헬퍼 설정이 달라집니다. 헬퍼 구현과 서버 스펙이 일치하는지만 최종 점검 부탁드립니다.
44-46: createFeed 멀티파트 → JSON 바디 전환 검증 완료
rg 검색 결과@Multipart/@Part및MultipartBody사용 흔적이 없으며, 모든createFeed호출부가CreateFeedRequest(JSON)로 올바르게 전환된 것을 확인했습니다.app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt (1)
49-53: 메모리 폭주 + 커넥션 누수 가능성: 파일 스트리밍 업로드 및 Response 자원 반납 필요readBytes()는 전체 파일을 메모리에 올립니다. asRequestBody(File)로 스트리밍 전송하고, execute() 결과는 use로 닫아주세요.
[Suggest_essential_refactor]- val requestBody = tempFile.readBytes().toRequestBody(mimeType.toMediaType()) + val requestBody = tempFile.asRequestBody(mimeType.toMediaType()) val request = Request.Builder() .url(presignedUrl) - .put(requestBody) + .put(requestBody) + .header("Content-Type", mimeType) .build() - val response = s3Client.newCall(request).execute() - - if (!response.isSuccessful) { - throw Exception("S3 upload failed: ${response.code} ${response.message}") - } + s3Client.newCall(request).execute().use { response -> + if (!response.isSuccessful) { + throw java.io.IOException("S3 upload failed: ${response.code} ${response.message}") + } + }추가 필요 import:
import okhttp3.RequestBody.Companion.asRequestBodyAlso applies to: 55-59
app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt (1)
26-39: 프리사인드 URL 노출 최소화: 이 클라이언트는 로깅 비활성화 권장디버그에서 BASIC이라도 요청 라인의 전체 URL(쿼리 포함)이 로그에 남아 프리사인드 쿼리 파라미터가 노출됩니다. S3 업로드 클라이언트는 로깅을 끄는 편이 안전합니다. 최소한 이 클라이언트만 Level.NONE으로 강제하십시오.
- if (BuildConfig.DEBUG) { + if (BuildConfig.DEBUG) { addInterceptor( HttpLoggingInterceptor().apply { - level = HttpLoggingInterceptor.Level.BASIC + level = HttpLoggingInterceptor.Level.NONE + redactHeader("Authorization") + redactHeader("Cookie") } ) }추가로, 업로드 요청만 로깅 제외하는 전용 OkHttpClient를 유지하는 것도 방법입니다.
🧹 Nitpick comments (4)
app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt (3)
75-93: 포맷 확장 고려: WEBP/HEIC 미지원안드로이드 갤러리/카메라에서 WEBP/HEIC가 흔합니다. 필요 시 허용 목록에 추가하세요. 서버 허용 포맷과 일치하는지 확인 바랍니다.
- val extension = when (mimeType) { + val extension = when (mimeType) { "image/png" -> "png" "image/jpeg", "image/jpg" -> "jpg" "image/gif" -> "gif" + "image/webp" -> "webp" + "image/heic" -> "heic" + "image/heif" -> "heif" else -> return@withContext null }
95-116: 예외 삼킴(detekt 지적) 및 사이즈 조회 순서
- 예외를 그대로 삼키면 원인 추적이 어렵습니다. 최소 warn 로그 남기거나 상위로 전달하세요.
- statSize(PFD) → OpenableColumns.SIZE 순으로 조회하면 I/O/권한 제약에서 더 안정적인 경우가 많습니다.
- private fun getFileSize(uri: Uri): Long? { - return try { - context.contentResolver.query(uri, arrayOf(OpenableColumns.SIZE), null, null, null)?.use { cursor -> - if (cursor.moveToFirst()) { - val sizeIndex = cursor.getColumnIndex(OpenableColumns.SIZE) - if (sizeIndex >= 0) { - val size = cursor.getLong(sizeIndex) - if (size > 0) return size - } - } - } - - context.contentResolver.openFileDescriptor(uri, "r")?.use { pfd -> - val size = pfd.statSize - if (size > 0) return size - } - - null - } catch (e: Exception) { - null - } - } + private fun getFileSize(uri: Uri): Long? { + return try { + // 1) PFD.statSize + context.contentResolver.openFileDescriptor(uri, "r")?.use { pfd -> + val s = pfd.statSize + if (s >= 0) return s + } + // 2) OpenableColumns.SIZE + context.contentResolver.query( + uri, arrayOf(OpenableColumns.SIZE), null, null, null + )?.use { c -> + if (c.moveToFirst()) { + val idx = c.getColumnIndex(OpenableColumns.SIZE) + if (idx >= 0) { + val s = c.getLong(idx) + if (s >= 0) return s + } + } + } + null + } catch (e: Exception) { + android.util.Log.w("ImageUploadHelper", "getFileSize failed for $uri", e) + null + } + }
27-29: 업로드 타임아웃 상향 및 callTimeout 권장이미지 업로드는 10초 제한이 짧을 수 있습니다. 네트워크 품질을 고려해 30~60초로 상향하고 callTimeout을 추가하세요.
- .connectTimeout(10, java.util.concurrent.TimeUnit.SECONDS) - .writeTimeout(10, java.util.concurrent.TimeUnit.SECONDS) - .readTimeout(10, java.util.concurrent.TimeUnit.SECONDS) + .connectTimeout(15, java.util.concurrent.TimeUnit.SECONDS) + .writeTimeout(60, java.util.concurrent.TimeUnit.SECONDS) + .readTimeout(60, java.util.concurrent.TimeUnit.SECONDS) + .callTimeout(90, java.util.concurrent.TimeUnit.SECONDS)app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (1)
87-126: uploadImagesToS3 예외 원인 보존·동시 업로드 제한·예외 타입 구체화
- onFailure에서 재throw할 때 원인(cause)을 함께 전달해 스택트레이스 보존 (
throw Exception("Failed to upload image…", exception))- presigned URL 개수 불일치 시
IllegalStateException사용으로 의도 명확화- 순차 처리 대신
Semaphore(permits = 3)+async/awaitAll로 동시 업로드 제한 병렬화 권장- presigned 응답의 Content-Type 강제와
ImageUploadHelper의 MIME 설정 일치 여부 확인
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt(3 hunks)app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt
[warning] 113-113: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (2)
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (2)
68-73: 이미지 유무 분기 깔끔합니다빈 목록 처리로 불필요한 네트워크 호출을 회피합니다.
82-85: 요청-응답 처리 플로우 적절handleBaseResponse().getOrThrow() 체인 일관적입니다.
| suspend fun uploadImageToS3( | ||
| uri: Uri, | ||
| presignedUrl: String | ||
| ): Result<Unit> = withContext(Dispatchers.IO) { | ||
| runCatching { | ||
| val mimeType = context.contentResolver.getType(uri) ?: "image/jpeg" | ||
| val tempFile = File(context.cacheDir, "temp_image_${System.currentTimeMillis()}") | ||
|
|
||
| try { | ||
| context.contentResolver.openInputStream(uri)?.use { inputStream -> | ||
| FileOutputStream(tempFile).use { outputStream -> | ||
| inputStream.copyTo(outputStream) | ||
| } | ||
| } ?: throw IllegalStateException("Failed to open input stream for URI: $uri") | ||
|
|
||
| val requestBody = tempFile.readBytes().toRequestBody(mimeType.toMediaType()) | ||
| val request = Request.Builder() | ||
| .url(presignedUrl) | ||
| .put(requestBody) | ||
| .build() | ||
|
|
||
| val response = s3Client.newCall(request).execute() | ||
|
|
||
| if (!response.isSuccessful) { | ||
| throw Exception("S3 upload failed: ${response.code} ${response.message}") | ||
| } | ||
| } finally { | ||
| if (tempFile.exists()) { | ||
| tempFile.delete() | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
메모리 폭주(OOM) 및 응답 자원 누수 가능성: 파일 전체 바이트 로드 + Response 미닫힘
- readBytes()로 전체 파일을 메모리에 올립니다. 대용량 이미지/동시 업로드 시 OOM 위험 큽니다.
- execute() 결과를 use로 닫지 않아 커넥션/바디 누수 소지 있습니다.
다음처럼 스트리밍 전송으로 변경하고 응답을 안전하게 닫아주세요.
@@
- val tempFile = File(context.cacheDir, "temp_image_${System.currentTimeMillis()}")
+ val tempFile = File.createTempFile("img_", null, context.cacheDir)
@@
- val requestBody = tempFile.readBytes().toRequestBody(mimeType.toMediaType())
+ val requestBody = tempFile.asRequestBody(mimeType.toMediaType())
val request = Request.Builder()
.url(presignedUrl)
.put(requestBody)
.build()
- val response = s3Client.newCall(request).execute()
-
- if (!response.isSuccessful) {
- throw Exception("S3 upload failed: ${response.code} ${response.message}")
- }
+ s3Client.newCall(request).execute().use { response ->
+ if (!response.isSuccessful) {
+ throw java.io.IOException("S3 upload failed: ${response.code} ${response.message}")
+ }
+ }추가 필요 import:
import okhttp3.RequestBody.Companion.asRequestBody
There was a problem hiding this comment.
Actionable comments posted: 3
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/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt (1)
75-84: 요청 진행 상태 토글로 버튼 비활성화/중복 요청 방지네트워크 호출 전후로
isActionInProgress를 토글해 UI의 enabled와 연동하세요.적용 예(diff):
viewModelScope.launch { - repository.joinOrCancelRoom(roomDetail.roomId, RoomAction.JOIN.value) + updateState { it.copy(isActionInProgress = true) } + repository.joinOrCancelRoom(roomDetail.roomId, RoomAction.JOIN.value) .onSuccess { + updateState { it.copy(isActionInProgress = false) } updateState { it.copy(currentButtonType = GroupBottomButtonType.CANCEL) } showToastMessage(stringResourceProvider.getString(R.string.success_participation_complete)) } .onFailure { error -> + updateState { it.copy(isActionInProgress = false) } showToastMessage(stringResourceProvider.getString(R.string.error_participation_failed, error.message ?: "")) } }추가로, 서버가 수용 인원 초과를 에러 코드로 반환한다면(예: close 처리처럼), 해당 코드에 한해
error_max_participate로 매핑하는 것도 고려해 주세요.
🧹 Nitpick comments (27)
app/src/main/java/com/texthip/thip/ui/common/forms/SearchBookTextField.kt (3)
54-54: 터치 타깃 최소 사이즈 충족 권장 (40dp → 48dp)Material 최소 터치 타깃(48dp)을 만족하도록 높이 상향을 고려해주세요. 디자인 확정이 40dp라면 넘어가셔도 됩니다.
적용 예:
- .height(40.dp) + .height(48.dp)
70-72: 검색 실행 시 trim 및 공백 가드의도치 않은 공백 검색 호출을 방지하면 UX가 좋아집니다.
- keyboardActions = KeyboardActions( - onSearch = { onSearch(text) } - ), + keyboardActions = KeyboardActions( + onSearch = { + val q = text.trim() + if (q.isNotEmpty()) onSearch(q) + } + ), ... - .clickable { - onSearch(text) - }, + .clickable { + val q = text.trim() + if (q.isNotEmpty()) onSearch(q) + },Also applies to: 104-106
92-97: a11y/i18n: contentDescription 문자열 리소스화 + 최소 인터랙티브 사이즈 확보
- 하드코딩 문자열을 stringResource로 교체해 i18n을 보장하세요.
- Icon의 터치 영역을 최소 48dp로 보장하면 접근성이 향상됩니다(semantic role도 Button 권장).
- Icon( + Icon( painter = painterResource(id = R.drawable.ic_x_circle_grey02), - contentDescription = "Clear text", - modifier = Modifier - .clickable { onValueChange("") }, + contentDescription = stringResource(R.string.a11y_clear_text), + modifier = Modifier + .minimumInteractiveComponentSize() + .clickable(role = Role.Button) { onValueChange("") }, tint = Color.Unspecified ) ... - Icon( + Icon( painter = painterResource(id = R.drawable.ic_search), - contentDescription = "Search", - modifier = Modifier - .clickable { - onSearch(text) - }, + contentDescription = stringResource(R.string.a11y_search), + modifier = Modifier + .minimumInteractiveComponentSize() + .clickable(role = Role.Button) { + val q = text.trim() + if (q.isNotEmpty()) onSearch(q) + }, tint = colors.White )추가 필요:
// import import androidx.compose.material3.minimumInteractiveComponentSize import androidx.compose.ui.semantics.Role import androidx.compose.ui.res.stringResource및 strings.xml에 a11y_clear_text, a11y_search 키 추가.
Also applies to: 101-108
app/src/main/java/com/texthip/thip/ui/common/forms/BookPageTextField.kt (3)
73-81: 포커스 진입 시 자동 전체 삭제는 파괴적 UX — 선택 전체로 대체 권장입력창에 다시 진입해 미세 수정하려는 경우에도 내용이 지워집니다. 자동 삭제 대신 포커스 인에서 전체 선택을 주면 사용자가 곧바로 덮어쓰기/부분 수정 중 선택할 수 있습니다.
예시(참고, 외부 변경 필요): TextFieldValue로 전환해 포커스 인 시
selection = TextRange(0, text.length)를 적용.
106-119: 지우기 아이콘: IconButton 사용·표시 조건·i18n 개선
- 접근성/터치 타겟을 위해
IconButton사용을 권장합니다.- 빈 문자열일 때는 아이콘을 숨기는 편이 일반적입니다.
- contentDescription를 문자열 리소스로 현지화하세요.
- trailingIcon = if (showClearButton && enabled) { - { - Icon( - painter = painterResource(id = R.drawable.ic_x_circle_grey), - contentDescription = "Clear text", - modifier = Modifier.clickable { - if (text.isNotEmpty()) { - onValueChange("") - } - }, - tint = Color.Unspecified - ) - } - } else null + trailingIcon = if (showClearButton && enabled && text.isNotEmpty()) { + { + androidx.compose.material3.IconButton(onClick = { onValueChange("") }) { + Icon( + painter = painterResource(id = R.drawable.ic_x_circle_grey), + contentDescription = stringResource(id = R.string.clear_text), + tint = Color.Unspecified + ) + } + } + } else null추가(파일 상단 import 필요):
import androidx.compose.material3.IconButton문자열 리소스 예시:
<string name="clear_text">텍스트 지우기</string>
59-63: 입력 무시는 사용성 저하 — 숫자만 추출해 정규화현재는 숫자 이외 문자가 포함되면 변경을 통째로 무시합니다. 붙여넣기 등 혼합 입력에서도 숫자만 걸러 반영하면 UX가 좋아집니다.
- onValueChange = { newText -> - if (newText.isEmpty() || newText.all { it.isDigit() }) { - onValueChange(newText) - } - }, + onValueChange = { newText -> + val digitsOnly = newText.filter(Char::isDigit) + onValueChange(digitsOnly) + },app/src/main/java/com/texthip/thip/ui/group/note/component/PageInputSection.kt (2)
75-77: API 반영은 OK이나 중복 조건입니다
BookPageTextField가 내부에서enabled를 함께 체크하므로 이 호출 지점에서showClearButton = !isGeneralReview는 기능상 중복입니다. 간결성을 위해 제거하거나 항상 기본값을 사용해도 동작 동일합니다.- isError = isError, - showClearButton = !isGeneralReview + isError = isError
46-52: 페이지 유효성: 1 미만 값에 대한 경계 체크 추가 권장현재는
bookTotalPage초과만 에러로 처리되고 0/음수는 허용됩니다. 일반적으로 유효 범위는 1..bookTotalPage입니다.- val isError = remember(pageText, bookTotalPage, isGeneralReview) { - if (isGeneralReview) { - false - } else { - pageText.toIntOrNull()?.let { it > bookTotalPage } ?: false - } - } + val isError = remember(pageText, bookTotalPage, isGeneralReview) { + if (isGeneralReview) { + false + } else { + pageText.toIntOrNull()?.let { it < 1 || it > bookTotalPage } ?: false + } + }app/src/main/java/com/texthip/thip/ui/common/header/ProfileBar.kt (3)
54-58: 0.5.dp 대신 Dp.Hairline로 1px 보더 일관성 확보 제안저해상도/고밀도 단말 혼재에서 0.5.dp는 라운딩으로 두께가 들쭉날쭉할 수 있어요. 1px 고정선이 필요하면 Dp.Hairline을 권장합니다.
- width = 0.5.dp, + width = Dp.Hairline,추가 import:
import androidx.compose.ui.unit.Dp
48-59: 원형 아바타 크롭 안정화를 위해 contentScale 추가네트워크 이미지 비율에 따라 레터박스가 보일 수 있습니다. CircleShape 사용 시 Crop을 권장합니다.
AsyncImage( model = profileImage, contentDescription = "프로필 이미지", + contentScale = ContentScale.Crop, modifier = Modifier .size(36.dp) .clip(CircleShape) .border(추가 import:
import androidx.compose.ui.layout.ContentScale
50-51: 콘텐츠 설명 문자열 리소스화고정 문자열("프로필 이미지", "화살표")은 stringResource로 통일하면 i18n/번역 관리가 용이합니다. 리소스 키가 없다면 추가를 권장합니다.
Also applies to: 93-97
app/src/main/java/com/texthip/thip/ui/common/header/ProfileBarWithDate.kt (3)
50-54: 보더 두께를 Dp.Hairline으로 통일여타 헤더/피드 컴포넌트와 함께 1px 보더로 밀도 독립적 표현을 보장하는 편이 안전합니다.
- .border( - width = 0.5.dp, + .border( + width = Dp.Hairline, color = colors.Grey02, shape = CircleShape )추가 import:
import androidx.compose.ui.unit.Dp
44-55: contentScale = Crop 추가로 썸네일 왜곡 방지원형 클리핑 시 가장자리 공백/왜곡을 방지합니다.
AsyncImage( model = profileImage, contentDescription = "프로필 이미지", + contentScale = ContentScale.Crop, modifier = Modifier .size(24.dp) .clip(CircleShape) .border(추가 import:
import androidx.compose.ui.layout.ContentScale
74-75: "메뉴" contentDescription도 리소스화접근성 문구를 stringResource로 이동하면 일관성이 높아집니다. 예: R.string.menu
app/src/main/java/com/texthip/thip/ui/common/header/ProfileBarFeed.kt (3)
53-57: 보더 두께 하이레벨(1px)로 정규화0.5.dp 대신 Dp.Hairline 사용을 권장합니다.
- .border( - width = 0.5.dp, + .border( + width = Dp.Hairline, color = colors.Grey02, shape = CircleShape )추가 import:
import androidx.compose.ui.unit.Dp
47-58: contentScale 추가로 동그란 아바타 잘림/레터박스 방지비정사각형 이미지 대응 품질이 좋아집니다.
AsyncImage( model = profileImage, contentDescription = "프로필 이미지", + contentScale = ContentScale.Crop, modifier = Modifier .size(24.dp) .clip(CircleShape) .border(추가 import:
import androidx.compose.ui.layout.ContentScale
60-66: 플레이스홀더(회색 원)에도 보더 적용해 시각적 일관성 유지이미지 있음/없음에 따라 테두리 유무가 바뀌면 미세한 레이아웃·시각 차이가 납니다. 동일 보더를 제안합니다.
} else { Box( modifier = Modifier .size(24.dp) .clip(CircleShape) + .border( + width = Dp.Hairline, + color = colors.Grey02, + shape = CircleShape + ) .background(colors.Grey) ) }추가 import:
import androidx.compose.ui.unit.Dpapp/src/main/java/com/texthip/thip/ui/feed/component/FeedSubscribelistBar.kt (3)
87-91: 아바타 보더 Dp.Hairline 적용 제안밀도별 두께 편차 최소화 목적입니다.
- .border( - width = 0.5.dp, + .border( + width = Dp.Hairline, color = colors.Grey02, shape = CircleShape )추가 import:
import androidx.compose.ui.unit.Dp
81-93: contentScale = Crop로 아바타 렌더링 안정화여러 원본 비율을 균일한 원형 썸네일로 표시합니다.
AsyncImage( model = imageUrl, contentDescription = null, + contentScale = ContentScale.Crop, modifier = Modifier .size(24.dp) .clip(CircleShape) .border(추가 import:
import androidx.compose.ui.layout.ContentScale
37-41: 컴포넌트 네이밍 정렬 제안: FeedSubscribelistBar파일명과 공개 API 명을 일치시키면 검색성과 가독성이 좋아집니다. 예: FeedSubscribelistBar
app/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt (3)
102-106: 보더 두께를 Dp.Hairline로 통일다른 화면과 동일한 1px 표현을 유지합니다.
- .border( - width = 0.5.dp, + .border( + width = Dp.Hairline, color = colors.Grey02, shape = CircleShape )추가 import:
import androidx.compose.ui.unit.Dp
96-109: contentScale = Crop 추가원형 아바타 품질 개선용입니다.
AsyncImage( model = profile.profileImageUrl, contentDescription = null, + contentScale = ContentScale.Crop, modifier = Modifier .size(36.dp) .clip(CircleShape) .border(추가 import:
import androidx.compose.ui.layout.ContentScale
41-45: 네이밍 일관성 제안: MySubscribelistBar파일명과 함수명을 일치시키면 유지보수성이 향상됩니다.
app/src/main/java/com/texthip/thip/ui/common/header/AuthorHeader.kt (1)
65-69: 원형 아바타 보더 적용 OK — 밀도별 일관성을 위해 Dp.Hairline 권장clip → border 순서 선택 적절합니다. 두께는 0.5.dp 대신 Dp.Hairline을 쓰면 모든 DPI에서 1px로 안정적입니다.
- .border( - width = 0.5.dp, + .border( + width = Dp.Hairline, color = colors.Grey02, shape = CircleShape )추가 제안(선택):
- 로딩/없음 상태(Box)에도 동일 보더를 주면 시각적 일관성이 좋아집니다. 순서는 clip → background → border 권장.
- AsyncImage에 contentScale = ContentScale.Crop를 지정하면 원 내 꽉 차게 표시되어 레터박싱을 피할 수 있습니다.
- 본 PR 전반에 유사한 보더 추가가 많다면 Avatar용 Modifier/Composable로 공통화하면 중복과 유지보수 비용을 줄일 수 있습니다.
app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt (1)
456-460: 비활성화 상태 텍스트 컬러 일관성 개선 제안disabledContainerColor는 지정했지만, 내부 Text는 color를 고정
colors.White로 지정해 버튼의 disabled 상태와 시그널이 어긋납니다. ButtonDefaults에 disabledContentColor를 지정하고, Text의 컬러 지정은 제거하세요.적용 예(선택한 범위 내 diff):
- colors = ButtonDefaults.buttonColors( - containerColor = if (uiState.isJoinButtonEnabled || buttonType != GroupBottomButtonType.JOIN) colors.Purple else colors.Grey02, - disabledContainerColor = colors.Grey02 - ), + colors = ButtonDefaults.buttonColors( + containerColor = if (uiState.isJoinButtonEnabled || buttonType != GroupBottomButtonType.JOIN) colors.Purple else colors.Grey02, + disabledContainerColor = colors.Grey02, + contentColor = colors.White, + disabledContentColor = colors.White.copy(alpha = 0.6f) + ),선택 범위 밖 보조 수정(내부 Text에서 color 제거):
Text( text = buttonText, style = typography.smalltitle_sb600_s18_h24 )app/src/main/res/values/strings.xml (1)
52-52: 문자열에 불필요한 따옴표/공백 포함 가능성
" 종료"는 실제 따옴표와 선행 공백이 그대로 노출될 수 있습니다. 기존 키들과의 일관성/디자인 의도 확인 후, 필요 없다면 따옴표와 공백을 제거해 주세요.권장 수정:
- <string name="card_item_finish">" 종료"</string> + <string name="card_item_finish">종료</string>의도적으로 공백/따옴표가 필요한 경우, 사용 지점에서 포맷팅으로 다루는 편이 안전합니다.
app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitUiState.kt (1)
20-21: 파생 상태 플래그 도입 좋습니다; 중복 계산 제거 제안
- isRoomFull / isJoinButtonEnabled 도입으로 UI-도메인 일관성이 좋아졌습니다.
- ViewModel에서도 동일 비교 대신
currentState.isRoomFull사용으로 중복 로직을 줄이길 권장합니다.- 중복 탭 방지를 위해 진행 상태 플래그를 추가해 주세요.
보조 변경(데이터 클래스에 진행 플래그 추가):
data class GroupRoomRecruitUiState( val roomDetail: RoomRecruitingResponse? = null, val isLoading: Boolean = false, val currentButtonType: GroupBottomButtonType? = null, val showToast: Boolean = false, val toastMessage: String = "", val showDialog: Boolean = false, val dialogTitle: String = "", val dialogDescription: String = "", val shouldNavigateToGroupScreen: Boolean = false, val shouldNavigateToRoomPlayingScreen: Boolean = false, val roomId: Int? = null, // NEW val isActionInProgress: Boolean = false ) { val hasRoomDetail: Boolean get() = roomDetail != null val isRoomFull: Boolean get() = roomDetail?.let { it.memberCount >= it.recruitCount } ?: false val isJoinButtonEnabled: Boolean get() = currentButtonType == GroupBottomButtonType.JOIN && !isRoomFull }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
app/src/main/java/com/texthip/thip/ui/common/forms/BookPageTextField.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/common/forms/SearchBookTextField.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/header/AuthorHeader.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/common/header/ProfileBar.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/common/header/ProfileBarFeed.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/common/header/ProfileBarWithDate.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/common/topappbar/LeftNameTopAppBar.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/component/FeedSubscribelistBar.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/note/component/PageInputSection.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt(1 hunks)app/src/main/res/values/strings.xml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/src/main/java/com/texthip/thip/ui/common/topappbar/LeftNameTopAppBar.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt (1)
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
showToastMessage(184-191)
🔇 Additional comments (4)
app/src/main/java/com/texthip/thip/ui/common/forms/SearchBookTextField.kt (2)
56-56: 팔레트 정리 LGTMDarkGrey 사용으로 톤 통일이 이뤄져 UI 일관성이 좋아졌습니다. 기능 영향 없음.
56-56: 배경색 변경에 따른 명암비/상태 컬러 검증 요청
- 텍스트(White)·플레이스홀더(Grey02)·커서(NeonGreen) 각각이 DarkGrey 배경에서 WCAG AA 명암비를 만족하는지 확인 부탁드립니다(일반 텍스트 4.5:1 권장).
- 비활성/에러 상태가 있다면 해당 상태 컬러도 DarkGrey와의 대비를 확인해주세요.
app/src/main/java/com/texthip/thip/ui/common/header/AuthorHeader.kt (1)
4-4: 보더 모듈 import 정상새로 추가된 원형 보더 적용을 위한 import가 적절합니다.
app/src/main/res/values/strings.xml (1)
428-429: 신규 에러 문자열 추가 LGTM풀방 상태 토스트 문자열 추가가 ViewModel 사용처와 일치합니다.
| var hasFocusCleared by remember(text) { mutableStateOf(false) } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
focus 클리어 플래그가 입력 변화에 따라 리셋됨
remember(text) 키 때문에 텍스트가 바뀔 때마다 hasFocusCleared가 false로 초기화되어 같은 포커스 세션에서 재삭제가 발생할 수 있습니다. 포커스 아웃에서만 리셋하도록 remember { ... }로 두는 것이 안전합니다.
- var hasFocusCleared by remember(text) { mutableStateOf(false) }
+ var hasFocusCleared by remember { mutableStateOf(false) }🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/common/forms/BookPageTextField.kt
around lines 54-55, the hasFocusCleared state is being recreated on every text
change because of remember(text); change it to remember { mutableStateOf(false)
} so the flag is not reset when the text updates, and only reset it explicitly
on focus-out handling (e.g., set hasFocusCleared = false inside the focus
change/out event handler) so the flag lifecycle is tied to focus events rather
than text changes.
| } | ||
| } | ||
| }, | ||
| enabled = buttonType != GroupBottomButtonType.JOIN || uiState.isJoinButtonEnabled, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
연속 탭(중복 요청) 방지: 진행중 플래그 반영
네트워크 호출 동안 버튼을 비활성화해 중복 JOIN/CANCEL 호출을 막아주세요. uiState에 isActionInProgress를 추가하고 enabled 조건에 포함하는 것을 권장합니다.
선택한 범위 내 diff:
- enabled = buttonType != GroupBottomButtonType.JOIN || uiState.isJoinButtonEnabled,
+ enabled = !uiState.isActionInProgress && (buttonType != GroupBottomButtonType.JOIN || uiState.isJoinButtonEnabled),보조 변경은 ViewModel/UiState 코멘트에 함께 제안했습니다. 필요하시면 패치 PR까지 만들어드릴게요.
📝 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.
| enabled = buttonType != GroupBottomButtonType.JOIN || uiState.isJoinButtonEnabled, | |
| enabled = !uiState.isActionInProgress && (buttonType != GroupBottomButtonType.JOIN || uiState.isJoinButtonEnabled), |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt
around line 456, the button enabled check doesn't prevent duplicate requests;
update the enabled expression to also require uiState.isActionInProgress ==
false (e.g., enabled = (buttonType != GroupBottomButtonType.JOIN ||
uiState.isJoinButtonEnabled) && !uiState.isActionInProgress). Also add an
isActionInProgress boolean to the UiState and update the ViewModel to set it
true before starting the JOIN/CANCEL network call and false in both success and
failure paths (including catch/finally) so the UI disables the button while the
request is in progress.
| val currentState = uiState.value | ||
| val roomDetail = currentState.roomDetail ?: return | ||
|
|
||
| // 인원이 가득 찬 경우 메시지 표시 | ||
| if (roomDetail.memberCount >= roomDetail.recruitCount) { | ||
| showToastMessage(stringResourceProvider.getString(R.string.error_max_participate)) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
TOCTOU·중복 탭 대비 가드 보강
- 계산 중 추가 탭을 무시하는 가드를 추가해 주세요.
- 풀방 체크는 UiState 파생값을 재사용하여 중복을 제거하세요.
적용 예(diff):
fun onParticipationClick() {
- val currentState = uiState.value
- val roomDetail = currentState.roomDetail ?: return
+ val currentState = uiState.value
+ if (currentState.isActionInProgress) return
+ val roomDetail = currentState.roomDetail ?: return
- // 인원이 가득 찬 경우 메시지 표시
- if (roomDetail.memberCount >= roomDetail.recruitCount) {
+ // 인원이 가득 찬 경우 메시지 표시
+ if (currentState.isRoomFull) {
showToastMessage(stringResourceProvider.getString(R.string.error_max_participate))
return
}📝 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.
| val currentState = uiState.value | |
| val roomDetail = currentState.roomDetail ?: return | |
| // 인원이 가득 찬 경우 메시지 표시 | |
| if (roomDetail.memberCount >= roomDetail.recruitCount) { | |
| showToastMessage(stringResourceProvider.getString(R.string.error_max_participate)) | |
| return | |
| } | |
| fun onParticipationClick() { | |
| val currentState = uiState.value | |
| if (currentState.isActionInProgress) return | |
| val roomDetail = currentState.roomDetail ?: return | |
| // 인원이 가득 찬 경우 메시지 표시 | |
| if (currentState.isRoomFull) { | |
| showToastMessage(stringResourceProvider.getString(R.string.error_max_participate)) | |
| return | |
| } | |
| // ...기존 로직 계속... | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt
around lines 66 to 74, strengthen the TOCTOU/duplicate-tap guard by
early-returning when an in-flight recruit action is already running (e.g., check
and set an isProcessing / isRecruiting flag in the ViewModel or derived UiState)
and reuse a single derived value for the "room full" check instead of reading
uiState.value twice; compute val state = uiState.value and val roomDetail =
state.roomDetail, then use a boolean like val isFull = roomDetail != null &&
roomDetail.memberCount >= roomDetail.recruitCount and return if isProcessing or
if isFull, ensuring you set/clear the processing flag around the async operation
to prevent duplicate taps.
There was a problem hiding this comment.
Pull Request Overview
QA 4차 수정을 위한 대규모 리팩토링으로, 피드 및 그룹 기능의 UX 개선과 API 커서 페이징 적용을 포함합니다.
Key Changes:
- 바텀 네비게이션 탭 재선택시 스크롤 최상단 및 새로고침 기능 구현
- 닉네임 입력 필드에 대문자 입력 방지 및 UI 개선
- 피드 이미지 업로드를 presigned URL 방식으로 변경
- 북마크/그룹 목록에 커서 기반 무한 스크롤 적용
Reviewed Changes
Copilot reviewed 61 out of 61 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| strings.xml | 모임방 만석 에러 메시지 추가 및 "종료" 텍스트 공백 수정 |
| ImageUploadHelper.kt | S3 presigned URL 기반 이미지 업로드 헬퍼 클래스 신규 추가 |
| SplashScreen.kt | Splash 화면 컴포넌트 분리 및 프리뷰 개선 |
| SignupNicknameScreen.kt, MypageEditScreen.kt | 닉네임 입력 필드에 대문자 입력 방지 옵션 추가 |
| FeedNavigation.kt, MainNavHost.kt | 피드 탭 재선택 트리거 파라미터 추가 |
| BottomNavigationBar.kt | 탭 재선택 핸들러 추가 |
| SavedBookViewModel.kt | 커서 기반 페이징으로 리팩토링 |
| MypageSaveScreen.kt | 무한 스크롤 및 컴포넌트 분리 적용 |
| GroupViewModel.kt | 커서 기반 페이징으로 전환 |
| GroupRoomRecruitViewModel.kt | 모임방 만석 상태 검증 로직 추가 |
| FeedWriteViewModel.kt | presigned URL 기반 이미지 업로드로 변경 |
| FeedRepository.kt | 이미지 업로드 로직을 S3 presigned URL 방식으로 변경 |
| WarningTextField.kt | BasicTextField 기반으로 재구현하여 대문자 입력 방지 및 UI 개선 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| BasicTextField( | ||
| value = value, | ||
| onValueChange = { input -> | ||
| var filtered = input | ||
| if (isNumberOnly) filtered = filtered.filter { it.isDigit() } | ||
| if (preventUppercase) filtered = filtered.lowercase() | ||
| if (filtered.length > maxLength) filtered = filtered.take(maxLength) | ||
| onValueChange(filtered) | ||
| }, |
There was a problem hiding this comment.
The filtering logic should preserve the original input's character position for better UX. Consider using a more sophisticated approach that only converts uppercase characters to lowercase while maintaining cursor position, rather than converting the entire string to lowercase.
| preventUppercase: Boolean = false | ||
| ) { | ||
| val myStyle = typography.menu_r400_s14_h24.copy(lineHeight = 14.sp) | ||
| val myStyle = typography.menu_r400_s14_h24.copy(lineHeight = 16.sp) |
There was a problem hiding this comment.
Magic number 16.sp should be defined as a constant or retrieved from theme specifications for consistency across the codebase.
| private val s3Client = OkHttpClient.Builder() | ||
| .connectTimeout(10, java.util.concurrent.TimeUnit.SECONDS) | ||
| .writeTimeout(10, java.util.concurrent.TimeUnit.SECONDS) | ||
| .readTimeout(10, java.util.concurrent.TimeUnit.SECONDS) |
There was a problem hiding this comment.
Timeout values (10 seconds) should be defined as constants to make them configurable and improve maintainability.
| private val s3Client = OkHttpClient.Builder() | |
| .connectTimeout(10, java.util.concurrent.TimeUnit.SECONDS) | |
| .writeTimeout(10, java.util.concurrent.TimeUnit.SECONDS) | |
| .readTimeout(10, java.util.concurrent.TimeUnit.SECONDS) | |
| private companion object { | |
| private const val S3_TIMEOUT_SECONDS = 10L | |
| } | |
| private val s3Client = OkHttpClient.Builder() | |
| .connectTimeout(S3_TIMEOUT_SECONDS, java.util.concurrent.TimeUnit.SECONDS) | |
| .writeTimeout(S3_TIMEOUT_SECONDS, java.util.concurrent.TimeUnit.SECONDS) | |
| .readTimeout(S3_TIMEOUT_SECONDS, java.util.concurrent.TimeUnit.SECONDS) |
| fun refreshDataAndScrollToTop() { | ||
| refreshData() | ||
| // 스크롤 상단 이동은 Screen에서 처리 | ||
| } |
There was a problem hiding this comment.
The comment indicates that scroll handling is done in the Screen, but this creates an unclear separation of concerns. Consider adding a proper documentation comment explaining the interaction between ViewModel and UI layer for this functionality.
| private fun BookUserSaveList.toBookDataFromSaved(): BookData { | ||
| return BookData( | ||
| title = this.bookTitle, | ||
| imageUrl = this.bookImageUrl, | ||
| author = this.authorName, | ||
| isbn = this.isbn | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
This extension function is defined but never used in the code. Consider removing it if it's not needed, or implement it where appropriate to avoid dead code.
| private fun BookUserSaveList.toBookDataFromSaved(): BookData { | |
| return BookData( | |
| title = this.bookTitle, | |
| imageUrl = this.bookImageUrl, | |
| author = this.authorName, | |
| isbn = this.isbn | |
| ) | |
| } |
| LaunchedEffect(shouldScrollToTop) { | ||
| if (shouldScrollToTop) { | ||
| currentListState.scrollToItem(0) | ||
| shouldScrollToTop = false | ||
| } | ||
| } | ||
|
|
||
| // 중복된 로직 제거 - 기존 bottomNavReselected 방식만 사용 |
There was a problem hiding this comment.
The comment mentions removing duplicate logic but doesn't provide context about what was removed or why. This comment should be more descriptive or removed if the refactoring is complete.
| LaunchedEffect(shouldScrollToTop) { | |
| if (shouldScrollToTop) { | |
| currentListState.scrollToItem(0) | |
| shouldScrollToTop = false | |
| } | |
| } | |
| // 중복된 로직 제거 - 기존 bottomNavReselected 방식만 사용 |
| .border( | ||
| width = 0.5.dp, | ||
| color = colors.Grey02, | ||
| shape = CircleShape | ||
| ) |
There was a problem hiding this comment.
The border width value (0.5.dp) is repeated across multiple files. Consider defining this as a theme constant to ensure consistency and easier maintenance.
➕ 이슈 링크
🔎 작업 내용
📸 스크린샷
😢 해결하지 못한 과제
[] TASK
📢 리뷰어들에게
Summary by CodeRabbit
신기능
개선
기타