[REFACTOR] 모임방 생성화면 책 선택 수정, Dialog 팝업 구현 #151
Conversation
Walkthrough모임 생성 플로우에서 그룹 책 탭을 제거하고 확인 대화상자 흐름을 도입했으며, 날짜 범위 검증·디바운스, 스크롤바(LazyListState) 오버로드 추가, 관련 뷰모델·UI 상태 정리 및 문자열 리소스가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Screen as GroupMakeRoomScreen
participant VM as GroupMakeRoomViewModel
participant Dialog as DialogPopup
participant API
User->>Screen: 모임 생성 버튼 클릭
Screen->>VM: onToggleConfirmDialog(true)
VM->>VM: showConfirmDialog = true
Screen->>Dialog: 다이얼로그 표시 (confirm/cancel texts)
alt 사용자가 확인
User->>Dialog: confirm
Dialog->>Screen: onConfirm
Screen->>VM: onToggleConfirmDialog(false)
Screen->>VM: onCreateGroup()
VM->>API: 모임 생성 요청
API-->>VM: 응답
else 사용자가 취소
User->>Dialog: cancel
Dialog->>Screen: onCancel
Screen->>VM: onToggleConfirmDialog(false)
end
sequenceDiagram
actor User
participant Picker as GroupRoomDurationPicker
participant Utils as WheelPickerUtils
participant VM as GroupMakeRoomViewModel
User->>Picker: 시작/종료일 선택
Picker->>Picker: isPickerTouched = true
note right of Picker: 100ms 디바운스 시작
loop debounce delay
end
Picker->>Utils: validateDateRange(start, end, min, max)
Utils-->>Picker: (validatedStart, validatedEnd)
Picker->>Picker: recruitmentDays 계산
Picker->>VM: onDateRangeSelected(validatedStart, validatedEnd)
Picker->>Picker: 모집기간 UI 표시
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-08-13T05:42:13.580ZApplied to files:
🧬 Code graph analysis (1)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/com/texthip/thip/utils/auth/AuthInterceptor.kt (1)
36-41: OkHttp 인터셉터에서 runBlocking 사용은 위험합니다.OkHttp 인터셉터는 OkHttp의 스레드 풀에서 실행되는데,
runBlocking을 사용하면 해당 스레드가 블로킹되어 성능 저하 및 스레드 풀 고갈 위험이 있습니다. 특히 다수의 요청이 동시에 인증 오류를 반환하는 경우 심각한 병목이 발생할 수 있습니다.다음 중 하나의 방법으로 개선하는 것을 권장합니다:
방법 1: TokenManager에 동기 메서드 추가 (권장)
TokenManager에 동기 버전의 메서드를 추가:
// TokenManager에 추가 fun clearTokensSync() { // 동기적으로 토큰 클리어 }if (shouldClearAuth) { - runBlocking { - tokenManager.clearTokens() - authStateManager.triggerTokenExpired() - } + tokenManager.clearTokensSync() + authStateManager.triggerTokenExpiredSync() }방법 2: GlobalScope 사용 (fire-and-forget 방식)
if (shouldClearAuth) { - runBlocking { + GlobalScope.launch(Dispatchers.IO) { tokenManager.clearTokens() authStateManager.triggerTokenExpired() } }참고: 이 문제는 기존의 lines 21-22에도 동일하게 적용됩니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt (1)
36-41: 90일 기준 불일치 확인됨 — 상수화를 통한 일관성 통일 필수검증 결과, 다음 두 위치에서 모임 기간 상한선이 일치하지 않음을 확인했습니다.
- GroupMakeRoomUiState.kt:39:
daysBetween in 1..90(최대 90일 허용)- GroupRoomDurationPicker.kt:96:
isOverLimit = daysBetween > 91(91일 초과시만 에러)91일 입력 시 발생하는 문제:
- UI 검증: 91 > 91 = false → 에러 미표시, 입력 진행 가능
- 제출 검증: 91 in 1..90 = false → 제출 실패
권장 수정 사항:
GroupRoomConstraints.kt신규 파일 생성 (제안된 경로 동일)GroupMakeRoomUiState.kt에서 상수 참조로 변경GroupRoomDurationPicker.kt의isOverLimit조건을daysBetween > GroupRoomConstraints.MAX_DURATION_DAYS또는daysBetween >= 91로 수정하여 검증 로직 통일
🧹 Nitpick comments (11)
app/src/main/java/com/texthip/thip/ui/common/modal/ScrollbarUtil.kt (2)
61-61: 고정된 스크롤바 높이가 컨텐츠 비율을 반영하지 않습니다.
scrollbarHeight = size.height / 8f로 고정되어 있어, 실제 컨텐츠 양과 뷰포트 크기의 비율을 반영하지 못합니다. 일반적인 스크롤바 구현에서는 썸 높이가(뷰포트 높이 / 전체 컨텐츠 높이) * 뷰포트 높이로 계산됩니다.다음과 같이 개선할 수 있습니다:
- val scrollbarHeight = size.height / 8f + val thumbHeightRatio = (viewportHeight / estimatedTotalHeight.toFloat()).coerceIn(0.1f, 1f) + val scrollbarHeight = size.height * thumbHeightRatio
84-97: 두 오버로드 간 스크롤바 그리기 로직이 중복됩니다.
ScrollState와LazyListState오버로드 모두 동일한drawRoundRect호출을 사용하여 트랙과 썸을 그립니다. 이 로직을 별도의 private 함수로 추출하면 코드 중복을 제거하고 유지보수성이 향상됩니다.다음과 같이 리팩토링할 수 있습니다:
private fun DrawScope.drawScrollbar( trackThickness: Dp, thumbThickness: Dp, trackColor: Color, thumbColor: Color, scrollbarOffsetY: Float, scrollbarHeight: Float ) { // 전체 고정 바 drawRoundRect( color = trackColor, topLeft = Offset(x = size.width - trackThickness.toPx(), y = 0f), size = Size(trackThickness.toPx(), size.height), cornerRadius = CornerRadius(trackThickness.toPx() / 2) ) // 핸들 바 drawRoundRect( color = thumbColor, topLeft = Offset(x = size.width - thumbThickness.toPx(), y = scrollbarOffsetY), size = Size(thumbThickness.toPx(), scrollbarHeight), cornerRadius = CornerRadius(thumbThickness.toPx() / 2) ) }그런 다음 두 오버로드에서 이 함수를 호출하면 됩니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt (1)
114-114: 방어적 코딩이 적용되었으나 실질적으로는 불필요합니다.
days.maxOrNull() ?: 1을 사용하여 null 안전성을 추가했습니다.days리스트는(1..lengthOfMonth()).toList()로 생성되므로 항상 최소 1개 이상의 요소를 포함하여maxOrNull()이 null을 반환할 수 없습니다.현재 코드도 문제없지만, 더 명확하게 하려면
days.max()를 그대로 사용하거나, 주석으로 방어적 코딩 의도를 명시하는 것을 고려할 수 있습니다.app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt (1)
45-47: 비밀번호 형식 검증 보완 제안(숫자 4자리 제한)현재 길이(4)만 확인하여 영문/기호도 통과합니다. 숫자 4자리만 허용하려면 아래처럼 보완하는 것을 권장합니다.
- val isPasswordValid: Boolean - get() = !isPrivate || password.length == 4 + val isPasswordValid: Boolean + get() = !isPrivate || (password.length == 4 && password.all { it.isDigit() })app/src/main/java/com/texthip/thip/ui/common/modal/DialogPopup.kt (1)
37-43: 고정 폭 320.dp → 반응형 제약으로 변경 권장작은/큰 화면 대응을 위해 fillMaxWidth + widthIn(max)를 권장합니다. 현재 고정 폭은 태블릿/분할 화면에서 부자연스러울 수 있습니다.
적용 diff:
@@ - .width(320.dp) + .fillMaxWidth() + .widthIn(max = 320.dp) .background( color = colors.DarkGrey, shape = RoundedCornerShape(12.dp) ) .padding(20.dp)필요 시 import:
+import androidx.compose.foundation.layout.widthInapp/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (3)
166-171: GenreChipRow에 width(12.dp) 적용은 오타/의도 오해 가능성 높음Row 너비를 12dp로 제한하면 칩이 잘리거나 배치가 비정상일 수 있습니다. 전체 폭 사용을 권장합니다.
제안 diff:
- GenreChipRow( - modifier = Modifier.width(12.dp), + GenreChipRow( + modifier = Modifier.fillMaxWidth(), genres = uiState.genres.toDisplayStrings(), selectedIndex = uiState.selectedGenreIndex, onSelect = onSelectGenre, horizontalArrangement = Arrangement.Center )
126-130: blur 조건이 많으면 저사양 기기에서 성능 저하 가능스크림(반투명 배경)만으로도 강조가 충분한 경우가 많습니다. blur 강도를 낮추거나 조건을 축소하는 방안을 고려해 주세요.
54-55: DATE_FORMATTER 로케일/중복 관리고정 패턴 대신 Locale에 맞춘 형식(또는 공용 유틸의 포맷터)을 재사용하면 일관성과 i18n 대응이 좋아집니다. utils에 이미 포맷터가 있다면 그쪽으로 통합 권장.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt (3)
31-31: 테마 색상 접근 방식 일관화 + 인디케이터 가독성 유지colors alias와 ThipTheme.colors를 혼용하면 가독성이 떨어집니다. 한 가지 방식으로 통일하는 것을 권장합니다. 아래처럼 alias import를 제거하고 ThipTheme.colors를 사용해 주세요.
- import com.texthip.thip.ui.theme.ThipTheme.colors- CircularProgressIndicator( - color = colors.White - ) + CircularProgressIndicator( + color = ThipTheme.colors.White + )추가로 로딩 시 수직 중앙정렬을 의도하셨다면 Box에 동일한 높이 정책을 부여해야 합니다(현재는 가로 중앙 정렬만 적용). 예시:
Box( modifier = Modifier .fillMaxWidth() .weight(1f) // <- 추가 (ColumnScope) .align(Alignment.CenterHorizontally), contentAlignment = Alignment.Center ) { CircularProgressIndicator(color = ThipTheme.colors.White) }Also applies to: 125-127
132-136: 로딩/목록 레이아웃의 높이/패딩 일관성 확보목록 분기에는 weight(1f)와 좌우 padding을 주셨지만, 로딩 분기(Box)에는 없어 상·하 위치가 달라 보일 수 있습니다. 상단 입력 영역 고정, 하단 콘텐츠 영역(로딩/목록 동일)로 레이아웃을 통일하면 시각적 점프가 줄어듭니다.
간단히는 로딩 Box에도 동일한 weight/패딩을 부여하거나, 아래처럼 공통 컨테이너로 감싸세요:
val contentModifier = Modifier .weight(1f) .padding(horizontal = 20.dp) if (isLoading) { Box(Modifier.fillMaxWidth().then(contentModifier), contentAlignment = Alignment.Center) { ... } } else { Column(Modifier.fillMaxWidth().then(contentModifier)) { ... } }
53-54: API 추가 사항 정리: showGroupBooksTab 기본값 문서화 및 사용처 검증기획상 모임 생성 플로우에서는 그룹 책 탭을 숨겨야 하므로, 해당 화면 호출부에서 showGroupBooksTab=false가 실제로 적용되는지 확인해 주세요. 또한 파라미터 의도를 KDoc으로 명시하면 재사용 시 혼동을 줄일 수 있습니다. 프리뷰에도 false 케이스를 하나 추가하는 것을 권장합니다.
프리뷰 예시:
@Preview(showBackground = true) @Composable fun PreviewBookSearchBottomSheet_SavedOnly() { ThipTheme { GroupBookSearchBottomSheet( onDismiss = {}, onBookSelect = {}, onRequestBook = {}, savedBooks = dummySavedBooks, groupBooks = emptyList(), showGroupBooksTab = false ) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
app/src/main/java/com/texthip/thip/ui/common/modal/DialogPopup.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/common/modal/ScrollbarUtil.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupMemberLimitPicker.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupRoomDurationPicker.kt(7 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/mock/GroupMakeRoomRequest.kt(0 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt(10 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/util/WheelPickerDisplayUtils.kt(2 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(8 hunks)app/src/main/java/com/texthip/thip/utils/auth/AuthInterceptor.kt(2 hunks)app/src/main/res/values/strings.xml(2 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/java/com/texthip/thip/ui/group/makeroom/mock/GroupMakeRoomRequest.kt
🧰 Additional context used
🧬 Code graph analysis (3)
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (2)
app/src/main/java/com/texthip/thip/ui/common/topappbar/InputTopAppBar.kt (1)
InputTopAppBar(25-69)app/src/main/java/com/texthip/thip/ui/common/modal/DialogPopup.kt (1)
DialogPopup(25-78)
app/src/main/java/com/texthip/thip/ui/common/modal/DialogPopup.kt (1)
app/src/main/java/com/texthip/thip/ui/common/buttons/ActionMediumButton.kt (1)
ActionMediumButton(31-80)
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (5)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt (1)
updateState(41-43)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt (1)
updateState(28-30)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(30-32)app/src/main/java/com/texthip/thip/ui/search/viewmodel/BookDetailViewModel.kt (1)
updateState(47-49)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
updateState(26-28)
🔇 Additional comments (15)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt (3)
71-75: 포매팅 개선 승인 - 가독성 향상람다 함수의 다중 라인 포매팅으로 코드 가독성이 향상되었습니다.
remember의존성 명시가 올바르고 클로저 변수 참조도 적절합니다.
77-81: 포매팅 개선 승인 - 일관된 스타일 유지
normalizeOffset람다의 다중 라인 포매팅이 이전getCircularIndex와 일관된 스타일을 유지합니다. 의존성 명시가 정확하고 함수 호출이 명확합니다.
83-87: 포매팅 개선 승인 - 일관된 코드 스타일
offsetToIndex람다의 포매팅도 이전 두 람다와 일관된 다중 라인 스타일을 유지합니다. 모든remember초기화가 통일된 형식으로 개선되어 코드 일관성이 향상되었습니다.app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupMemberLimitPicker.kt (1)
31-31: 범위 변경은 올바르며 상태 검증 계층으로 보호됩니다.
memberLimit상태의 검증자가 이미memberLimit in 2..30범위를 강제하고 있어서, 이 변경은 안전합니다. 선택기 범위(2..30)가 상태 검증자와 일치하며, 기본값(30)도 유효합니다.setMemberLimit()호출 시 상태 검증자가 1 이하의 값을 거부하므로 추가적인 확인이 불필요합니다.app/src/main/java/com/texthip/thip/ui/common/modal/ScrollbarUtil.kt (1)
28-42: 주석 추가로 가독성이 향상되었습니다.한글 주석이 추가되어 스크롤바의 트랙과 썸 렌더링 로직을 이해하기 쉬워졌습니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt (1)
58-61: 리뷰 의견 철회됨 – 제약 조건이 적절히 제공됩니다.
GroupBookListWithScrollbar의 모든 production 사용처는ModalBottomSheetLayout내에서.fillMaxHeight(0.8f)로 제약된 부모 컨테이너에 중첩되어 있습니다. 부모Column의.weight(1f)수정자가 높이 제약을 전달하므로,fillMaxHeight()사용은 안전합니다.app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt (1)
43-49:remember블록 추가로 성능 최적화되었습니다.years와 days 계산을
remember블록으로 감싸고 적절한 의존성을 지정하여 불필요한 재계산을 방지했습니다. 이는 컴포지션 성능을 향상시킵니다.app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt (1)
32-34: 확인 다이얼로그 상태 추가, 방향성 적합
showConfirmDialog도입으로 VM의 토글 API와 일관됩니다. UI 제어에 적합해 보입니다.app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
328-330: 확인 다이얼로그 토글 API 추가, 일관성 양호UIState와 연결되는 단순 토글로 역할이 명확합니다. 성공/실패 콜백에서 다이얼로그를 닫는 정책만 화면단과 합의해 주세요.
app/src/main/res/values/strings.xml (3)
34-35: 버튼 공통 라벨 추가 OK
cancel,confirm추가로 다이얼로그 컴포넌트 재사용성이 좋아집니다.
309-309: 모집 기간 메시지 자리표시자 점검
%d일수 포맷은 GroupRoomDurationPicker에서ChronoUnit.DAYS로 전달되어 일관적입니다. 0일(내일 시작) 케이스도 자연스럽게 표시되는지 한 번만 실제 UI로 확인 부탁드립니다.
311-313: 생성 확인 메시지 포맷 인자 개수/형식 확인
%1$s~%4$s총 4개 문자열이 필요합니다. 실제 사용처에서 로케일 포맷(예:yyyy.MM.dd)로 렌더링된 날짜 문자열을 전달하는지 확인해 주세요. 인자 누락 시 런타임 예외가 발생합니다.app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupRoomDurationPicker.kt (2)
68-76: 공용 유틸을 통한 날짜 자동 보정 👍
validateDateRange로 보정 후 변경이 없을 때만 콜백하는 흐름이 깔끔합니다.
60-66: 모집기간 디바운스 계산, UX 향상에 적합입력 중 깜빡임을 줄여 가독성이 좋아집니다. 이 상태는 초기화/재선택 시 잘 리셋되고 있습니다.
초기 진입 직후 1초 내 빠른 조작에서도 기대대로 갱신되는지 실제 기기에서 한 번만 확인 부탁드립니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (1)
75-83: 확인 다이얼로그 뒤에 생성 트리거 이동 — 잘 반영됨생성 호출을 onToggleConfirmDialog 경유로 변경해 사용자 확인 단계를 추가한 점 좋습니다.
| confirmText: String = stringResource(R.string.yes), | ||
| cancelText: String = stringResource(R.string.no), |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| val averageItemHeight = if (layoutInfo.visibleItemsInfo.isNotEmpty()) { | ||
| layoutInfo.visibleItemsInfo.map { it.size }.average().toFloat() | ||
| } else { | ||
| 100f | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
평균 아이템 높이 계산이 이질적인 리스트에서 부정확할 수 있습니다.
현재 구현은 표시되는 아이템들의 평균 높이를 사용하여 전체 컨텐츠 높이를 추정합니다. 아이템 크기가 크게 다를 경우 스크롤바 위치가 부정확해질 수 있습니다. 또한 fallback 값 100f는 임의로 선택된 값입니다.
이 컴포넌트가 크기가 균일한 아이템 리스트에서만 사용되는지 확인하세요:
🏁 Script executed:
#!/bin/bash
# drawVerticalScrollbar(LazyListState) 사용처 확인
rg -nP --type=kotlin -C10 'drawVerticalScrollbar\s*\(\s*\w+\s*:\s*LazyListState'Length of output: 0
검증을 위해 더 광범위한 코드베이스 검색이 필요합니다.
---
🏁 Script executed:
```shell
#!/bin/bash
# drawVerticalScrollbar 함수 정의 찾기
rg -nP --type=kotlin 'fun\s+drawVerticalScrollbar' -A 15
Length of output: 0
🏁 Script executed:
#!/bin/bash
# drawVerticalScrollbar 함수의 모든 호출처 검색
rg -nP --type=kotlin 'drawVerticalScrollbar\s*\(' -B 5 -A 2Length of output: 3398
🏁 Script executed:
#!/bin/bash
# ScrollbarUtil.kt 파일 전체 내용 확인
fd -t f 'ScrollbarUtil\.kt' --exec cat {}Length of output: 3756
추가 검증을 위해 실제 사용 사례를 확인하겠습니다.
---
🏁 Script executed:
```shell
#!/bin/bash
# GroupBookListWithScrollbar.kt 파일 내용 확인
fd -t f 'GroupBookListWithScrollbar\.kt' --exec cat {}
Length of output: 3464
🏁 Script executed:
#!/bin/bash
# GroupBookListWithScrollbar의 컴포지션 호출처 검색
rg -nP --type=kotlin 'GroupBookListWithScrollbar\s*\(' -B 3 -A 3Length of output: 4887
LazyListState 버전 스크롤바 위치 계산을 개선해야 합니다.
현재 구현(drawVerticalScrollbar(lazyListState))은 visibleItemsInfo에 표시되는 아이템들의 평균 높이만 사용하여 전체 컨텐츠 높이를 추정합니다. 실제 사용처인 GroupBookListWithScrollbar에서 각 아이템이 CardBookSearch(가변 높이) + Spacers(25.dp 고정)로 구성되어 있기 때문에, 비가시 영역의 아이템들이 다른 높이를 가질 경우 스크롤바 위치가 정확하지 않을 수 있습니다. 또한 fallback 값 100f는 임의적입니다.
더 정확한 스크롤 위치 계산을 위해 다음을 고려해야 합니다:
- 전체 아이템의 높이 정보를 누적하여 추적하거나
- 평균값을 더 보수적으로 계산하거나
- 스크롤 진행률 계산 로직을 재검토
🤖 Prompt for AI Agents
app/src/main/java/com/texthip/thip/ui/common/modal/ScrollbarUtil.kt around lines
64-68: the scrollbar current code estimates total content height using only the
average of visibleItemsInfo (with a hardcoded 100f fallback), which yields
inaccurate scroll positions for variable-height items; fix it by replacing the
arbitrary fallback and naive average with a conservative estimation using
measured data — compute the sum of the heights of visibleItemsInfo, derive an
average from those measured items (only if count>0), then estimate unseen items
height as average * (totalItemsCount - visibleItemsInfo.count), and use measured
sum + estimated unseen height as total content height; alternatively implement a
per-item height cache (update via onGloballyPositioned) and use the exact summed
heights when available to compute precise scrollbar position.
| val tabs = if (showGroupBooksTab) { | ||
| listOf( | ||
| stringResource(R.string.group_saved_book), | ||
| stringResource(R.string.group_book) | ||
| ) | ||
| } else { | ||
| listOf(stringResource(R.string.group_saved_book)) | ||
| } |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt
Show resolved
Hide resolved
| // 날짜 범위 계산 | ||
| val daysBetween = ChronoUnit.DAYS.between(startDate, endDate) | ||
| val isOverLimit = daysBetween > 91 | ||
|
|
There was a problem hiding this comment.
최대 기간 상수화로 UiState와 기준 통일 필요
isOverLimit = daysBetween > 91는 UiState의 90일 기준과 어긋납니다. 앞서 제안한 상수를 사용해 통일해 주세요.
- val isOverLimit = daysBetween > 91
+ import com.texthip.thip.ui.group.makeroom.GroupRoomConstraints
+ val isOverLimit = daysBetween > GroupRoomConstraints.MAX_DURATION_DAYSCommittable 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/GroupRoomDurationPicker.kt
around lines 94 to 97, replace the hardcoded 91 with the shared max-duration
constant used by UiState (e.g., MAX_ROOM_DURATION_DAYS = 90) so the check uses
the same 90-day threshold; import or reference the existing constant from the
UiState/shared constants file and update the comparison to use that constant
(daysBetween > MAX_ROOM_DURATION_DAYS) to ensure both files use the same limit.
| if (uiState.isLoading) { | ||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .background(Color.Black.copy(alpha = 0.5f)), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| CircularProgressIndicator(color = colors.NeonGreen) | ||
| } | ||
| } |
There was a problem hiding this comment.
로딩 오버레이가 터치 이벤트를 차단하지 않음 → 중복 입력/내부 클릭 가능성
배경에 색만 입힌 Box는 터치를 소비하지 않습니다. 클릭/포인터를 소비해 입력을 차단하세요.
적용 diff:
@@
- if (uiState.isLoading) {
- Box(
- modifier = Modifier
- .fillMaxSize()
- .background(Color.Black.copy(alpha = 0.5f)),
- contentAlignment = Alignment.Center
- ) {
- CircularProgressIndicator(color = colors.NeonGreen)
- }
- }
+ if (uiState.isLoading) {
+ Box(
+ modifier = Modifier
+ .fillMaxSize()
+ .background(Color.Black.copy(alpha = 0.5f))
+ .clickable(
+ indication = null,
+ interactionSource = remember { MutableInteractionSource() }
+ ) { /* consume clicks while loading */ },
+ contentAlignment = Alignment.Center
+ ) {
+ CircularProgressIndicator(color = colors.NeonGreen)
+ }
+ }필요 import:
+import androidx.compose.foundation.clickable
+import androidx.compose.foundation.interaction.MutableInteractionSource
+import androidx.compose.runtime.remember📝 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.
| if (uiState.isLoading) { | |
| Box( | |
| modifier = Modifier | |
| .fillMaxSize() | |
| .background(Color.Black.copy(alpha = 0.5f)), | |
| contentAlignment = Alignment.Center | |
| ) { | |
| CircularProgressIndicator(color = colors.NeonGreen) | |
| } | |
| } | |
| if (uiState.isLoading) { | |
| Box( | |
| modifier = Modifier | |
| .fillMaxSize() | |
| .background(Color.Black.copy(alpha = 0.5f)) | |
| .clickable( | |
| indication = null, | |
| interactionSource = remember { MutableInteractionSource() } | |
| ) { /* consume clicks while loading */ }, | |
| contentAlignment = Alignment.Center | |
| ) { | |
| CircularProgressIndicator(color = colors.NeonGreen) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt
around lines 318 to 327, the semi-transparent Box used for the loading overlay
doesn't consume touch events so underlying UI can still receive clicks; update
the Box modifier to consume pointer input (e.g. add .pointerInput(Unit) {
awaitPointerEventScope { while (true) { awaitPointerEvent() } } } or use a
non-enabled clickable) so all touch events are intercepted while loading, and
add the necessary imports (androidx.compose.ui.input.pointer.pointerInput and
androidx.compose.ui.input.pointer.awaitPointerEventScope).
| @JvmStatic | ||
| fun validateDateRange( | ||
| startDate: LocalDate, | ||
| endDate: LocalDate, | ||
| minDate: LocalDate, | ||
| maxDate: LocalDate | ||
| ): Pair<LocalDate, LocalDate> { | ||
| // 시작 날짜 유효성 검사 | ||
| val validatedStart = when { | ||
| startDate.isBefore(minDate) -> minDate | ||
| startDate.isAfter(maxDate) -> maxDate | ||
| else -> startDate | ||
| } | ||
|
|
||
| // 종료 날짜 유효성 검사 | ||
| val minEndDate = validatedStart.plusDays(1) | ||
| val validatedEnd = when { | ||
| endDate.isAfter(maxDate) -> maxDate | ||
| endDate.isBefore(minEndDate) -> minEndDate | ||
| else -> endDate | ||
| } | ||
|
|
||
| return validatedStart to validatedEnd | ||
| } |
There was a problem hiding this comment.
경계값 버그: start=maxDate일 때 end가 maxDate를 넘어설 수 있음
현재 validatedStart= maxDate가 되면 minEndDate = validatedStart.plusDays(1)가 maxDate+1이 되어, endDate <= maxDate인 경우에도 분기에서 minEndDate를 반환해 상한을 위반할 소지가 있습니다. 시작일 상한을 maxDate.minusDays(1)로 제한하고, 종료일은 그 최소값과 maxDate 사이로 재조정해야 합니다.
권장 수정:
fun validateDateRange(
startDate: LocalDate,
endDate: LocalDate,
minDate: LocalDate,
maxDate: LocalDate
): Pair<LocalDate, LocalDate> {
- // 시작 날짜 유효성 검사
- val validatedStart = when {
- startDate.isBefore(minDate) -> minDate
- startDate.isAfter(maxDate) -> maxDate
- else -> startDate
- }
+ // 시작일은 최소 minDate, 최대 (maxDate - 1일)로 제한
+ val maxStart = maxDate.minusDays(1)
+ val validatedStart = when {
+ startDate.isBefore(minDate) -> minDate
+ startDate.isAfter(maxStart) -> maxStart
+ else -> startDate
+ }
- // 종료 날짜 유효성 검사
- val minEndDate = validatedStart.plusDays(1)
- val validatedEnd = when {
- endDate.isAfter(maxDate) -> maxDate
- endDate.isBefore(minEndDate) -> minEndDate
- else -> endDate
- }
+ // 종료일은 최소 (validatedStart+1일), 최대 maxDate
+ val minEndDate = validatedStart.plusDays(1)
+ val validatedEnd = when {
+ endDate.isBefore(minEndDate) -> minEndDate
+ endDate.isAfter(maxDate) -> maxDate
+ else -> endDate
+ }
return validatedStart to validatedEnd
}추가로(선택), 최대 기간(예: 90일) 제한도 이 유틸에서 옵션 인자로 처리하면 화면/VM 간 기준 일원화에 도움이 됩니다.
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/util/WheelPickerDisplayUtils.kt
around lines 32 to 55, the current validation lets validatedStart become maxDate
which makes minEndDate = maxDate.plusDays(1) and can force validatedEnd beyond
maxDate; change the start upper bound to maxDate.minusDays(1) so validatedStart
= min(startDate, maxDate.minusDays(1)) and then clamp validatedEnd between
validatedStart.plusDays(1) and maxDate (i.e., validatedEnd = max(minEndDate,
min(endDate, maxDate))). Optionally add a parameter for a maximum span (e.g.,
maxDays) and enforce validatedEnd <= validatedStart.plusDays(maxDays) if
provided.
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
Show resolved
Hide resolved
| fun loadMoreSearchResults() { | ||
| val currentState = _uiState.value | ||
| if (currentState.isLoadingMoreSearchResults || | ||
| if (currentState.isLoadingMoreSearchResults || | ||
| currentState.isSearching || | ||
| currentState.isLastSearchPage || | ||
| currentState.isLastSearchPage || | ||
| currentState.searchResults.isEmpty() || | ||
| currentState.currentSearchQuery.isBlank()) { | ||
| currentState.currentSearchQuery.isBlank() | ||
| ) { | ||
| return |
There was a problem hiding this comment.
검색 ‘더보기’에서 이전 상태로 병합하는 stale merge 가능성
currentState 스냅샷을 사용해 searchResults = currentState.searchResults + newResults로 병합하면, 호출 사이에 상태가 변한 경우(예: 빠른 추가 검색, 다른 페이지 로딩 완료) 신규 항목 손실/중복이 생길 수 있습니다. 업데이트 클로저 내에서 최신 상태를 기반으로 병합하세요.
- updateState {
- it.copy(
- searchResults = currentState.searchResults + newResults,
- searchPage = response.page,
- isLastSearchPage = response.last,
- isLoadingMoreSearchResults = false
- )
- }
+ updateState { state ->
+ state.copy(
+ searchResults = state.searchResults + newResults,
+ searchPage = response.page,
+ isLastSearchPage = response.last,
+ isLoadingMoreSearchResults = false
+ )
+ }Also applies to: 262-267
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
around lines 236-244 (and similarly 262-267), the code snapshots currentState
and then writes searchResults = currentState.searchResults + newResults, which
can lose or duplicate items if state changed between read and write; instead,
perform the merge inside an atomic state update using the stateflow's
update/compare-and-set style updater so the lambda receives the latest state and
you merge using prev.searchResults + newResults before returning prev.copy(...),
applying the same change to the other occurrence at 262-267.
| private fun isAuthError(response: Response): Boolean { | ||
| return try { | ||
| val body = response.peekBody(Long.MAX_VALUE).string() | ||
| // 에러 코드 40108 (인증 처리 중 서버 오류) 확인 | ||
| body.contains("40108") | ||
| } catch (e: Exception) { | ||
| false | ||
| } | ||
| } |
There was a problem hiding this comment.
에러 감지 로직이 취약하고 성능 문제가 있을 수 있습니다.
현재 구현의 문제점:
-
취약한 에러 감지:
body.contains("40108")은 응답 본문 어디든 "40108" 문자열이 있으면 인증 에러로 간주합니다. 이는 실제 데이터나 다른 필드에 해당 문자열이 포함된 경우 false positive를 발생시킬 수 있습니다. -
성능 이슈:
peekBody(Long.MAX_VALUE)는 매우 큰 응답의 경우 메모리와 성능에 부담을 줄 수 있습니다. -
광범위한 예외 처리: 모든 예외를 catch하고
false를 반환하면 실제 문제(예: JSON 파싱 오류)를 은폐할 수 있습니다.
JSON 응답을 파싱하여 정확한 에러 코드 필드를 확인하는 것을 권장합니다:
private fun isAuthError(response: Response): Boolean {
return try {
- val body = response.peekBody(Long.MAX_VALUE).string()
- // 에러 코드 40108 (인증 처리 중 서버 오류) 확인
- body.contains("40108")
- } catch (e: Exception) {
+ val body = response.peekBody(2048).string() // 적절한 크기로 제한
+ val jsonObject = JSONObject(body)
+ val errorCode = jsonObject.optString("code", "")
+ errorCode == "40108"
+ } catch (e: JSONException) {
+ false
+ } catch (e: IOException) {
false
}
}또는 프로젝트에서 사용 중인 JSON 라이브러리(Gson, Moshi 등)를 활용할 수 있습니다.
🤖 Prompt for AI Agents
app/src/main/java/com/texthip/thip/utils/auth/AuthInterceptor.kt lines 46-54:
the current isAuthError implementation uses peekBody(Long.MAX_VALUE) and
body.contains("40108") which causes false positives, memory/CPU issues and
swallows all exceptions; instead read a bounded peek of the response (limit to a
reasonable size, e.g., 64KB), parse the body as JSON using the project’s JSON
library (Gson/Moshi) to extract the explicit error code field (e.g., "code" or
"error.code"), compare that numeric value to 40108, and only catch and log
JSON/parsing/IO exceptions (do not silently swallow unexpected exceptions)
returning false on parse errors; ensure the code handles absent bodies/nulls
defensively.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (3)
292-316: Box 오버레이 → Compose Dialog 전환, 접근성/포커스/뒤로가기 보장Dialog로 교체되어 시스템 레벨 동작이 확보되었습니다. onDismissRequest 처리도 적절합니다. 이전 리뷰 지적을 잘 반영하셨습니다.
299-304: 문자열 포맷 인자 중복(meetingStartDate 2회) — 모집 종료일을 전달해야 합니다.strings.xml의 group_create_confirm_message는 4개 인자를 기대하며, 2번째는 “모집 종료일”이어야 합니다. 현재 2번째·3번째에 meetingStartDate가 중복 전달됩니다. UiState에 recruitmentEndDate(또는 동등 필드)를 추가·관리한 뒤 아래처럼 교체하세요.
description = stringResource( R.string.group_create_confirm_message, java.time.LocalDate.now().format(DATE_FORMATTER), - uiState.meetingStartDate.format(DATE_FORMATTER), + uiState.recruitmentEndDate.format(DATE_FORMATTER), uiState.meetingStartDate.format(DATE_FORMATTER), uiState.meetingEndDate.format(DATE_FORMATTER) ),검증 스크립트(플레이스홀더 순서 확인):
#!/bin/bash # strings.xml에서 group_create_confirm_message 확인 fd -a strings.xml | while read f; do echo "== $f" rg -nC2 'name="group_create_confirm_message"' "$f" done
318-327: 로딩 오버레이가 터치를 차단하지 않습니다 → 중복 입력/오작동 위험반투명 Box는 포인터를 소비하지 않습니다. 로딩 중에는 입력을 반드시 인터셉트하세요.
@@ -import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.Color +import androidx.compose.foundation.clickable +import androidx.compose.foundation.interaction.MutableInteractionSource +import androidx.compose.runtime.remember @@ - if (uiState.isLoading) { + if (uiState.isLoading) { Box( modifier = Modifier .fillMaxSize() - .background(Color.Black.copy(alpha = 0.5f)), + .background(Color.Black.copy(alpha = 0.5f)) + .clickable( + indication = null, + interactionSource = remember { MutableInteractionSource() } + ) { /* consume clicks while loading */ }, contentAlignment = Alignment.Center ) { CircularProgressIndicator(color = colors.NeonGreen) } }
🧹 Nitpick comments (4)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt (3)
62-75: 예외 기반 폴백 대신 계산 기반으로 단순화하세요.정상 흐름에서 try/catch는 불필요한 오버헤드입니다. 해당 월의 길이로 안전한 day를 계산해 바로 생성하면 가독성과 성능이 개선됩니다. 월 변경 블록(라인 95-101)에도 동일 적용을 권장합니다.
+import kotlin.math.min @@ - val fallbackDate = try { - LocalDate.of(newYear, month, day) - } catch (e: Exception) { - val lastDay = LocalDate.of(newYear, month, 1).lengthOfMonth() - LocalDate.of(newYear, month, lastDay) - } + val lastDay = LocalDate.of(newYear, month, 1).lengthOfMonth() + val fallbackDate = LocalDate.of(newYear, month, min(day, lastDay))월 변경 시:
- val fallbackDate = try { - LocalDate.of(year, newMonth, day) - } catch (e: Exception) { - val lastDay = LocalDate.of(year, newMonth, 1).lengthOfMonth() - LocalDate.of(year, newMonth, lastDay) - } + val lastDay = LocalDate.of(year, newMonth, 1).lengthOfMonth() + val fallbackDate = LocalDate.of(year, newMonth, min(day, lastDay))
68-74: 클램프 로직 중복 → 헬퍼로 추출해 DRY 유지.min/maxDate 보정 when 블록이 연-월-일에 반복됩니다. private 헬퍼(예: clamp(date, min, max))로 추출하거나 기존 공용 유틸이 있다면 재사용해 단일 소스로 관리하세요. 유지보수 시 실수 가능성이 줄어듭니다.
+private fun clamp(date: LocalDate, minDate: LocalDate, maxDate: LocalDate): LocalDate = + when { + date.isBefore(minDate) -> minDate + date.isAfter(maxDate) -> maxDate + else -> date + } @@ - val validatedDate = when { - fallbackDate.isBefore(minDate) -> minDate - fallbackDate.isAfter(maxDate) -> maxDate - else -> fallbackDate - } + val validatedDate = clamp(fallbackDate, minDate, maxDate) @@ - val validatedDate = when { - fallbackDate.isBefore(minDate) -> minDate - fallbackDate.isAfter(maxDate) -> maxDate - else -> fallbackDate - } + val validatedDate = clamp(fallbackDate, minDate, maxDate) @@ - val validatedDate = when { - newDate.isBefore(minDate) -> minDate - newDate.isAfter(maxDate) -> maxDate - else -> newDate - } + val validatedDate = clamp(newDate, minDate, maxDate)Also applies to: 102-106, 130-134
43-49: 월/일 목록을 min/maxDate에 맞춰 동적 제한 권장(UX).현재는 불가능한 월/일도 노출됐다가 선택 시점에 클램프됩니다. 선택 목록 자체를 범위에 맞추면 혼란이 줄어듭니다.
- val months = remember { (1..12).toList() } - val days = remember(year, month) { - (1..LocalDate.of(year, month, 1).lengthOfMonth()).toList() - } + val months = remember(year, minDate, maxDate) { + when (year) { + minDate.year -> (minDate.monthValue..12).toList() + maxDate.year -> (1..maxDate.monthValue).toList() + else -> (1..12).toList() + } + } + val days = remember(year, month, minDate, maxDate) { + val last = LocalDate.of(year, month, 1).lengthOfMonth() + val start = if (year == minDate.year && month == minDate.monthValue) minDate.dayOfMonth else 1 + val end = if (year == maxDate.year && month == maxDate.monthValue) minDate.withYear(year).withMonth(month).let { + maxDate.dayOfMonth + } else last + (start..end).toList() + }또한 day 선택 항목을 다음처럼 보정하면 일관성이 높습니다:
- selectedItem = day.coerceAtMost(days.maxOrNull() ?: 1), + selectedItem = day.coerceIn(days.firstOrNull() ?: 1, days.lastOrNull() ?: 1),app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (1)
166-172: 가로 중앙 정렬 의도라면 고정 width(12.dp) 제거가 필요합니다.현재 Modifier.width(12.dp)로 영역이 12dp로 제한되어 Center가 무의미합니다. 리스트 폭을 확보하세요.
- GenreChipRow( - modifier = Modifier.width(12.dp), + GenreChipRow( + modifier = Modifier.fillMaxWidth(), genres = uiState.genres.toDisplayStrings(), selectedIndex = uiState.selectedGenreIndex, onSelect = onSelectGenre, horizontalArrangement = Arrangement.Center )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira 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/ui/group/makeroom/component/GroupDatePicker.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (2)
app/src/main/java/com/texthip/thip/ui/common/topappbar/InputTopAppBar.kt (1)
InputTopAppBar(25-69)app/src/main/java/com/texthip/thip/ui/common/modal/DialogPopup.kt (1)
DialogPopup(25-78)
🔇 Additional comments (2)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt (1)
126-136: 일 선택 시 안전 보정 + 콜백 전달 처리, 잘 적용됐습니다.out-of-range를 min/max로 클램프 후 onDateSelected 전달 로직이 명확합니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (1)
139-140: 완료 버튼 → 확인 다이얼로그 트리거, 좋습니다.플로우가 명확해졌고 실수 제출을 줄일 수 있습니다.
1 participant
➕ 이슈 링크
🔎 작업 내용
📸 스크린샷
😢 해결하지 못한 과제
[] TASK
📢 리뷰어들에게
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
버그 수정