Refactor/#166 Figma 컴포넌트 업데이트 반영 - #167
Open
cmj7271 wants to merge 11 commits into
Open
Conversation
📝 WalkthroughWalkthrough검색 헤더와 네비게이션 컴포넌트가 확장되었고, 프로필·히스토리 화면의 레이아웃과 상호작용이 변경되었습니다. 모달 버튼, 하단 바, 배지 버튼의 스타일과 관련 문자열도 업데이트되었습니다. Changes디자인 시스템 및 프로필 UI 개편
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant 사용자
participant PotiHeaderPageSearch
participant PotiSearchBar
participant BasicTextField
사용자->>BasicTextField: 검색어 입력
BasicTextField->>PotiSearchBar: onValueChange(value)
PotiSearchBar->>PotiHeaderPageSearch: 입력값 전달
사용자->>PotiSearchBar: 검색 아이콘 또는 키보드 검색 액션
PotiSearchBar->>PotiHeaderPageSearch: onSearch(value)
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 5
🧹 Nitpick comments (1)
app/src/main/java/com/poti/android/presentation/main/MainBottomBar.kt (1)
51-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win그림자 및 라운딩 값을 Dimens 토큰으로 추출해 주세요.
20.dp,4.dp등의 디자인 값이 컴포넌트에 직접 하드코딩되어 있어 다른 화면과 값이 어긋날 수 있습니다. 기존Dimens또는 디자인 시스템 토큰이 있다면 재사용해 주세요. As per path instructions, 하드코딩된 크기 대신 Theme/Dimens 리소스를 사용해야 합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/poti/android/presentation/main/MainBottomBar.kt` around lines 51 - 63, Extract the hardcoded 20.dp corner radius and 4.dp shadow radius/offset values used by MainBottomBar into the existing Dimens or design-system tokens, then reference those tokens when creating topRoundedShape and Shadow. Reuse matching existing tokens where available rather than introducing duplicate values.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchBar.kt`:
- Line 64: Update the requester initialization in the SearchBar composable so an
externally supplied focusRequester is always used directly, while only the
internally created FocusRequester is remembered as the fallback. Avoid
remembering the nullable selection itself, ensuring a later external requester
replaces the fallback.
In
`@app/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderPrimary.kt`:
- Around line 136-141: Add selectable accessibility semantics to both
PotiHeaderPrimaryToggleItem in PotiHeaderPrimary.kt (lines 136-141) and
PotiHeaderSectionChip in PotiHeaderSection.kt (lines 73-88), passing each
component’s selected value and Role.Tab while preserving the existing click
behavior and visual styling.
In `@app/src/main/java/com/poti/android/presentation/main/MainBottomBar.kt`:
- Around line 51-64: Update the innerShadow configuration in MainBottomBar’s Row
to match the Figma shadow color and alpha, replacing the low-contrast
white-based Shadow values with the appropriate contrasting design token while
preserving the existing shape, radius, and offset.
In `@app/src/main/java/com/poti/android/presentation/user/component/UserInfo.kt`:
- Around line 32-50: Update the UserInfo layout so the variable-length activity
message and joined date can wrap onto multiple lines instead of being forced
into one fixed Row with the bullet icon. Use an adaptive FlowRow or equivalent
Compose layout, preserving the existing spacing, alignment, styling, and content
so both UserInfoText values remain visible on large fonts or narrow screens.
In `@app/src/main/java/com/poti/android/presentation/user/mypage/MyPageScreen.kt`:
- Around line 91-100: Restrict the header-below scroll areas to the remaining
parent height by replacing fillMaxSize() with fillMaxWidth().weight(1f) in
MyPageScreen.kt at lines 91-100 and ProfileScreen.kt at lines 75-85; preserve
the existing scrolling, background, padding, and arrangement behavior at both
sites.
---
Nitpick comments:
In `@app/src/main/java/com/poti/android/presentation/main/MainBottomBar.kt`:
- Around line 51-63: Extract the hardcoded 20.dp corner radius and 4.dp shadow
radius/offset values used by MainBottomBar into the existing Dimens or
design-system tokens, then reference those tokens when creating topRoundedShape
and Shadow. Reuse matching existing tokens where available rather than
introducing duplicate values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 06f37b09-9248-462b-8ff8-fd1bd3bea6df
📒 Files selected for processing (13)
app/src/main/java/com/poti/android/core/designsystem/component/button/PotiModalButton.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchBar.ktapp/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderPage.ktapp/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderPrimary.ktapp/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderSection.ktapp/src/main/java/com/poti/android/presentation/main/MainBottomBar.ktapp/src/main/java/com/poti/android/presentation/user/component/BadgeButton.ktapp/src/main/java/com/poti/android/presentation/user/component/HistorySummaryCard.ktapp/src/main/java/com/poti/android/presentation/user/component/UserInfo.ktapp/src/main/java/com/poti/android/presentation/user/component/UserProfile.ktapp/src/main/java/com/poti/android/presentation/user/mypage/MyPageScreen.ktapp/src/main/java/com/poti/android/presentation/user/profile/ProfileScreen.ktapp/src/main/res/values/strings.xml
| onClear: () -> Unit = { onValueChange("") }, | ||
| focusRequester: FocusRequester? = null, | ||
| ) { | ||
| val requester = remember { focusRequester ?: FocusRequester() } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 --glob '*.kt' '\bfocusRequester\s*=' app/src/main/java
rg -n -C 3 --glob '*.kt' '\bPoti(SearchBar|HeaderPageSearch)\s*\(' app/src/main/javaRepository: team-poti/POTI-ANDROID
Length of output: 10433
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '54,95p' app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchBar.kt
sed -n '116,151p' app/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderPage.ktRepository: team-poti/POTI-ANDROID
Length of output: 2970
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 --glob '*.kt' 'PotiHeaderPageSearch\(' app/src/main/javaRepository: team-poti/POTI-ANDROID
Length of output: 2809
외부 FocusRequester 전달 시에는 내부 requester를 재사용하지 않도록 해주세요.
현재 호출부에서는 외부 focusRequester가 전달되지 않아 이슈가 나타나지 않지만, 이 파라미터를 공개 API로 사용하려 한다면 현재 remember { focusRequester ?: FocusRequester() } 방식은 한번 null이면 이후 외부 value가 전달되어도 반영되지 않습니다. fallback은 기억하되, 외부 requester가 있으면 이를 그대로 사용해 주세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchBar.kt`
at line 64, Update the requester initialization in the SearchBar composable so
an externally supplied focusRequester is always used directly, while only the
internally created FocusRequester is remembered as the fallback. Avoid
remembering the nullable selection itself, ensuring a later external requester
replaces the fallback.
Comment on lines
+51
to
+64
| val topRoundedShape = RoundedCornerShape(topStart = 20.dp, topEnd = 20.dp) | ||
|
|
||
| Row( | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .background(PotiTheme.colors.white) | ||
| .background(PotiTheme.colors.white, topRoundedShape) | ||
| .innerShadow( | ||
| shape = topRoundedShape, | ||
| shadow = Shadow( | ||
| radius = 4.dp, | ||
| color = PotiTheme.colors.white.copy(alpha = 0.5f), | ||
| offset = DpOffset(x = 0.dp, y = 4.dp), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Inner shadow 색상 대비를 확인해 주세요.
배경과 그림자가 모두 흰색 계열(white, white.copy(alpha = 0.5f))이라 알파 블렌딩 후에도 시각적 깊이감이 거의 나타나지 않을 수 있습니다. Figma의 실제 shadow 색상과 alpha를 확인하고, 필요한 경우 대비되는 디자인 토큰을 사용해 주세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/poti/android/presentation/main/MainBottomBar.kt` around
lines 51 - 64, Update the innerShadow configuration in MainBottomBar’s Row to
match the Figma shadow color and alpha, replacing the low-contrast white-based
Shadow values with the appropriate contrasting design token while preserving the
existing shape, radius, and offset.
Comment on lines
+32
to
50
| Row( | ||
| modifier = modifier | ||
| .clip(RoundedCornerShape(12.dp)) | ||
| .background(PotiTheme.colors.gray100) | ||
| .padding(all = 12.dp), | ||
| verticalArrangement = Arrangement.Center, | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| ) { | ||
| UserInfoItem( | ||
| infoContent = activityMessage, | ||
| modifier = Modifier.fillMaxWidth(), | ||
| ) | ||
| UserInfoItem( | ||
| infoContent = formatJoinedDate(joinedAt), | ||
| modifier = Modifier.fillMaxWidth(), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Composable | ||
| private fun UserInfoItem( | ||
| infoContent: String, | ||
| modifier: Modifier = Modifier, | ||
| ) { | ||
| Row( | ||
| modifier = modifier, | ||
| horizontalArrangement = Arrangement.Start, | ||
| .background(PotiTheme.colors.white) | ||
| .padding(horizontal = 12.dp, vertical = 16.dp), | ||
| horizontalArrangement = Arrangement.Center, | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| ) { | ||
| UserInfoText(text = activityMessage) | ||
|
|
||
| Icon( | ||
| imageVector = ImageVector.vectorResource(R.drawable.ic_bullet), | ||
| contentDescription = null, | ||
| modifier = Modifier.size(24.dp), | ||
| tint = PotiTheme.colors.gray800, | ||
| ) | ||
|
|
||
| Text( | ||
| text = infoContent, | ||
| color = PotiTheme.colors.black, | ||
| style = PotiTheme.typography.caption12m, | ||
| ) | ||
| UserInfoText(text = formatJoinedDate(joinedAt)) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
큰 글꼴에서 두 정보가 함께 표시되도록 레이아웃을 적응형으로 바꿔주세요.
가변 길이인 활동 문구와 가입일을 고정 아이콘과 같은 Row에 배치하면, 큰 글꼴 또는 좁은 화면에서 가입일이 밀리거나 잘릴 수 있습니다. FlowRow 또는 줄바꿈 가능한 구성으로 두 정보를 항상 확인할 수 있게 해주세요.
As per path instructions, “Kotlin 및 Jetpack Compose 모범 사례”를 적용했습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/poti/android/presentation/user/component/UserInfo.kt`
around lines 32 - 50, Update the UserInfo layout so the variable-length activity
message and joined date can wrap onto multiple lines instead of being forced
into one fixed Row with the bullet icon. Use an adaptive FlowRow or equivalent
Compose layout, preserving the existing spacing, alignment, styling, and content
so both UserInfoText values remain visible on large fonts or narrow screens.
Source: Path instructions
Comment on lines
91
to
+100
| Column( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .background(PotiTheme.colors.gray100) | ||
| .verticalScroll(scrollState) | ||
| .padding( | ||
| horizontal = 16.dp, | ||
| vertical = 20.dp, | ||
| ), | ||
| verticalArrangement = Arrangement.spacedBy(24.dp), | ||
| verticalArrangement = Arrangement.spacedBy(12.dp), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
헤더 아래 스크롤 영역을 남은 높이로 제한해주세요. 두 화면 모두 헤더가 먼저 높이를 차지한 부모 Column 안에서 스크롤 자식에 fillMaxSize()를 적용해 viewport가 부모 경계를 초과할 수 있습니다.
app/src/main/java/com/poti/android/presentation/user/mypage/MyPageScreen.kt#L91-L100:fillMaxSize()를fillMaxWidth().weight(1f)로 교체해주세요.app/src/main/java/com/poti/android/presentation/user/profile/ProfileScreen.kt#L75-L85: 동일하게fillMaxWidth().weight(1f)로 교체해주세요.
As per path instructions, “Jetpack Compose 성능 및 UI 최적화”를 적용했습니다.
📍 Affects 2 files
app/src/main/java/com/poti/android/presentation/user/mypage/MyPageScreen.kt#L91-L100(this comment)app/src/main/java/com/poti/android/presentation/user/profile/ProfileScreen.kt#L75-L85
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/poti/android/presentation/user/mypage/MyPageScreen.kt`
around lines 91 - 100, Restrict the header-below scroll areas to the remaining
parent height by replacing fillMaxSize() with fillMaxWidth().weight(1f) in
MyPageScreen.kt at lines 91-100 and ProfileScreen.kt at lines 75-85; preserve
the existing scrolling, background, padding, and arrangement behavior at both
sites.
Source: Path instructions
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderPrimary.kt (1)
116-127: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win토글 항목을
selectableGroup으로 묶어주세요.
PotiHeaderPrimaryToggleItem이selectable(role = Role.Tab)로 구현되어 있으므로,PotiHeaderPrimaryToggle의 부모Row에Modifier.selectableGroup()을 적용하면 탭 집합으로서의 접근성 탐색과 선택 상태 전달이 더 원활해집니다. 이때selectableGroup()을 적용하려면selectableGroupimport도 추가해주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderPrimary.kt` around lines 116 - 127, Apply Modifier.selectableGroup() to the parent Row in PotiHeaderPrimaryToggle so its PotiHeaderPrimaryToggleItem children form an accessible tab group, and add the required selectableGroup import.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@app/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderPrimary.kt`:
- Around line 116-127: Apply Modifier.selectableGroup() to the parent Row in
PotiHeaderPrimaryToggle so its PotiHeaderPrimaryToggleItem children form an
accessible tab group, and add the required selectableGroup import.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ec24d6cc-5182-4e34-af66-f7a00d7c76ce
📒 Files selected for processing (2)
app/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderPrimary.ktapp/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderSection.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderSection.kt
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
1 participant
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.
Related issue 🛠️
Work Description ✏️
Figma 컴포넌트 가이드 갱신분을 반영했습니다.
컴포넌트를 먼저 고치고 → 화면에 적용하는 순서로 커밋을 나눴습니다.
디자인 시스템
PotiHeaderSection진행중 3처럼 라벨에 포함PotiSearchBarPotiHeaderPageSearchPotiHeaderPrimaryToggle모집내역 | 참여내역토글 헤더PotiHeaderPagePotiModalButtonSECONDARY타입 추가BadgeButtoncontainerColor파라미터 추가 (기본값 흰색) —PotiHeaderPage·PotiHeaderPageSearch·PotiHeaderPrimary·PotiHeaderPrimaryToggle마이페이지 · 프로필
HistorySummaryCardUserInfoUserProfileemailnullable → 타인 프로필에서 이메일 숨김바텀 내비게이션
Modifier.innerShadow())Screenshot 📸
Uncompleted Tasks 😅
ProfileResponse에participationSummary가 없어임시로
0을 표시합니다. 서버 추가 후 후속 PR에서 연결 예정 (코드에 TODO)PotiHeaderPageSearch·PotiHeaderPrimaryToggle·ModalButtonType.SECONDARY. 컴포넌트만 먼저 만들었고 화면 연결은 후속 이슈PotiHeaderPrimaryToggle은 만들어 뒀지만,적용 시 모드 전환 동작까지 함께 바뀌어야 해서 후속 이슈로 분리했습니다
To Reviewers 📢
1. 명암 반전
기존
흰 배경 + 회색 섹션에서회색 배경 + 흰 섹션으로 뒤집혔습니다.헤더도 화면 배경과 이어져야 하는데 배경색이
white로 하드코딩되어 있어,헤더 컴포넌트 4종에
containerColor파라미터를 추가했습니다.기본값이 흰색이라 나머지 호출부는 영향이 없고, 마이페이지·프로필에서만 회색을 넘깁니다.
(Figma도 컴포넌트 기본값은 흰색이고 해당 화면에서만 회색으로 오버라이드되어 있습니다)
2.
HistorySummaryType.ALL을 남겨뒀습니다전체항목이 UI에서 빠졌지만 enum과HistorySummary.total은 유지했습니다.API 응답에 있고 분철 내역 진입 파라미터로도 쓰이기 때문입니다.
미사용이 된
user_history_all문자열은 제거했습니다.커밋 순서
앞쪽 7개는 컴포넌트 단위로, 뒤쪽 3개는 화면 적용으로 나눴습니다.
header-section커밋만 호출부(HistoryListScreen)가 함께 들어가 있습니다.칩 전환으로 화면 수정이 같이 필요해서 수정되었습니다.
Summary by CodeRabbit