[UI] 모임방 만들기 페이지 구현 완료#39
Conversation
|
""" Walkthrough이 변경사항은 "모임방 만들기" 페이지의 UI 기능을 대규모로 도입합니다. 새로운 컴포저블 컴포넌트, 뷰모델, 목 데이터, 유틸리티, 다이얼로그, 입력 필드, 휠 피커, 날짜 선택기, 멤버 제한 선택기, 카드, 칩, 시트, 그리고 관련 리소스 및 문자열이 추가 및 수정되었습니다. 화면 전체의 흐름과 상태 관리를 위한 로직이 포괄적으로 구현되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GroupMakeRoomScreen
participant GroupMakeRoomViewModel
participant GroupRepository
User->>GroupMakeRoomScreen: 입력/선택(책, 장르, 제목 등)
GroupMakeRoomScreen->>GroupMakeRoomViewModel: 상태 업데이트 함수 호출
User->>GroupMakeRoomScreen: "완료" 버튼 클릭
GroupMakeRoomScreen->>GroupMakeRoomViewModel: createGroup() 호출
GroupMakeRoomViewModel->>GroupRepository: createGroup(request)
GroupRepository-->>GroupMakeRoomViewModel: ApiResult 응답
GroupMakeRoomViewModel->>GroupMakeRoomScreen: 성공/실패 콜백
GroupMakeRoomScreen->>User: 결과 표시/네비게이션
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements the “Create Group Room” UI end-to-end and refines the existing group room pager design.
- Added Korean string resources for group creation flows.
- Updated vector icons and adjusted spacing/padding in existing group screens and components.
- Introduced new screens (GroupRegisterBookScreen, GroupMakeRoomScreen) and reusable components (pickers, bottom sheet, etc.).
Reviewed Changes
Copilot reviewed 25 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/strings.xml | Added all group-creation strings |
| app/src/main/res/drawable/ic_x_circle_grey.xml | New grey “x” icon vector |
| app/src/main/res/drawable/ic_notice.xml | Resized/redesigned notice icon |
| GroupScreen.kt | Enabled notification icon in top app bar |
| GroupMyScreen.kt | Adjusted spacing and padding for my-room list |
| GroupPager.kt | Converted fixed card width/spacing to dynamic |
| GroupMainCard.kt | Switched from fixed to fillMaxWidth card sizing |
| GroupDeadlineRoomSection.kt | Refactored pager padding, scaling, removed dots |
| GroupRegisterBookScreen.kt | New book-registration screen |
| GroupMakeRoomScreen.kt | New create-room screen with all input flows |
All component/* under makeroom and myroom |
New composables: pickers, book selector, bottom sheet |
| WarningTextField.kt, SearchBookTextField.kt | Extended common form fields with validation, icons |
| CardBookSearch.kt | Removed number prefix |
| GenreChipRow.kt | Refactored chip row spacing logic |
| CustomBottomSheet.kt | New reusable bottom sheet implementation |
Files not reviewed (2)
- .idea/deviceManager.xml: Language not supported
- .idea/misc.xml: Language not supported
Comments suppressed due to low confidence (2)
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt:36
- This newly added composable lacks associated tests or UI snapshots. Consider adding unit/UI tests (e.g., using Compose testing) to ensure stability across future changes.
fun GroupMakeRoomScreen(modifier: Modifier = Modifier) {
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt:115
- The resource R.string.change is not defined in strings.xml; this will cause a build error. Add a Change entry or use an existing resource.
text = stringResource(R.string.change),
|
|
||
| // --- 공개 설정 --- | ||
| Text( | ||
| text = "공개 설정", |
There was a problem hiding this comment.
Hard-coded Korean text should use stringResource for localization. Move "공개 설정" into strings.xml and reference it via stringResource().
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (8)
app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt (2)
55-61: 알림 표시 플래그를 하드코딩(true)하지 말고 상태값으로 분리하세요
현재hasNotification파라미터가 항상true로 고정되어 있어 실제로 읽지 않은 알림이 없을 때도 뱃지가 표시될 위험이 있습니다.viewModel에 미읽은 알림 여부를 노출하는StateFlow를 두고 구독하도록 개선하면 UI-상태 불일치를 예방할 수 있습니다.- LogoTopAppBar( + val hasNotification by viewModel.hasUnreadNotification.collectAsState() + + LogoTopAppBar( leftIcon = painterResource(R.drawable.ic_done), - hasNotification = true, + hasNotification = hasNotification, onLeftClick = { }, onRightClick = { } )
37-40: 미사용navController파라미터 제거 제안
GroupScreen함수 시그니처에navController가 존재하지만 내부에서 사용되지 않습니다. 불필요한 인자를 제외하면 호출부와 테스트 코드가 간결해집니다.app/src/main/res/drawable/ic_notice.xml (1)
1-10: 아이콘 이름과 실제 모양의 불일치
ic_notice.xml이라는 파일명은 일반적으로 알림(bell) 아이콘을 의미하지만, 실제로는 느낌표가 있는 원형 아이콘으로 변경되었습니다. 아이콘의 목적이 변경되었다면ic_warning.xml또는ic_exclamation.xml과 같은 더 명확한 이름으로 변경하는 것이 좋겠습니다.app/src/main/java/com/texthip/thip/ui/common/forms/SearchBookTextField.kt (1)
101-105: 미리보기의 하드코딩된 문자열미리보기 함수의 힌트 텍스트가 하드코딩되어 있습니다. 일관성을 위해 StringResource로 추출하는 것이 좋습니다.
SearchBookTextField( - hint = "책 제목, 저자검색", + hint = stringResource(R.string.search_book_hint), onSearch = { /* 검색 실행 */ } )app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt (1)
224-241: 단일 아이템일 때 불필요한 그라데이션 오버레이아이템이 하나뿐이어서 스크롤이 비활성화된 경우에도 그라데이션 오버레이가 표시됩니다. 이는 불필요한 UI 요소입니다.
// 그라데이션 오버레이 (아이템이 여러 개일 때만 표시) +if (items.size > 1) { Box( modifier = Modifier .fillMaxSize() .background( brush = Brush.verticalGradient( colors = listOf( colors.Black.copy(alpha = 0.8f), Color.Transparent, Color.Transparent, colors.Black.copy(alpha = 0.8f) ), startY = 0f, endY = Float.POSITIVE_INFINITY ) ) ) +}app/src/main/java/com/texthip/thip/ui/common/forms/WarningTextField.kt (1)
111-120: 미리보기의 하드코딩된 문자열미리보기 함수들의 힌트와 경고 메시지가 하드코딩되어 있습니다. StringResource로 추출하는 것이 좋습니다.
미리보기를 위한 별도의 문자열 리소스를 만들거나, 미리보기용 상수를 정의하는 것을 고려해보세요:
private object PreviewStrings { const val PASSWORD_HINT = "4자리 숫자로 입장 비밀번호를 설정" const val PASSWORD_WARNING = "4자리 숫자를 입력해주세요." }Also applies to: 133-142
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt (1)
56-56: 검색 기능 구현을 위한 TODO 추가를 고려하세요.현재 검색 기능이 주석으로만 처리되어 있습니다. 향후 구현을 위해 더 명확한 TODO 주석을 추가하는 것이 좋습니다.
- onSearch = { /* 검색 구현 */ } + onSearch = { query -> + // TODO: 책 검색 API 호출 구현 + // - 검색어: query + // - 검색 결과를 savedBooks 또는 groupBooks 목록에 반영 + }app/src/main/java/com/texthip/thip/ui/common/bottomsheet/CustomBottomSheet.kt (1)
100-100: 내부 클릭 처리 방식을 개선할 수 있습니다.빈 람다로 내부 클릭을 처리하는 방식이 다소 명확하지 않습니다.
더 명확한 방식으로 개선할 수 있습니다:
- .clickable(enabled = true) {} // 내부 클릭 먹히게 + .clickable( + indication = null, + interactionSource = remember { MutableInteractionSource() } + ) { /* 내부 클릭 이벤트 소비 */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.idea/deviceManager.xml(1 hunks).idea/misc.xml(1 hunks)app/src/main/java/com/texthip/thip/ui/common/bottomsheet/CustomBottomSheet.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipRow.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/common/cards/CardBookSearch.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/forms/SearchBookTextField.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/forms/WarningTextField.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupEmptyBookSheetContent.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupInputField.kt(1 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(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/mock/GroupBookData.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupRegisterBookScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupDeadlineRoomSection.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupPager.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt(1 hunks)app/src/main/res/drawable/ic_notice.xml(1 hunks)app/src/main/res/drawable/ic_x_circle_grey.xml(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
app/src/main/java/com/texthip/thip/ui/common/forms/SearchBookTextField.kt (1)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupInputField.kt (1)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
app/src/main/res/values/strings.xml (1)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
app/src/main/java/com/texthip/thip/ui/common/forms/WarningTextField.kt (1)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt (1)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
🧬 Code Graph Analysis (4)
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupRegisterBookScreen.kt (2)
app/src/main/java/com/texthip/thip/ui/common/topappbar/DefaultTopAppBar.kt (1)
DefaultTopAppBar(23-66)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt (2)
app/src/main/java/com/texthip/thip/ui/common/cards/CardBookSearch.kt (1)
CardBookSearch(28-66)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
app/src/main/java/com/texthip/thip/ui/common/bottomsheet/CustomBottomSheet.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/makeroom/component/GroupInputField.kt (1)
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/group/makeroom/component/GroupRoomDurationPicker.kt
[warning] 86-86: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (28)
app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt (1)
73-77: UI 간격 조정이 적절합니다.필터 행과 리스트 사이의 간격을 줄이고 LazyColumn에 상단 패딩을 추가하여 더 균형 잡힌 레이아웃을 만들었습니다. 이러한 변경은 사용자 경험을 개선하는 좋은 개선사항입니다.
app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt (1)
60-60: 반응형 레이아웃으로의 개선이 좋습니다.고정된 너비에서
fillMaxWidth()로 변경하여 다양한 화면 크기에 적응할 수 있도록 개선했습니다. 이는 사용자 경험을 향상시키는 좋은 변경사항입니다.app/src/main/res/drawable/ic_x_circle_grey.xml (1)
1-16: 벡터 드로어블이 올바르게 구성되었습니다.회색 원 배경에 흰색 X 표시가 있는 닫기/취소 아이콘이 적절하게 구현되었습니다. 스트로크 속성과 색상 설정이 일관되고 UI에서 사용하기에 적합합니다.
app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupPager.kt (1)
24-37: 반응형 레이아웃으로의 리팩토링이 훌륭합니다.고정된 크기 값들을 컨테이너 너비를 기반으로 한 동적 계산으로 변경하여 다양한 화면 크기에 적응할 수 있도록 개선했습니다. 특히
pageSpacing계산이 더 정확해졌고, 일관된 시각적 간격을 유지할 수 있게 되었습니다.app/src/main/java/com/texthip/thip/ui/common/cards/CardBookSearch.kt (1)
29-38: 컴포넌트 단순화가 적절합니다.
number파라미터와 관련 UI 요소를 제거하여 컴포넌트를 더 단순하고 재사용 가능하게 만들었습니다. 이는 다양한 컨텍스트에서 활용할 수 있는 좋은 리팩토링입니다.app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupRegisterBookScreen.kt (2)
21-53: 간단하고 깔끔한 화면 구조입니다.책 등록 화면의 기본 구조가 잘 구현되어 있습니다. 현재는 연락처 정보를 표시하는 플레이스홀더 형태로 되어 있네요.
31-31: 내비게이션 구현을 고려해보세요.현재
onLeftClick핸들러가 비어있는데, PR 목표에서 언급한 "내비게이션 연결" 작업이 완료되면 이 부분에 뒤로가기 기능을 구현해야 할 것 같습니다.app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupEmptyBookSheetContent.kt (1)
18-50: 잘 구현된 빈 상태 컴포넌트입니다.이미지, 텍스트, 버튼을 포함한 레이아웃이 깔끔하게 구성되어 있고, 테마 색상과 타이포그래피를 일관되게 사용하고 있습니다. 콜백 처리도 적절합니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupMemberLimitPicker.kt (2)
26-84: 멤버 제한 피커 컴포넌트가 잘 구현되었습니다.1-30명의 적절한 범위를 가진 휠 피커가 깔끔하게 구현되어 있습니다.
remember를 사용하여 리스트 생성을 최적화한 것과 테마 색상 및 타이포그래피 사용이 일관적입니다.
32-32: 리스트 생성 최적화가 잘 되었습니다.
remember를 사용하여 1-30 범위의 리스트를 한 번만 생성하도록 최적화한 것이 좋습니다.app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt (3)
23-58: 스크롤 가능한 책 목록 컴포넌트가 잘 구현되었습니다.커스텀 스크롤바와 함께 책 목록을 표시하는 컴포넌트가 깔끔하게 구현되어 있습니다. 아이템 간 구분선과 적절한 간격이 잘 적용되어 있습니다.
33-33: 고정 높이 사용에 대한 고려사항.320dp의 고정 높이를 사용하고 있는데, 이는 특정 디자인 요구사항에 따른 것으로 보입니다. 다양한 화면 크기에서의 동작을 확인해보시기 바랍니다.
48-54: 아이템 구분선 구현이 적절합니다.각 책 아이템 사이에 적절한 간격과 구분선을 추가하여 시각적 구분을 명확히 한 것이 좋습니다.
app/src/main/res/values/strings.xml (1)
170-199: 새로운 그룹 모임방 기능을 위한 문자열 리소스 추가 잘 구현되었습니다.모임방 만들기 기능에 필요한 모든 문자열이 체계적으로 구성되어 있고, 명명 규칙도 일관성 있게 적용되었습니다. 하드코딩된 문자열을 리소스 파일로 추출하는 좋은 관행을 따르고 있습니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupRoomDurationPicker.kt (3)
1-21: 패키지 구조와 import 문이 잘 구성되어 있습니다.필요한 의존성만 깔끔하게 import하고 있으며, 패키지 구조도 적절합니다.
35-64: 날짜 범위 계산 로직이 잘 구현되어 있습니다.동적으로 유효한 년도, 월, 일 범위를 계산하는 로직이 정확하고 사용자 친화적입니다. 특히 현재 날짜와 최대 날짜 제한을 고려한 구현이 우수합니다.
65-81: 날짜 유효성 검사 로직이 잘 구현되어 있습니다.LaunchedEffect를 사용하여 날짜 변경 시 자동으로 유효성을 검사하고 보정하는 로직이 적절합니다. 오늘 이전 날짜 선택을 방지하는 기능도 잘 구현되어 있습니다.
app/src/main/java/com/texthip/thip/ui/common/bottomsheet/CustomBottomSheet.kt (3)
27-32: 컴포넌트 설계가 우수합니다.슬롯 기반 콘텐츠 구조로 재사용성을 높인 설계가 훌륭합니다. ColumnScope를 사용하여 유연한 콘텐츠 구성이 가능하도록 잘 구현되어 있습니다.
33-44: 상태 관리와 애니메이션 구현이 잘 되어 있습니다.Animatable을 사용한 부드러운 애니메이션과 상태 관리 로직이 적절하게 구현되어 있습니다. 등장 애니메이션도 사용자 경험을 향상시키는 좋은 구현입니다.
80-99: 드래그 제스처 처리가 잘 구현되어 있습니다.드래그 제스처를 통한 바텀시트 해제 기능이 직관적이고 사용자 친화적으로 구현되어 있습니다. 임계값(100f)을 사용한 해제 조건도 적절합니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt (3)
28-40: 컴포넌트 인터페이스가 잘 설계되어 있습니다.년, 월, 일을 개별적으로 선택할 수 있는 유연한 인터페이스와 각각의 콜백 함수가 적절하게 구성되어 있습니다. 외부에서 유효한 날짜 목록을 제공받는 구조도 좋습니다.
41-92: UI 레이아웃 구성이 우수합니다.GroupWheelPicker를 사용하여 일관된 UI를 제공하고, 적절한 간격과 레이블로 사용자 친화적인 인터페이스를 구현했습니다. 문자열 리소스 사용도 좋은 관행을 따르고 있습니다.
94-153: 프리뷰 구현이 완성도 높습니다.실제 사용 시나리오를 잘 반영한 프리뷰로, 월별 일수 계산과 독립적인 날짜 상태 관리가 적절하게 구현되어 있습니다.
app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupDeadlineRoomSection.kt (5)
19-19: GenreChipRow 컴포넌트 이동 승인GenreChipRow가 그룹 전용 컴포넌트에서 공통 버튼 패키지로 이동된 것은 컴포넌트 재사용성을 향상시키는 좋은 변경입니다.
65-65: 타입 안전성 개선 승인
mutableStateOf(0)에서mutableIntStateOf(0)로 변경한 것은 타입 안전성을 향상시키는 좋은 개선입니다.
87-87: 패딩 증가 승인카드 내부 horizontal 패딩을
18.dp에서20.dp로 증가시킨 것은 더 나은 시각적 균형을 제공합니다.
259-259: SimplePagerIndicator 제거 여부 확인 필요
GroupDeadlineRoomSection.kt에서 SimplePagerIndicator 호출이 삭제되었는데, 아래와 같이 여전히 다른 컴포넌트(GroupPager.kt)에서 사용되고 있어 의도된 디자인 변경인지 확인이 필요합니다.
app/src/main/java/com/texthip/thip/ui/group/myroom/component/SimplePagerIndicator.kt에 정의된 컴포저블app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupPager.kt에서 여전히 호출 중디자인 사양이나 기획 의도에 따라 해당 섹션에서 페이지 인디케이터를 제거한 것이 맞는지 검토 부탁드립니다.
33-33: 동적 레이아웃 계산 로직 검증 완료
sideMargin(30.dp)은contentPadding = PaddingValues(horizontal = 30.dp)등 코드 전반의 패딩 값과 일치합니다.desiredGap = 12.dp를 기반으로 한
pageSpacing = (-(cardWidth - (cardWidth * scale)) / 2) + desiredGap
계산식도 의도한 대로 카드 간 간격과 피크(peek) 효과를 제공합니다.추가 제안
contentPadding = PaddingValues(horizontal = sideMargin)처럼 하드코딩 대신sideMargin변수를 재사용하면 유지보수성이 향상됩니다.desiredGap상수를 최상단이나 별도 파일로 분리하고, TODO 주석을 제거하면 코드가 더 깔끔해집니다.
| <component name="CodeInsightWorkspaceSettings"> | ||
| <option name="optimizeImportsOnTheFly" value="true" /> | ||
| </component> | ||
| <component name="ExternalStorageConfigurationManager" enabled="true" /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
IDE 개인 설정을 저장소에 커밋하면 불필요한 충돌을 유발할 수 있습니다
.idea 하위 파일은 개발자·IDE 환경별로 달라지므로 보통 .gitignore 에 추가하여 VCS 추적을 피합니다.
팀원 간 환경이 달라질 경우 해당 설정이 덮어써지며 충돌이 잦아질 수 있으니, 프로젝트 공용 설정이 아닌 개인 설정 파일은 제외하는 것이 좋습니다. .gitignore 검토 후 필요 시 파일을 제거해 주세요.
다음 스크립트로 .gitignore에 .idea 항목이 있는지 확인해 주세요.
🏁 Script executed:
#!/usr/bin/env bash
# .gitignore 파일에서 .idea 항목 유무 확인
if [ -f .gitignore ]; then
grep -nE "^\s*\.idea" .gitignore || echo ".idea 항목을 찾을 수 없습니다."
else
echo ".gitignore 파일이 존재하지 않습니다."
fiLength of output: 96
IDE 개인 설정 파일(.idea) Git 추적 제외 필요
현재 .gitignore에는 .idea/deploymentTargetSelector.xml만 등록되어 있어, 여전히 .idea/misc.xml 등 개인 설정 파일이 커밋되어 충돌 위험이 있습니다. 아래 사항을 반영해 주세요.
- 파일 제거
.idea/misc.xml등 개인 설정 파일을 저장소에서 삭제git rm --cached .idea/misc.xml
.gitignore수정- 루트
.gitignore에 전체.idea/디렉토리를 무시하도록 추가/.idea/
- 루트
- 커밋 및 푸시
- 위 변경사항을 커밋한 뒤 리모트에 푸시하여 불필요한 충돌을 방지
🤖 Prompt for AI Agents
In .idea/misc.xml at line 5, the file is a personal IDE configuration that
should not be tracked by Git to avoid conflicts. Remove the file from Git
tracking using 'git rm --cached .idea/misc.xml', then update the root .gitignore
file to include '/.idea/' to ignore the entire .idea directory. Finally, commit
these changes and push to the remote repository to prevent future conflicts.
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <project version="4"> | ||
| <component name="DeviceTable"> | ||
| <option name="columnSorters"> | ||
| <list> | ||
| <ColumnSorterState> | ||
| <option name="column" value="Name" /> | ||
| <option name="order" value="ASCENDING" /> | ||
| </ColumnSorterState> | ||
| </list> | ||
| </option> | ||
| </component> | ||
| </project> No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
새로운 .idea 설정 파일 커밋 여부 재검토 필요
deviceManager.xml 역시 IDE 내부 상태(디바이스 정렬 방식 등)를 담고 있어, 팀 전체에 공유할 필요성이 낮은 개인 설정에 가깝습니다.
필요한 경우 Settings Repository (공용 IDE 설정)나 문서화된 프로젝트 설정으로 관리하고, 일반 개발 흐름에서는 VCS 추적을 피하는 편이 권장됩니다. .gitignore에 추가할지 논의 부탁드립니다.
.gitignore에서 .idea 디렉토리가 무시되고 있는지 확인합니다.
🏁 Script executed:
#!/bin/bash
# .gitignore에서 .idea 디렉토리 무시 설정 확인
if [ -f .gitignore ]; then
grep -n "\.idea" .gitignore || echo "no matches"
else
echo ".gitignore 파일이 없습니다."
fiLength of output: 226
.idea/deviceManager.xml 개별 무시 설정 추가 필요
현재 .gitignore에는 일부 .idea 파일만 무시되고 있어 deviceManager.xml은 여전히 버전 관리됩니다. 이 파일은 개인 IDE 정렬 상태를 담고 있어 팀 전체에 커밋할 필요가 없습니다.
.gitignore에 다음 라인 추가를 권장합니다:
# IDE 개인 설정 무시
.idea/deviceManager.xml- 또는 팀 정책에 따라
.idea/폴더 전체 무시를 고려할 수 있습니다.
📝 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.
| <?xml version="1.0" encoding="UTF-8"?> | |
| <project version="4"> | |
| <component name="DeviceTable"> | |
| <option name="columnSorters"> | |
| <list> | |
| <ColumnSorterState> | |
| <option name="column" value="Name" /> | |
| <option name="order" value="ASCENDING" /> | |
| </ColumnSorterState> | |
| </list> | |
| </option> | |
| </component> | |
| </project> | |
| # IDE 개인 설정 무시 | |
| .idea/deviceManager.xml |
🤖 Prompt for AI Agents
In the .idea/deviceManager.xml file at lines 1 to 13, the issue is that this
IDE-specific file is currently tracked by version control but should be ignored
as it contains personal IDE settings. To fix this, add a rule to the .gitignore
file to exclude deviceManager.xml specifically or consider ignoring the entire
.idea/ directory depending on team policy. This prevents personal IDE
configuration files from being committed to the repository.
app/src/main/java/com/texthip/thip/ui/group/makeroom/mock/GroupBookData.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/common/forms/SearchBookTextField.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt
Show resolved
Hide resolved
| ) { | ||
| Icon( | ||
| painter = painterResource(id = R.drawable.ic_search), | ||
| contentDescription = "검색 아이콘", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
하드코딩된 문자열을 strings.xml로 추출하세요.
검색 관련 하드코딩된 문자열들을 strings.xml로 추출해야 합니다.
- contentDescription = "검색 아이콘",
+ contentDescription = stringResource(R.string.search_icon),- text = "검색해서 찾기",
+ text = stringResource(R.string.search_to_find),strings.xml에 해당 리소스를 추가하세요:
<string name="search_icon">검색 아이콘</string>
<string name="search_to_find">검색해서 찾기</string>Also applies to: 69-69
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt
at lines 64 and 69, the strings "검색 아이콘" and "검색해서 찾기" are hardcoded. Move these
hardcoded strings into strings.xml by adding <string name="search_icon">검색
아이콘</string> and <string name="search_to_find">검색해서 찾기</string> there, then
replace the hardcoded strings in the Kotlin file with references to these string
resources.
|
|
||
| // --- 공개 설정 --- | ||
| Text( | ||
| text = "공개 설정", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
하드코딩된 문자열을 strings.xml로 추출하세요.
공개 설정 관련 하드코딩된 문자열들을 strings.xml로 추출해야 합니다.
- text = "공개 설정",
+ text = stringResource(R.string.group_public_setting),- text = "비공개로 설정하기",
+ text = stringResource(R.string.group_set_private),- warningMessage = "4자리 숫자를 입력해주세요.",
+ warningMessage = stringResource(R.string.group_password_warning),strings.xml에 해당 리소스를 추가하세요:
<string name="group_public_setting">공개 설정</string>
<string name="group_set_private">비공개로 설정하기</string>
<string name="group_password_warning">4자리 숫자를 입력해주세요.</string>Also applies to: 216-216, 236-236
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt
at lines 205, 216, and 236, the strings are hardcoded in Korean. Extract these
hardcoded strings into the strings.xml resource file by adding the provided
string resources with keys group_public_setting, group_set_private, and
group_password_warning. Then replace the hardcoded text in the Kotlin file with
references to these string resources using the appropriate context.getString or
stringResource method.
| // 날짜 객체로 변환 | ||
| val startDate = remember(startYear, startMonth, startDay) { | ||
| try { | ||
| LocalDate.of(startYear, startMonth, startDay) | ||
| } catch (e: Exception) { | ||
| // 유효하지 않은 날짜인 경우 오늘 날짜로 fallback | ||
| today | ||
| } | ||
| } | ||
| val endDate = remember(startDate) { startDate.plusDays(1) } | ||
|
|
||
| // 90일 초과 체크 | ||
| val daysBetween = ChronoUnit.DAYS.between(startDate, endDate) | ||
| val isOverLimit = daysBetween > 90 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
예외 처리 개선이 필요합니다.
try-catch 블록에서 예외를 조용히 처리하고 있어 원래 예외 정보가 손실될 수 있습니다.
다음과 같이 로깅을 추가하여 예외 정보를 보존하는 것을 권장합니다:
val startDate = remember(startYear, startMonth, startDay) {
try {
LocalDate.of(startYear, startMonth, startDay)
} catch (e: Exception) {
+ // 예외 정보를 로깅하여 디버깅에 도움
+ println("Invalid date: $startYear-$startMonth-$startDay, fallback to today")
// 유효하지 않은 날짜인 경우 오늘 날짜로 fallback
today
}
}🧰 Tools
🪛 detekt (1.23.8)
[warning] 86-86: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupRoomDurationPicker.kt
around lines 82 to 96, the try-catch block silently handles exceptions when
creating the LocalDate, causing loss of original error details. Modify the catch
block to log the caught exception before falling back to today's date, ensuring
the exception information is preserved for debugging.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupRoomDurationPicker.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupDeadlineRoomSection.kt
Show resolved
Hide resolved
Docstrings generation was requested by @rbqks529. * #39 (comment) The following files were modified: * `app/src/main/java/com/texthip/thip/ui/common/bottomsheet/CustomBottomSheet.kt` * `app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipRow.kt` * `app/src/main/java/com/texthip/thip/ui/common/cards/CardBookSearch.kt` * `app/src/main/java/com/texthip/thip/ui/common/forms/SearchBookTextField.kt` * `app/src/main/java/com/texthip/thip/ui/common/forms/WarningTextField.kt` * `app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt` * `app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt` * `app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt` * `app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupEmptyBookSheetContent.kt` * `app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupInputField.kt` * `app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupMemberLimitPicker.kt` * `app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupRoomDurationPicker.kt` * `app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt` * `app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt` * `app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt` * `app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupRegisterBookScreen.kt` * `app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupDeadlineRoomSection.kt` * `app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt` * `app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupPager.kt` * `app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt` * `app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt`
|
Note Generated docstrings for this pull request at #40 |
Nico1eKim
left a comment
There was a problem hiding this comment.
와 picker부분들이 진짜 빡세네요 ... 고생 많았습니닷 !! 👍🫡
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt
Show resolved
Hide resolved
| savedBooks: List<BookData> = emptyList(), | ||
| groupBooks: List<BookData> = emptyList(), |
There was a problem hiding this comment.
아직은 잘 모르겠긴 한데 서버에서 하나의 list로 보내주고 boolean값같은 필드를 넣어서 이 두 데이터를 구분지을 수도 있겠다는 생각이 들어서 그 방식도 고려하면 좋을 것 같아요 ~
서버에서 지금 api 명세서는 어느정도 작성한거같아서 참고하면서 개발하면 좋을거같습니다
There was a problem hiding this comment.
서버 api 확인하고 있으면 수정하겠습니다!
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt
Outdated
Show resolved
Hide resolved
| years: List<Int>, | ||
| months: List<Int>, | ||
| days: List<Int>, |
There was a problem hiding this comment.
얘네는 인자로 안받고 year, month, day 인자로 받은 것들로 컴포넌트 안에서 변수 새로 만들어서 관리하면 안되나요 ?
| Image( | ||
| painter = painterResource(id = R.drawable.ic_notice), |
| .fillMaxWidth() | ||
| .height(44.dp) | ||
| .border( | ||
| BorderStroke(1.dp, colors.Grey02), |
There was a problem hiding this comment.
이건 처음보네욧 BorderStroke를 쓰는것과 안쓰는 것에 차이가 있나요?
There was a problem hiding this comment.
완전히 동일한걸로 알고 있어요 좀 더 직관적?
| // index가 음수/양수 모두에서 올바르게 작동하도록 | ||
| fun getCircularIndex(index: Int): Int { | ||
| val size = items.size | ||
| return ((index % size) + size) % size | ||
| } | ||
|
|
||
| // offset을 항상 0 ~ (items.size-1)*itemSpacing 범위로 | ||
| fun normalizeOffset(offset: Float): Float { | ||
| if (!circular) return offset | ||
| val total = items.size * itemSpacing | ||
| return ((offset % total) + total) % total | ||
| } | ||
|
|
||
| // 오프셋을 아이템 인덱스로 변환 (0이 중앙) | ||
| fun offsetToIndex(offset: Float): Int { | ||
| val total = items.size * itemSpacing | ||
| val normalized = if (circular) normalizeOffset(offset) else offset | ||
| val centerIndex = (-normalized / itemSpacing).roundToInt() | ||
| return if (circular) getCircularIndex(centerIndex) else centerIndex.coerceIn( | ||
| 0, | ||
| items.size - 1 | ||
| ) | ||
| } |
| var roomDescription by remember { mutableStateOf("") } | ||
| var meetingStartDate by remember { mutableStateOf(LocalDate.now()) } | ||
| var meetingEndDate by remember { mutableStateOf(LocalDate.now().plusDays(1)) } | ||
| var selectedCount by remember { mutableIntStateOf(30) } | ||
| var isPrivate by remember { mutableStateOf(false) } | ||
| var password by remember { mutableStateOf("") } |
There was a problem hiding this comment.
얘네 data로 안만들고 이렇게 하나하나 만든 이유가 있나요?
There was a problem hiding this comment.
상태로 해야 리컴포지션이 되서 화면에 보이는거 생각하긴 했는데 데이터 클래스로 만들어서 서버에 전송하는거까지 생각하고 수정하겠습니다
| Text( | ||
| text = stringResource(R.string.group_request_book_comment), | ||
| style = typography.copy_r400_s14, | ||
| color = colors.White | ||
| ) |
There was a problem hiding this comment.
이거 피그마보니까 가로 중앙 정렬같은데 스크린샷에는 디자인이 좀 다른거같아서 가로 중앙정렬 적용해주세욧 ~
| @@ -48,16 +48,21 @@ fun GroupRoomDeadlineSection( | |||
| .height(588.dp), | |||
There was a problem hiding this comment.
이렇게 높이를 하드코딩하면 반응형 적용할 때 좋지 않을거같아욧 ..! 웬만하면 사이즈 하드코딩은 비추입니다 ~
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt (1)
201-208: 접근성 지원 누락Wheel picker의 선택된 항목에 대한 접근성 정보가 없습니다. 스크린 리더 사용자를 위한 contentDescription이 필요합니다.
🧹 Nitpick comments (2)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupRoomDurationPicker.kt (1)
119-119: 하드코딩된 문자열을 리소스로 추출하세요날짜 범위 구분자 "~"가 하드코딩되어 있습니다. 일관성과 유지보수성을 위해 문자열 리소스로 추출하는 것이 좋습니다.
strings.xml에 추가:
<string name="group_date_range_separator">~</string>코드 수정:
- text = "~", + text = stringResource(R.string.group_date_range_separator),app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt (1)
145-145: Preview의 하드코딩된 문자열을 추출하세요Preview 컴포저블에 하드코딩된 한글 문자열이 있습니다. 일관성을 위해 문자열 리소스로 추출하는 것이 좋습니다.
strings.xml에 추가:
<string name="group_start_date_label">시작 날짜</string> <string name="group_end_date_label">끝 날짜</string>코드 수정:
- Text("시작 날짜", color = colors.White) + Text(stringResource(R.string.group_start_date_label), color = colors.White) ... - Text("끝 날짜", color = colors.White) + Text(stringResource(R.string.group_end_date_label), color = colors.White)Also applies to: 156-156
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
app/src/main/java/com/texthip/thip/ui/common/topappbar/LogoTopAppBar.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupEmptyBookSheetContent.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupInputField.kt(1 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(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupRegisterBookScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/util/WheelPickerDisplayUtils.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupDeadlineRoomSection.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/myPage/screen/MyPageScreen.kt(1 hunks)app/src/main/res/drawable/ic_notice.xml(1 hunks)app/src/main/res/drawable/ic_notification.xml(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- app/src/main/java/com/texthip/thip/ui/myPage/screen/MyPageScreen.kt
- app/src/main/java/com/texthip/thip/ui/common/topappbar/LogoTopAppBar.kt
- app/src/main/res/drawable/ic_notification.xml
- app/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (11)
- app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt
- app/src/main/res/drawable/ic_notice.xml
- app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupEmptyBookSheetContent.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupRegisterBookScreen.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookListWithScrollbar.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupInputField.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupMemberLimitPicker.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupDeadlineRoomSection.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt (1)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupRoomDurationPicker.kt (1)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
🧬 Code Graph Analysis (4)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt (1)
app/src/main/java/com/texthip/thip/ui/group/makeroom/util/WheelPickerDisplayUtils.kt (3)
normalizeOffset(11-16)offsetToIndex(18-29)getCircularIndex(6-9)
app/src/main/java/com/texthip/thip/ui/group/makeroom/util/WheelPickerDisplayUtils.kt (1)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt (2)
normalizeOffset(60-62)getCircularIndex(56-58)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupRoomDurationPicker.kt (2)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt (1)
GroupDatePicker(29-123)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt (2)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupWheelPicker.kt (1)
GroupWheelPicker(29-230)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
🔇 Additional comments (1)
app/src/main/java/com/texthip/thip/ui/group/makeroom/util/WheelPickerDisplayUtils.kt (1)
5-30: 유틸리티 함수 구현이 적절합니다wheel picker의 순환 인덱스 계산과 오프셋 처리를 위한 유틸리티 함수들이 잘 구현되었습니다. 특히 음수 인덱스 처리를 위한 이중 모듈로 연산 사용이 적절합니다.
| LaunchedEffect(startDate) { | ||
| val adjustedStartDate = when { | ||
| startDate.isBefore(today) -> today | ||
| startDate.isAfter(maxDate) -> maxDate | ||
| else -> startDate | ||
| } | ||
|
|
||
| if (adjustedStartDate != startDate) { | ||
| startDate = adjustedStartDate | ||
| } | ||
|
|
||
| // 끝 날짜가 시작 날짜보다 빠르면 조정 | ||
| if (endDate.isBefore(startDate.plusDays(1))) { | ||
| endDate = startDate.plusDays(1) | ||
| } | ||
| } | ||
|
|
||
| LaunchedEffect(endDate) { | ||
| val adjustedEndDate = when { | ||
| endDate.isAfter(maxDate) -> maxDate | ||
| endDate.isBefore(startDate.plusDays(1)) -> startDate.plusDays(1) | ||
| else -> endDate | ||
| } | ||
|
|
||
| if (adjustedEndDate != endDate) { | ||
| endDate = adjustedEndDate | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
날짜 유효성 검사 로직을 유틸리티 함수로 추출하세요
LaunchedEffect 내의 날짜 유효성 검사 로직이 반복되고 있습니다. 재사용성과 테스트 용이성을 위해 별도의 함수로 추출하는 것이 좋습니다.
다음과 같이 유틸리티 함수를 추가하세요:
private fun validateDateRange(
startDate: LocalDate,
endDate: LocalDate,
today: LocalDate,
maxDate: LocalDate
): Pair<LocalDate, LocalDate> {
val validStartDate = when {
startDate.isBefore(today) -> today
startDate.isAfter(maxDate) -> maxDate
else -> startDate
}
val validEndDate = when {
endDate.isAfter(maxDate) -> maxDate
endDate.isBefore(validStartDate.plusDays(1)) -> validStartDate.plusDays(1)
else -> endDate
}
return validStartDate to validEndDate
}그리고 LaunchedEffect를 단순화하세요:
- LaunchedEffect(startDate) {
- val adjustedStartDate = when {
- startDate.isBefore(today) -> today
- startDate.isAfter(maxDate) -> maxDate
- else -> startDate
- }
-
- if (adjustedStartDate != startDate) {
- startDate = adjustedStartDate
- }
-
- // 끝 날짜가 시작 날짜보다 빠르면 조정
- if (endDate.isBefore(startDate.plusDays(1))) {
- endDate = startDate.plusDays(1)
- }
- }
-
- LaunchedEffect(endDate) {
- val adjustedEndDate = when {
- endDate.isAfter(maxDate) -> maxDate
- endDate.isBefore(startDate.plusDays(1)) -> startDate.plusDays(1)
- else -> endDate
- }
-
- if (adjustedEndDate != endDate) {
- endDate = adjustedEndDate
- }
- }
+ LaunchedEffect(startDate, endDate) {
+ val (validStartDate, validEndDate) = validateDateRange(startDate, endDate, today, maxDate)
+ if (validStartDate != startDate) {
+ startDate = validStartDate
+ }
+ if (validEndDate != endDate) {
+ endDate = validEndDate
+ }
+ }🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupRoomDurationPicker.kt
around lines 56 to 83, the date validation logic inside the two LaunchedEffect
blocks is duplicated. Extract this logic into a single utility function named
validateDateRange that takes startDate, endDate, today, and maxDate as
parameters and returns a Pair of validated start and end dates. Then simplify
the LaunchedEffect blocks by calling this function and updating startDate and
endDate only if they differ from the validated values.
| // 유효한 범위 계산 | ||
| val years = (minDate.year..maxDate.year).toList() | ||
| val months = (1..12).toList() | ||
| val days = (1..selectedDate.lengthOfMonth()).toList() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
날짜 유효성 검사 로직을 개선하세요
현재 구현은 월 변경 시 유효하지 않은 날짜에 대해 일을 1로 리셋합니다. 사용자 경험을 개선하기 위해 가능한 경우 현재 선택한 일을 유지하도록 수정하는 것이 좋습니다.
days 범위 계산을 동적으로 수정:
- // 유효한 범위 계산
- val years = (minDate.year..maxDate.year).toList()
- val months = (1..12).toList()
- val days = (1..selectedDate.lengthOfMonth()).toList()
+ // 유효한 범위 계산
+ val years = (minDate.year..maxDate.year).toList()
+ val months = (1..12).toList()
+ // 현재 선택된 년/월에 따른 동적 일수 계산
+ val maxDayOfMonth = try {
+ LocalDate.of(year, month, 1).lengthOfMonth()
+ } catch (e: Exception) {
+ 28 // 안전한 기본값
+ }
+ val days = (1..maxDayOfMonth).toList()날짜 생성 로직 개선:
onItemSelected = { newYear ->
- val newDate = try {
- LocalDate.of(newYear, month, day)
- } catch (e: Exception) {
- LocalDate.of(newYear, month, 1)
- }
+ val maxDay = try {
+ LocalDate.of(newYear, month, 1).lengthOfMonth()
+ } catch (e: Exception) {
+ 28
+ }
+ val validDay = day.coerceAtMost(maxDay)
+ val newDate = LocalDate.of(newYear, month, validDay)
onDateSelected(newDate)
},월 선택 로직도 동일하게 수정:
onItemSelected = { newMonth ->
- val newDate = try {
- LocalDate.of(year, newMonth, day)
- } catch (e: Exception) {
- LocalDate.of(year, newMonth, 1)
- }
+ val maxDay = LocalDate.of(year, newMonth, 1).lengthOfMonth()
+ val validDay = day.coerceAtMost(maxDay)
+ val newDate = LocalDate.of(year, newMonth, validDay)
onDateSelected(newDate)
},Also applies to: 60-64, 85-89, 110-111
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupDatePicker.kt
around lines 42-45, 60-64, 85-89, and 110-111, the current date validation
resets the day to 1 when the month changes and the selected day is invalid. To
improve user experience, modify the days range calculation to dynamically
reflect the maximum valid day for the selected month and year, and update the
date creation logic to retain the current day if it is valid; otherwise, adjust
it to the last valid day of the month. Apply similar dynamic validation and
adjustment logic to the month selection to ensure the selected date remains
valid without unnecessarily resetting the day.
JJUYAAA
left a comment
There was a problem hiding this comment.
수고하셧습니다아 ~~~~ 먼가 동작을 확인하고 싶은 코드가 많은데 담에는 영상으로 쪄주시면 확인하기 더 좋을 것 같아유
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt
Outdated
Show resolved
Hide resolved
| LogoTopAppBar( | ||
| leftIcon = painterResource(R.drawable.ic_done), | ||
| hasNotification = false, | ||
| hasNotification = true, |
There was a problem hiding this comment.
여기를 true 로 해두면 사용자가 이 페이지 접근할 때마다 알림이 있는것처럼 보여질 것 같아요 ! 차라리 초기값은 false로 하고, 뷰모델 사용해서 서버 응답 기반으로 hasNotification 상태 변경되도록 하는게 좋을 것 같아용
There was a problem hiding this comment.
확인했습니다 확실히 기본은 false가 좋을거 같네요
app/src/main/res/values/strings.xml
Outdated
| <string name="group_register_book">책 등록하기</string> | ||
| <string name="group_register_book_comment">현재 등록된 책이 아닙니다.\n원하시는 책을 신청해주세요.</string> | ||
| <string name="group_request_book">책 신청</string> | ||
| <string name="group_thip_email">texthip2025@gmail.com</string> |
There was a problem hiding this comment.
앗 이거 이미 뽑아둔 스트링이라 안해도됐었는데!ㅋㅋㅋㅋㅎㅋ
ctrl+F 로 한번 검색해보시는 것 추천 ㅎㅋ
| <string name="group_room_explain_hint">방에 대한 짧은 소개글을 작성해주세요</string> | ||
| <string name="group_room_duration_title">모임 활동기간</string> | ||
| <string name="group_room_duration_comment">모임 활동 기간은 최대 3개월까지 설정 가능합니다.</string> | ||
| <string name="group_room_duration_error">(선택된 기간: %1$d일)</string> |
There was a problem hiding this comment.
요거 저도 같은 실수 했는데 인자 하나니까 %d 해도 될걸유~?
There was a problem hiding this comment.
이거 기본 안드로이드 string 추출로 사용하긴 했는데 차이가 있을까요?
| <string name="group_room_member_limit_comment">모집 인원은 최대 30명까지 입력할 수 있습니다.</string> | ||
| <string name="group_room_limit">명의 독서 메이트를 모집합니다</string> | ||
| <string name="group_genre_selected_comment">1개만 선택 가능합니다.</string> | ||
| <string name="group_selected_book_author">%1$s 저</string> |
| ) | ||
| } | ||
|
|
||
| // 바깥 클릭 감지 |
There was a problem hiding this comment.
👍
오싱기하네여 ㄷㄷ
담엔 요런거 동작되는거 화면녹화 보여주시면 더 조을듯
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
15-17: ViewModel 클래스를 final로 선언하는 것을 고려하세요.ViewModel은 일반적으로 상속되지 않아야 하므로
open키워드를 제거하는 것이 좋습니다. Preview를 위한 mock이 필요하다면 인터페이스를 사용하거나 다른 접근 방식을 고려하세요.-open class GroupMakeRoomViewModel( +class GroupMakeRoomViewModel( private val groupRepository: GroupRepository // 의존성 주입 ) : ViewModel() {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/SectionDivider.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/mock/GroupMakeRoomRequest.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/mock/GroupMakeRoomUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupRegisterBookScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- app/src/main/java/com/texthip/thip/ui/group/makeroom/component/SectionDivider.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupRegisterBookScreen.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/mock/GroupMakeRoomRequest.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt
- app/src/main/res/values/strings.xml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/myPage/groupPage/GroupPageScreen.kt:34-37
Timestamp: 2025-07-01T07:19:01.239Z
Learning: GroupPageScreen의 MyPageViewModel은 임시 ViewModel로, 현재는 프로토타입 단계이므로 오류 처리 등의 추가 기능은 나중에 구현 예정입니다.
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (2)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/myPage/groupPage/GroupPageScreen.kt:34-37
Timestamp: 2025-07-01T07:19:01.239Z
Learning: GroupPageScreen의 MyPageViewModel은 임시 ViewModel로, 현재는 프로토타입 단계이므로 오류 처리 등의 추가 기능은 나중에 구현 예정입니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/myPage/groupPage/GroupPageScreen.kt:34-37
Timestamp: 2025-07-01T07:19:01.239Z
Learning: GroupPageScreen의 MyPageViewModel은 임시 ViewModel로, 현재는 프로토타입 단계이므로 오류 처리 등의 추가 기능은 나중에 구현 예정입니다.
🔇 Additional comments (6)
app/src/main/java/com/texthip/thip/ui/group/makeroom/mock/GroupMakeRoomUiState.kt (3)
6-19: 데이터 클래스 구조가 잘 설계되었습니다!모든 필드에 적절한 기본값이 설정되어 있어 초기화가 안전합니다.
42-56: 서버 요청 변환 메서드가 잘 구현되었습니다!문자열 trim 처리와 조건부 패스워드 설정이 적절합니다.
21-25: 날짜 유효성 검사 로직을 개선하세요.현재 로직은 시작일이 종료일보다 늦은 경우를 허용합니다. 음수 일수도 검증에 포함시켜야 합니다.
다음과 같이 수정하세요:
val isDurationValid: Boolean get() { val daysBetween = ChronoUnit.DAYS.between(meetingStartDate, meetingEndDate) - return daysBetween in 1..90 + return meetingStartDate.isBefore(meetingEndDate) && daysBetween in 1..90 }Likely an incorrect or invalid review comment.
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (2)
24-73: 상태 업데이트 메서드들이 잘 구현되었습니다!각 메서드가 단일 책임을 가지고 있으며, 특히
togglePrivate에서 비공개 해제 시 패스워드를 초기화하는 로직이 적절합니다.
110-125: Repository 인터페이스와 데이터 클래스가 잘 설계되었습니다!클린 아키텍처 원칙을 잘 따르고 있으며, 제네릭을 활용한 ApiResult는 재사용성이 높습니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (1)
223-256: Preview와 Mock 구현이 잘 되어있습니다!테스트와 개발에 유용한 Mock 구현이며, 초기 상태 설정 예시도 도움이 됩니다.
| private val _uiState = MutableStateFlow(GroupMakeRoomUiState()) | ||
| val uiState: StateFlow<GroupMakeRoomUiState> = _uiState.asStateFlow() | ||
|
|
||
| private val genres = listOf("문학", "과학·IT", "사회과학", "인문학", "예술") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
장르 목록을 더 나은 방식으로 관리하세요.
하드코딩된 장르 목록은 유지보수성이 떨어집니다. Repository에서 가져오거나 최소한 companion object의 상수로 정의하는 것이 좋습니다.
다음 중 하나의 방식을 고려하세요:
옵션 1: Companion object 사용
+ companion object {
+ private val GENRES = listOf("문학", "과학·IT", "사회과학", "인문학", "예술")
+ }
+
- private val genres = listOf("문학", "과학·IT", "사회과학", "인문학", "예술")
+ private val genres = GENRES옵션 2: Repository에서 가져오기
// Repository에 메서드 추가
suspend fun getGenres(): List<String>
// ViewModel에서 사용
init {
viewModelScope.launch {
genres = groupRepository.getGenres()
}
}🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
at line 22, the genres list is hardcoded which reduces maintainability. To fix
this, either move the genres list into a companion object as a constant or fetch
it from a repository method. For the repository approach, add a suspend function
getGenres() in the repository and update the ViewModel to asynchronously load
genres inside an init block using viewModelScope.launch, assigning the result to
a mutable genres property.
| // 그룹 생성 요청 | ||
| fun createGroup(onSuccess: () -> Unit, onError: (String) -> Unit) { | ||
| val currentState = _uiState.value | ||
|
|
||
| if (!currentState.isFormValid) { | ||
| onError("입력 정보를 확인해주세요") | ||
| return | ||
| } | ||
|
|
||
| viewModelScope.launch { | ||
| try { | ||
| _uiState.value = currentState.copy(isLoading = true, errorMessage = null) | ||
|
|
||
| val request = currentState.toRequest() | ||
| val result = groupRepository.createGroup(request) | ||
|
|
||
| if (result.isSuccess) { | ||
| onSuccess() | ||
| } else { | ||
| onError(result.message ?: "그룹 생성에 실패했습니다") | ||
| } | ||
| } catch (e: Exception) { | ||
| onError("네트워크 오류가 발생했습니다: ${e.message}") | ||
| } finally { | ||
| _uiState.value = _uiState.value.copy(isLoading = false) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 처리 로직의 일관성을 개선하세요.
현재 에러 메시지가 UI state와 콜백 모두에서 처리되고 있어 혼란스럽습니다. 한 가지 방식으로 통일하는 것이 좋습니다.
UI state의 errorMessage를 활용하도록 수정:
fun createGroup(onSuccess: () -> Unit, onError: (String) -> Unit) {
val currentState = _uiState.value
if (!currentState.isFormValid) {
- onError("입력 정보를 확인해주세요")
+ _uiState.value = currentState.copy(errorMessage = "입력 정보를 확인해주세요")
return
}
viewModelScope.launch {
try {
_uiState.value = currentState.copy(isLoading = true, errorMessage = null)
val request = currentState.toRequest()
val result = groupRepository.createGroup(request)
if (result.isSuccess) {
onSuccess()
} else {
- onError(result.message ?: "그룹 생성에 실패했습니다")
+ _uiState.value = _uiState.value.copy(
+ errorMessage = result.message ?: "그룹 생성에 실패했습니다"
+ )
}
} catch (e: Exception) {
- onError("네트워크 오류가 발생했습니다: ${e.message}")
+ _uiState.value = _uiState.value.copy(
+ errorMessage = "네트워크 오류가 발생했습니다: ${e.message}"
+ )
} finally {
_uiState.value = _uiState.value.copy(isLoading = 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 createGroup(onSuccess: () -> Unit, onError: (String) -> Unit) { | |
| val currentState = _uiState.value | |
| if (!currentState.isFormValid) { | |
| onError("입력 정보를 확인해주세요") | |
| return | |
| } | |
| viewModelScope.launch { | |
| try { | |
| _uiState.value = currentState.copy(isLoading = true, errorMessage = null) | |
| val request = currentState.toRequest() | |
| val result = groupRepository.createGroup(request) | |
| if (result.isSuccess) { | |
| onSuccess() | |
| } else { | |
| onError(result.message ?: "그룹 생성에 실패했습니다") | |
| } | |
| } catch (e: Exception) { | |
| onError("네트워크 오류가 발생했습니다: ${e.message}") | |
| } finally { | |
| _uiState.value = _uiState.value.copy(isLoading = false) | |
| } | |
| } | |
| } | |
| // 그룹 생성 요청 | |
| fun createGroup(onSuccess: () -> Unit, onError: (String) -> Unit) { | |
| val currentState = _uiState.value | |
| if (!currentState.isFormValid) { | |
| _uiState.value = currentState.copy(errorMessage = "입력 정보를 확인해주세요") | |
| return | |
| } | |
| viewModelScope.launch { | |
| try { | |
| _uiState.value = currentState.copy(isLoading = true, errorMessage = null) | |
| val request = currentState.toRequest() | |
| val result = groupRepository.createGroup(request) | |
| if (result.isSuccess) { | |
| onSuccess() | |
| } else { | |
| _uiState.value = _uiState.value.copy( | |
| errorMessage = result.message ?: "그룹 생성에 실패했습니다" | |
| ) | |
| } | |
| } catch (e: Exception) { | |
| _uiState.value = _uiState.value.copy( | |
| errorMessage = "네트워크 오류가 발생했습니다: ${e.message}" | |
| ) | |
| } finally { | |
| _uiState.value = _uiState.value.copy(isLoading = false) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
lines 75 to 102, the error handling is inconsistent because error messages are
sent both via the onError callback and the UI state's errorMessage field. To fix
this, remove the onError callback calls for errors and instead update the
_uiState.value.errorMessage with the appropriate error messages. Keep onSuccess
callback for success handling only, ensuring all error messages are consistently
managed through the UI state.
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.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/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
22-22: 장르 목록을 더 나은 방식으로 관리하세요.하드코딩된 장르 목록은 유지보수성과 확장성이 떨어집니다. Repository에서 가져오거나 companion object의 상수로 정의하는 것을 권장합니다.
🧹 Nitpick comments (2)
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (2)
15-17: ViewModel 클래스에 KDoc 문서를 추가하세요.공개 클래스와 함수들에 대한 문서화가 부족합니다. 다른 개발자들이 이해하기 쉽도록 KDoc 주석을 추가하는 것이 좋습니다.
예시:
/** * 모임방 생성 화면의 상태를 관리하는 ViewModel * * @property groupRepository 그룹 관련 데이터를 처리하는 Repository */ open class GroupMakeRoomViewModel( private val groupRepository: GroupRepository ) : ViewModel() {
79-97: 하드코딩된 문자열을 StringResource로 추출하세요.에러 메시지들이 하드코딩되어 있습니다. 유지보수성과 다국어 지원을 위해 strings.xml로 추출하는 것이 좋습니다.
strings.xml에 추가:
<string name="error_invalid_input">입력 정보를 확인해주세요</string> <string name="error_group_creation_failed">그룹 생성에 실패했습니다</string> <string name="error_network">네트워크 오류가 발생했습니다: %s</string>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/myPage/groupPage/GroupPageScreen.kt:34-37
Timestamp: 2025-07-01T07:19:01.239Z
Learning: GroupPageScreen의 MyPageViewModel은 임시 ViewModel로, 현재는 프로토타입 단계이므로 오류 처리 등의 추가 기능은 나중에 구현 예정입니다.
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (2)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/myPage/groupPage/GroupPageScreen.kt:34-37
Timestamp: 2025-07-01T07:19:01.239Z
Learning: GroupPageScreen의 MyPageViewModel은 임시 ViewModel로, 현재는 프로토타입 단계이므로 오류 처리 등의 추가 기능은 나중에 구현 예정입니다.
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
🔇 Additional comments (1)
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
63-68: 비밀번호 초기화 동작을 확인해주세요.비밀방 설정을 해제할 때 비밀번호가 초기화됩니다. 사용자가 실수로 토글했다가 다시 활성화할 때 비밀번호를 다시 입력해야 하는데, 이것이 의도된 동작인지 확인이 필요합니다.
만약 의도하지 않은 동작이라면, 비밀번호를 유지하면서 비밀방 설정만 변경하는 방식을 고려해보세요.
| // 그룹 생성 요청 | ||
| fun createGroup(onSuccess: () -> Unit, onError: (String) -> Unit) { | ||
| val currentState = _uiState.value | ||
|
|
||
| if (!currentState.isFormValid) { | ||
| //onError("입력 정보를 확인해주세요") | ||
| return | ||
| } | ||
|
|
||
| viewModelScope.launch { | ||
| try { | ||
| _uiState.value = currentState.copy(isLoading = true, errorMessage = null) | ||
|
|
||
| val request = currentState.toRequest() | ||
| val result = groupRepository.createGroup(request) | ||
|
|
||
| if (result.isSuccess) { | ||
| onSuccess() | ||
| } else { | ||
| //onError(result.message ?: "그룹 생성에 실패했습니다") | ||
| } | ||
| } catch (e: Exception) { | ||
| //onError("네트워크 오류가 발생했습니다: ${e.message}") | ||
| } finally { | ||
| _uiState.value = _uiState.value.copy(isLoading = false) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
사용하지 않는 onError 콜백 매개변수를 제거하고 에러 처리를 완성하세요.
현재 onError 콜백이 함수 시그니처에는 있지만 실제로는 사용되지 않고 있습니다. 또한 주석 처리된 에러 처리 코드가 있습니다.
다음과 같이 수정하세요:
-fun createGroup(onSuccess: () -> Unit, onError: (String) -> Unit) {
+fun createGroup(onSuccess: () -> Unit) {
val currentState = _uiState.value
if (!currentState.isFormValid) {
- //onError("입력 정보를 확인해주세요")
+ _uiState.value = currentState.copy(errorMessage = "입력 정보를 확인해주세요")
return
}
viewModelScope.launch {
try {
_uiState.value = currentState.copy(isLoading = true, errorMessage = null)
val request = currentState.toRequest()
val result = groupRepository.createGroup(request)
if (result.isSuccess) {
onSuccess()
} else {
- //onError(result.message ?: "그룹 생성에 실패했습니다")
+ _uiState.value = _uiState.value.copy(
+ errorMessage = result.message ?: "그룹 생성에 실패했습니다"
+ )
}
} catch (e: Exception) {
- //onError("네트워크 오류가 발생했습니다: ${e.message}")
+ _uiState.value = _uiState.value.copy(
+ errorMessage = "네트워크 오류가 발생했습니다: ${e.message}"
+ )
} finally {
_uiState.value = _uiState.value.copy(isLoading = false)
}
}
}🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
between lines 75 and 102, the onError callback parameter is declared but not
used, and error handling code is commented out. To fix this, remove the onError
parameter from the function signature if it is not needed, or uncomment and
properly implement the onError calls inside the function to handle validation
errors, unsuccessful results, and exceptions by invoking onError with
appropriate error messages.
➕ 이슈 링크
🔎 작업 내용
📸 스크린샷
내 모임방 Pager 디자인 수정 완료

모임방 만들기 페이지 내부 컴포넌트 구현

-입력 필드
책 검색 bottom sheet 구현


picker 구현


screen 구현


😢 해결하지 못한 과제
[] 네비게이션 연결
📢 리뷰어들에게
Summary by CodeRabbit
신규 기능
버그 수정 및 개선
리소스 및 문자열