Conversation
THIP-TextHip#27 TopAppBar Pull, Merge
Walkthrough여러 UI 컴포넌트가 리팩토링 및 개선되었습니다. 기존의 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GroupRoomScreen
participant DefaultTopAppBar
User->>GroupRoomScreen: 화면 진입
GroupRoomScreen->>DefaultTopAppBar: TopBar 렌더링(타이틀/아이콘 숨김)
DefaultTopAppBar-->>GroupRoomScreen: TopBar UI 반환
GroupRoomScreen->>User: 나머지 그룹방 UI 표시
sequenceDiagram
participant User
participant CardAlarm
User->>CardAlarm: badgeText, title 등 파라미터 전달
CardAlarm->>User: badgeText 포함 알림 카드 UI 렌더링
sequenceDiagram
participant User
participant FormTextFieldDefault
User->>FormTextFieldDefault: showLimit, limit, showIcon 등 전달
FormTextFieldDefault->>User: 글자수 제한/아이콘 적용 텍스트필드 표시
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 변경사항 내에서 범위를 벗어난 기능적 코드 변경은 발견되지 않았습니다.) Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
💤 Files with no reviewable changes (2)
🧰 Additional context used🧬 Code Graph Analysis (1)app/src/main/java/com/texthip/thip/ui/myPage/groupPage/GroupPageScreen.kt (1)
🔇 Additional comments (16)
✨ 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 UI for various group-related screens, integrates standardized top app bars, and enhances shared form and card components.
- Refactored MyGroupScreen, GroupRoomScreen, and GroupPageScreen to use DefaultTopAppBar/LogoTopAppBar
- Enhanced FormTextFieldDefault with character limits, limit display, and clear-icon toggle
- Renamed NotificationCard to CardAlarm with dynamic badgeText and removed an unused string resource
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/strings.xml | Removed unused <string name="group"> resource |
| app/src/main/java/com/texthip/thip/ui/myPage/myGroup/MyGroupScreen.kt | Swapped custom header for DefaultTopAppBar and adjusted layout nesting |
| app/src/main/java/com/texthip/thip/ui/myPage/groupRoom/GroupRoomScreen.kt | Wrapped content with DefaultTopAppBar and cleaned up back arrow code |
| app/src/main/java/com/texthip/thip/ui/myPage/groupPage/MainTopAppBar.kt | Deleted obsolete MainTopAppBar |
| app/src/main/java/com/texthip/thip/ui/myPage/groupPage/GroupPageScreen.kt | Replaced MainTopAppBar with LogoTopAppBar |
| app/src/main/java/com/texthip/thip/ui/common/forms/FormTextFieldDefault.kt | Added character-limit logic, trailing-icon toggle, and preview updates |
| app/src/main/java/com/texthip/thip/ui/common/cards/CardAlarm.kt | Renamed card, introduced badgeText, and removed hardcoded size |
Comments suppressed due to low confidence (2)
app/src/main/res/values/strings.xml:40
- The
groupstring resource was removed—ensure allstringResource(R.string.group)calls have been replaced or removed to prevent missing resource errors.
<string name="time_ago">시간 전</string>
app/src/main/java/com/texthip/thip/ui/myPage/groupPage/GroupPageScreen.kt:53
- Missing import for
painterResource. Addimport androidx.compose.ui.res.painterResourceto avoid unresolved reference.
leftIcon = painterResource(R.drawable.ic_done),
| modifier = Modifier | ||
| .size(width = 320.dp, height = 48.dp), |
There was a problem hiding this comment.
[nitpick] Hardcoding size(320.dp, 48.dp) can limit responsiveness. Consider using fillMaxWidth() or passing size via modifier for better adaptability on different screen widths.
| modifier = Modifier | |
| .size(width = 320.dp, height = 48.dp), | |
| modifier = modifier | |
| .fillMaxWidth() | |
| .padding(vertical = 8.dp), |
| Box( | ||
| modifier = Modifier | ||
| .size(width = 40.dp, height = 24.dp) | ||
| .clip(RoundedCornerShape(13.dp)) |
There was a problem hiding this comment.
[nitpick] After removing the fixed .size(...) on the badge container, consider adding minWidth/minHeight or a fixed height to ensure consistent badge dimensions across varying badgeText lengths.
➕ 이슈 링크
🔎 작업 내용
어떤 부분이 구현되었는지 설명해주세요
내 모임방(진행 중, 모집 중)
네비게이션과 연결된 메인 모임 페이지
모임방 페이지(참여, 참여 취소, 모집 마감)
주연이가 부탁한 컴포넌트 수정
📸 스크린샷
**내 모임방 (진행 중)** 내 모임방 (모집 중)
네비게이션과 연결된 메인 모임 페이지
모임방 페이지(참여, 참여 취소, 모집 마감)
컴포넌트 수정
FormTextFieldDefault
CardAlarm
😢 해결하지 못한 과제
모임방 참여 페이지 배경
페이지에서 컴포넌트를 눌렀을 때 click event(화면 연결)
CardAlarm에서 ~시간 전과 날짜를 보여주는 부분은 서버에서 어떻게 데이터를 보내주는지에 따라서 많이 바뀔 것 같아서 검토가 필요할듯 합니당
📢 리뷰어들에게
Summary by CodeRabbit
신규 기능
UI 개선
기타