Merged
Conversation
hyeminililo
added
ui 🎨
Walkthrough홈 화면에 위치 정보와 멤버 수를 표시하는 새로운 UI 컴포넌트와 벡터 드로어블 리소스를 추가합니다. HomeLocationCount, HomeMemberCount, HomeGroupMemberCount 컴포넌트와 관련 아이콘(ic_home_location, ic_home_person, ic_home_people)으로 구성됩니다. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/java/com/gildongmu/dduru/presentation/feature/home/component/HomeLocation.kt (1)
19-24: 컴포넌트 이름을 역할에 맞게 맞춰 주세요.현재 구현은 위치 정보만 렌더링하고 count 개념은 없어서
HomeLocationCount라는 이름이 API 이해를 방해합니다. PR 설명과 프리뷰 이름에 맞춰HomeLocation처럼 책임이 바로 드러나는 이름이 더 자연스럽습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/gildongmu/dduru/presentation/feature/home/component/HomeLocation.kt` around lines 19 - 24, Rename the composable function HomeLocationCount to HomeLocation to match its actual responsibility and preview/PR naming: update the function declaration (HomeLocationCount) and all call sites, previews, and imports/usages to the new name HomeLocation so the API reflects that it only renders location text without any count semantics.app/src/main/java/com/gildongmu/dduru/presentation/feature/home/component/HomeMemberCount.kt (1)
44-65: 그룹 버전도 스타일을 주입받게 열어두면 재사용성이 더 좋아집니다.
HomeMemberCount는 색상/타이포를 파라미터로 받는데HomeGroupMemberCount만gray500과Body2를 고정하고 있어서, 카드 타입이 늘어나면 같은 컴포넌트를 다시 분기하게 될 가능성이 큽니다. 두 컴포넌트의 API를 맞추거나 공통 레이아웃을 하나로 묶는 편이 더 유연합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/gildongmu/dduru/presentation/feature/home/component/HomeMemberCount.kt` around lines 44 - 65, HomeGroupMemberCount fixes: make its styling configurable to match HomeMemberCount by adding parameters for text color and typography (e.g., color: Color = DduruTheme.colors.gray500, textStyle: TextStyle = DduruTheme.typography.Body2) and use those in the Text and Icon tint; alternatively extract a shared composable used by both HomeMemberCount and HomeGroupMemberCount to avoid duplicated fixed styles—update HomeGroupMemberCount signature and usages to accept the new style params so components remain reusable as card types grow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/home/component/HomeMemberCount.kt`:
- Around line 36-39: The hard-coded member count string in the HomeMemberCount
composable (Text call using "$current/${max}명") should be moved to a string
resource with placeholders to support localization and easier maintenance;
replace the inline interpolation in the Text composable(s) (both at the shown
occurrence and the similar one around lines 60-63) with a call to
stringResource(R.string.member_count_format, current, max) and add a
corresponding formatted string resource (e.g. "<xliff:g
id="current">%1$d</xliff:g> / <xliff:g id="max">%2$d</xliff:g>명" or similar) in
strings.xml so translations only need to change the resource.
---
Nitpick comments:
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/home/component/HomeLocation.kt`:
- Around line 19-24: Rename the composable function HomeLocationCount to
HomeLocation to match its actual responsibility and preview/PR naming: update
the function declaration (HomeLocationCount) and all call sites, previews, and
imports/usages to the new name HomeLocation so the API reflects that it only
renders location text without any count semantics.
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/home/component/HomeMemberCount.kt`:
- Around line 44-65: HomeGroupMemberCount fixes: make its styling configurable
to match HomeMemberCount by adding parameters for text color and typography
(e.g., color: Color = DduruTheme.colors.gray500, textStyle: TextStyle =
DduruTheme.typography.Body2) and use those in the Text and Icon tint;
alternatively extract a shared composable used by both HomeMemberCount and
HomeGroupMemberCount to avoid duplicated fixed styles—update
HomeGroupMemberCount signature and usages to accept the new style params so
components remain reusable as card types grow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9113192a-0927-43d0-b0b7-de06571404e1
📒 Files selected for processing (5)
app/src/main/java/com/gildongmu/dduru/presentation/feature/home/component/HomeLocation.ktapp/src/main/java/com/gildongmu/dduru/presentation/feature/home/component/HomeMemberCount.ktapp/src/main/res/drawable/ic_home_location.xmlapp/src/main/res/drawable/ic_home_people.xmlapp/src/main/res/drawable/ic_home_person.xml
Comment on lines
+36
to
+39
| Text( | ||
| text = "$current/${max}명", | ||
| color = textColor, | ||
| style = textStyle |
There was a problem hiding this comment.
인원 표시 문자열은 리소스로 분리해 두는 편이 안전합니다.
"$current/${max}명"를 코드에 직접 박아두면 문구 변경이나 다국어 대응 시 두 군데를 같이 수정해야 합니다. 포맷 문자열은 리소스로 올려두는 쪽이 유지보수와 로컬라이징에 유리합니다.
Also applies to: 60-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/home/component/HomeMemberCount.kt`
around lines 36 - 39, The hard-coded member count string in the HomeMemberCount
composable (Text call using "$current/${max}명") should be moved to a string
resource with placeholders to support localization and easier maintenance;
replace the inline interpolation in the Text composable(s) (both at the shown
occurrence and the similar one around lines 60-63) with a call to
stringResource(R.string.member_count_format, current, max) and add a
corresponding formatted string resource (e.g. "<xliff:g
id="current">%1$d</xliff:g> / <xliff:g id="max">%2$d</xliff:g>명" or similar) in
strings.xml so translations only need to change the resource.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ 작업 내용
📄 상세 내용
HomeMemberCount
여행 카드에서 인원 정보를 표시하기 위한 컴포넌트입니다. single 아이콘과 group 아이콘을 각각 나눠서 컴포넌트로 구현했습니다.
아이콘 + 인원수 텍스트 구조로 구성되어 있으며, current / max 형태의 인원 정보를 전달받아 표시하도록 구현했습니다.
HomeLocation
여행 카드에서 여행 위치 정보를 표시하기 위한 컴포넌트입니다.
위치 아이콘과 장소명을 함께 표시하는 형태로 구성했습니다.
👀 To Reviewer
따로 없습니다 ,,! ! 컴포넌트인 만큼 빠른 리뷰 부탁드려요호 !!
📷 스크린샷 / 영상
Summary by CodeRabbit