Feature/#132 프로필 수정 ui 구현 #146
Merged
Conversation
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
Walkthrough새 드롭다운 UI(DesignSystem) 추가, 마이페이지(Persona 드롭다운/모델) 및 프로필 화면 갱신, 메인/마이페이지/프로필 내비게이션 경로와 헬퍼 추가, 상태/사이드이펙트 확장, 문자열 리소스 추가. 마이페이지에서 프로필로, 프로필에서 마이페이지로의 이동 훅이 MainScreen과 각 NavGraph에 연동됨. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 사용자
participant Main as MainScreen
participant Nav as MainNavigator
participant MyG as mypageNavGraph
participant ProG as profileNavGraph
participant MP as MypageScreen
participant PR as ProfileScreen
User->>Main: 앱 실행/탭 전환
Main->>MyG: mypageNavGraph 등록(navigateToProfile)
Main->>ProG: profileNavGraph 등록(navigateToMypage)
User->>MP: 프로필 편집 클릭
MP->>MyG: navigateToProfile 호출
MyG->>Nav: navigator.navigateToProfile(navOptions)
Nav->>ProG: navigate(Profile)
Note over PR: 프로필 화면 표시
User->>PR: 완료/뒤로 등 액션
PR->>ProG: navigateToMypage 호출
ProG->>Nav: navigator.navigateToMypage(navOptions)
Nav->>MyG: navigate(Mypage)
sequenceDiagram
autonumber
actor User as 사용자
participant UI as SolplyPersonaDropDown
participant DS as SolplyBasicDropDown
User->>DS: 헤더 클릭(아이콘)
DS-->>UI: isDropped 토글 요청(onClickDropIcon)
DS->>DS: AnimatedVisibility(열림/닫힘)
alt 열림
DS->>UI: 콘텐츠 슬롯 렌더
loop 각 아이템(선택 제외)
User->>UI: 아이템 클릭
UI-->>Caller: onClickItem(index)
end
else 닫힘
DS-->>User: 헤더만 표시
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/ProfileScreen.kt (1)
133-136: 완료 버튼이 네비게이션을 호출하지 않습니다.
ProfileRoute에서navigateToMypage를 넘겨주지만, 실제 버튼의onClick이 비어 있어 사용자가 완료를 눌러도 아무 동작이 없습니다. 최소한 넘겨준 콜백을 호출하도록 연결해야 합니다.SolplyBasicButton( text = "완료", - onClick = {}, + onClick = onCompleteButtonClick, modifier = Modifier.padding(vertical = 24.dp, horizontal = 16.dp) )
🧹 Nitpick comments (1)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/navigation/ProfileNavigation.kt (1)
15-19: NavOptions 파라미터에 기본값을 두어 API 사용성을 높여주세요.기존
navigateMypage()류와 달리NavOptions를 항상 만들어 전달해야 해 ergonomics가 떨어집니다.null기본값을 허용하고 내부에서 그대로 위임하도록 하면 호출부 부담이 줄어듭니다.-fun NavController.navigateProfile( - navOptions: NavOptions -) { - navigate(Profile, navOptions) +fun NavController.navigateProfile( + navOptions: NavOptions? = null +) { + navigate(Profile, navOptions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/dropdown/SolplyBasicDropDown.kt(1 hunks)core/designsystem/src/main/res/drawable/ic_arrow_down.xml(1 hunks)feature/main/src/main/java/com/teamsolply/solply/main/MainNavigator.kt(2 hunks)feature/main/src/main/java/com/teamsolply/solply/main/MainScreen.kt(2 hunks)feature/mypage/src/main/java/com/teamsolply/solply/mypage/MypageContract.kt(1 hunks)feature/mypage/src/main/java/com/teamsolply/solply/mypage/MypageScreen.kt(3 hunks)feature/mypage/src/main/java/com/teamsolply/solply/mypage/component/SolplyPersonaDropDown.kt(1 hunks)feature/mypage/src/main/java/com/teamsolply/solply/mypage/model/DropDownPersonaItem.kt(1 hunks)feature/mypage/src/main/java/com/teamsolply/solply/mypage/navigation/MypageNavigation.kt(1 hunks)feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/ProfileScreen.kt(4 hunks)feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/navigation/ProfileNavigation.kt(1 hunks)feature/mypage/src/main/res/values/strings.xml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/component/SolplyPersonaDropDown.kt (1)
core/designsystem/src/main/java/com/teamsolply/solply/designsystem/theme/Theme.kt (1)
SolplyTheme(46-53)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/navigation/ProfileNavigation.kt (1)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/ProfileScreen.kt (1)
ProfileRoute(36-48)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/navigation/MypageNavigation.kt (1)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/MypageScreen.kt (1)
MypageRoute(33-45)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/ProfileScreen.kt (1)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/component/SolplyPersonaDropDown.kt (1)
SolplyPersonaDropDown(27-74)
🪛 detekt (1.23.8)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/MypageContract.kt
[warning] 14-16: The class or object MypageIntent is empty.
(detekt.empty-blocks.EmptyClassBlock)
🔇 Additional comments (6)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/model/DropDownPersonaItem.kt (1)
3-5: 단순한 페르소나 항목 모델 추가 확인했습니다.
구조가 명확하고 이후 컴포넌트 연동에 문제 없어 보입니다.feature/mypage/src/main/res/values/strings.xml (1)
6-6: 신규 문자열 리소스 추가 좋아요.
프로필 화면 문구와 일관성이 맞춰졌습니다.core/designsystem/src/main/res/drawable/ic_arrow_down.xml (1)
1-13: 벡터 아이콘 정의 문제없습니다.
드롭다운 토글에 필요한 스타일과 스트로크값이 적절합니다.feature/main/src/main/java/com/teamsolply/solply/main/MainScreen.kt (1)
339-354: 마이페이지 ↔ 프로필 네비게이션 연결 확인했습니다.
navOptions 구성과 popUpTo 설정이 기대한 스택 정리에 부합합니다.feature/mypage/src/main/java/com/teamsolply/solply/mypage/navigation/MypageNavigation.kt (1)
20-33: 탐색 훅 전달 로직 명확합니다.
새로운 navigateToProfile 파라미터가 정상적으로 라우트에 전파됩니다.feature/main/src/main/java/com/teamsolply/solply/main/MainNavigator.kt (1)
141-155: 네비게이터 보조 메서드 추가 확인했습니다.
탭 전환 외 개별 화면 호출 경로가 넓어져 재사용성이 좋아졌습니다.
Comment on lines
+73
to
+88
| Icon( | ||
| painter = painterResource(R.drawable.ic_arrow_down), | ||
| contentDescription = "", | ||
| modifier = Modifier | ||
| .padding(end = 20.dp, top = 14.dp, bottom = 14.dp) | ||
| .height(24.dp) | ||
| .width(24.dp) | ||
| .scale( | ||
| scaleX = 1f, | ||
| scaleY = if (isDropped) -1f else 1f | ||
| ) | ||
| .customClickable( | ||
| rippleEnabled = false, | ||
| onClick = onClickDropIcon | ||
| ) | ||
| ) |
There was a problem hiding this comment.
드롭다운 토글 아이콘에 접근성 라벨이 없습니다.
아이콘에 contentDescription=""로 설정된 상태에서 클릭 동작을 부여하면 스크린리더가 “레이블 없는 버튼”으로 인식해 조작 의도를 전달하지 못합니다. 최소한 기본 라벨을 읽어 주도록 설정해 주세요.
Icon(
painter = painterResource(R.drawable.ic_arrow_down),
- contentDescription = "",
+ contentDescription = defaultLabel,
modifier = Modifier
.padding(end = 20.dp, top = 14.dp, bottom = 14.dp)
.height(24.dp)
.width(24.dp)Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+39
to
+44
| defaultLabel = if (isSelected) dropDownContents.get(selectedIndex).label else placeholder, | ||
| onClickDropIcon = onClickDropIcon, | ||
| isDropped = isDropped, | ||
| modifier = modifier | ||
| ) { | ||
| dropDownContents.forEachIndexed { index, item -> |
There was a problem hiding this comment.
선택 인덱스 검증 없이 get을 호출하면 크래시가 납니다.
selectedIndex가 리스트 범위를 벗어난 상태(예: 서버 갱신으로 리스트가 짧아진 경우)에서 isSelected가 true면 get이 바로 IndexOutOfBoundsException을 던집니다. 안전하게 getOrNull을 사용해 플레이스홀더로 대체하는 방식을 권장합니다.
- defaultLabel = if (isSelected) dropDownContents.get(selectedIndex).label else placeholder,
+ defaultLabel = if (isSelected) {
+ dropDownContents.getOrNull(selectedIndex)?.label ?: placeholder
+ } else {
+ placeholder
+ },📝 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.
Suggested change
| defaultLabel = if (isSelected) dropDownContents.get(selectedIndex).label else placeholder, | |
| onClickDropIcon = onClickDropIcon, | |
| isDropped = isDropped, | |
| modifier = modifier | |
| ) { | |
| dropDownContents.forEachIndexed { index, item -> | |
| defaultLabel = if (isSelected) { | |
| dropDownContents.getOrNull(selectedIndex)?.label ?: placeholder | |
| } else { | |
| placeholder | |
| }, | |
| onClickDropIcon = onClickDropIcon, | |
| isDropped = isDropped, | |
| modifier = modifier | |
| ) { | |
| dropDownContents.forEachIndexed { index, item -> |
🤖 Prompt for AI Agents
In
feature/mypage/src/main/java/com/teamsolply/solply/mypage/component/SolplyPersonaDropDown.kt
around lines 39–44, calling dropDownContents.get(selectedIndex) without
validating selectedIndex can throw IndexOutOfBoundsException when the list has
shrunk; replace the unsafe get with a safe lookup and fallback to the
placeholder (e.g. use dropDownContents.getOrNull(selectedIndex)?.label ?:
placeholder) and/or ensure isSelected is only true when selectedIndex is within
bounds so the defaultLabel resolves safely to the selected item's label or the
placeholder.
Comment on lines
+14
to
+16
| sealed interface MypageIntent : UiIntent { | ||
|
|
||
| sealed interface MypageSideEffect : SideEffect | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
detekt 경고의 원인이 된 빈 인터페이스 본문 제거 필요
sealed interface MypageIntent : UiIntent {} 형태로 바뀌면서 detekt.empty-blocks.EmptyClassBlock 경고가 발생하고, 파이프라인에서 실패할 가능성이 높습니다. 아직 구현할 멤버가 없다면 기존처럼 본문 없이 선언하거나 TODO를 추가해 주세요.
-sealed interface MypageIntent : UiIntent {
-
-}
+sealed interface MypageIntent : UiIntentAs per static analysis hints
🧰 Tools
🪛 detekt (1.23.8)
[warning] 14-16: The class or object MypageIntent is empty.
(detekt.empty-blocks.EmptyClassBlock)
🤖 Prompt for AI Agents
In feature/mypage/src/main/java/com/teamsolply/solply/mypage/MypageContract.kt
around lines 14-16, the sealed interface is declared with an empty block "sealed
interface MypageIntent : UiIntent { }" which triggers
detekt.empty-blocks.EmptyClassBlock; remove the empty braces so it reads "sealed
interface MypageIntent : UiIntent" or, if you intend to add members later,
replace the empty block with a single TODO comment (e.g. "// TODO: add intents")
to satisfy static analysis.
Comment on lines
+91
to
97
| modifier = Modifier | ||
| .padding(top = 12.dp) | ||
| .customClickable( | ||
| rippleEnabled = false, | ||
| onClick = onProfileEditClick | ||
| ) | ||
| ) { |
There was a problem hiding this comment.
프로필 수정 탭 영역이 Material 최소 터치 타깃(48dp)을 만족하지 않습니다.
Row가 fillMaxWidth() 없이 텍스트 크기만큼만 확장되어 터치 영역이 매우 좁습니다. 접근성 측면에서 최소 48dp 정사각형 이상을 보장해야 하므로 가로폭과 패딩을 넓혀 주세요.
- Row(
- modifier = Modifier
- .padding(top = 12.dp)
- .customClickable(
- rippleEnabled = false,
- onClick = onProfileEditClick
- )
- ) {
+ Row(
+ modifier = Modifier
+ .fillMaxWidth()
+ .padding(top = 12.dp)
+ .customClickable(
+ rippleEnabled = false,
+ onClick = onProfileEditClick
+ )
+ .padding(horizontal = 20.dp, vertical = 12.dp),
+ verticalAlignment = Alignment.CenterVertically
+ ) {📝 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.
Suggested change
| modifier = Modifier | |
| .padding(top = 12.dp) | |
| .customClickable( | |
| rippleEnabled = false, | |
| onClick = onProfileEditClick | |
| ) | |
| ) { | |
| Row( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding(top = 12.dp) | |
| .customClickable( | |
| rippleEnabled = false, | |
| onClick = onProfileEditClick | |
| ) | |
| .padding(horizontal = 20.dp, vertical = 12.dp), | |
| verticalAlignment = Alignment.CenterVertically | |
| ) { | |
| // ... existing content ... | |
| } |
🤖 Prompt for AI Agents
In feature/mypage/src/main/java/com/teamsolply/solply/mypage/MypageScreen.kt
around lines 91–97, the profile-edit Row only sizes to its text and its touch
target is smaller than the Material minimum (48dp); change the Modifier to
expand the tappable area by adding fillMaxWidth() and increasing horizontal
padding, and enforce a minimum height (e.g., heightIn(min = 48.dp) or
sizeIn(minHeight = 48.dp)) so the customClickable covers a 48dp-high touch
target across the full width; ensure the Modifier order applies
fillMaxWidth()/padding()/heightIn() before customClickable so the whole area is
clickable.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
📌 PR 요약
🌱 작업한 내용
🌱 PR 포인트
📸 스크린샷
|스크린샷|
mypage_profile_nav.webm
📮 관련 이슈
Summary by CodeRabbit
신기능
스타일
기타