[design] #108 종석 담당의 아이콘 교체 및 UI 컴포넌트 변경사항 반영#111
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough디자인 시스템 아이콘/이미지 리팩터링: 여러 기존 벡터 드로어블 10개 삭제, 9개 신규 드로어블 추가, 일부 드로어블 수정. 이를 참조하는 Compose 컴포저블들에서 리소스 참조, 레이아웃·패딩·타이포그래피 및 일부 시그니처(SettingProfileImage) 변경이 적용되었습니다. 변경사항
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25분 시
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 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: 2
🤖 Fix all issues with AI agents
In `@core/designsystem/src/main/res/drawable/icon_info_storke.xml`:
- Around line 1-20: The drawable resource filename has a typo
(icon_info_storke); rename the resource to icon_info_stroke and update all
references to it (e.g., R.drawable.icon_info_storke →
R.drawable.icon_info_stroke) such as in AnchoredDraggablePanel.kt; ensure the
new XML file name matches the resource name and rebuild resources so the new
identifier is generated and no other references remain to the old name.
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/AnchoredDraggablePanel.kt`:
- Line 240: In AnchoredDraggablePanel (the ImageVector.vectorResource call that
references R.drawable.icon_info_storke) the drawable name appears misspelled;
rename the drawable resource file from icon_info_storke to icon_info_stroke
(update the actual vector/XML filename and its entry in resources) and update
all usages to R.drawable.icon_info_stroke (including the
ImageVector.vectorResource call) so code and resource names match; run a
repository-wide search for "icon_info_storke" and replace occurrences, then
rebuild to confirm no references remain.
🧹 Nitpick comments (7)
feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/component/PeopleCountBottomSheet.kt (1)
77-93: 선택/미선택 항목 간 텍스트 정렬 불일치 (기존 이슈)이번 변경 자체는 문제 없으나, 기존부터 선택된 항목에만 24dp 아이콘이 표시되고 미선택 항목에는 아이콘이 없어서 텍스트 시작 위치가 수평으로 어긋납니다. 미선택 시에도 동일한 크기의
Spacer(Modifier.size(24.dp))를 두거나, 아이콘 자리를 항상 확보하는 방식으로 개선하면 정렬이 일관됩니다.♻️ 정렬 개선 제안
if (item == selectedItem) { Icon( modifier = Modifier.size(24.dp), imageVector = ImageVector.vectorResource(R.drawable.icon_check), contentDescription = null, tint = NekiTheme.colorScheme.primary500, ) + } else { + Spacer(modifier = Modifier.size(24.dp)) }core/designsystem/src/main/res/drawable/icon_check.xml (1)
1-11: 스트로크 기반 체크 아이콘으로의 전환이 깔끔합니다.전체적으로 디자인 시스템의 스트로크 스타일 통일 방향에 잘 맞는 변경입니다.
한 가지 선택적 제안:
strokeLineCap은round로 설정되어 있지만strokeLineJoin이 지정되지 않아 기본값인miter(뾰족한 꺾임)가 적용됩니다. 체크마크 꺾이는 부분에서 시각적 일관성을 위해strokeLineJoin="round"를 추가하는 것도 고려해 볼 수 있습니다.♻️ 선택적 변경 제안
android:strokeWidth="2" android:fillColor="#00000000" android:strokeColor="#B7B9C3" - android:strokeLineCap="round"/> + android:strokeLineCap="round" + android:strokeLineJoin="round"/>feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/PhotoBoothMarker.kt (1)
170-196:cachedBitmap유무에 따른Image수정자 중복 코드 개선 가능
cachedBitmap != null/null두 분기의Modifier체인이 동일합니다. 공통 modifier를 추출하면 가독성이 향상됩니다. 기존 코드이므로 필수는 아니지만, 참고용으로 남깁니다.♻️ 리팩토링 제안
+ val imageModifier = Modifier + .size(if (isFocused) FOCUSED_MARKER_IMAGE_SIZE.dp else MARKER_IMAGE_SIZE.dp) + .clip( + RoundedCornerShape( + if (isFocused) FOCUSED_MARKER_IMAGE_RADIUS.dp else MARKER_IMAGE_RADIUS.dp, + ), + ) if (cachedBitmap != null) { Image( - modifier = Modifier - .size(if (isFocused) FOCUSED_MARKER_IMAGE_SIZE.dp else MARKER_IMAGE_SIZE.dp) - .clip( - RoundedCornerShape( - if (isFocused) FOCUSED_MARKER_IMAGE_RADIUS.dp else MARKER_IMAGE_RADIUS.dp, - ), - ), + modifier = imageModifier, bitmap = cachedBitmap, contentScale = ContentScale.Crop, contentDescription = null, ) } else { Image( - modifier = Modifier - .size(if (isFocused) FOCUSED_MARKER_IMAGE_SIZE.dp else MARKER_IMAGE_SIZE.dp) - .clip( - RoundedCornerShape( - if (isFocused) FOCUSED_MARKER_IMAGE_RADIUS.dp else MARKER_IMAGE_RADIUS.dp, - ), - ), + modifier = imageModifier, painter = painterResource(R.drawable.icon_photo_booth_empty), contentScale = ContentScale.Crop, contentDescription = null, ) }feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/PhotoBoothDetailContent.kt (1)
102-115: 긴 브랜드명/지점명에 대한 텍스트 오버플로우 처리가 누락되어 있습니다.
title20SemiBold로 폰트 크기가 커졌고weight(1f)로 공간이 제한되었지만,brandName과branchNameText에maxLines와overflow = TextOverflow.Ellipsis처리가 없습니다. 긴 텍스트가 줄바꿈되어 카드 높이가 의도치 않게 늘어나거나 레이아웃이 깨질 수 있습니다.✏️ 텍스트 오버플로우 처리 제안
+import androidx.compose.ui.text.style.TextOverflow ... Text( text = photoBooth.brandName, color = NekiTheme.colorScheme.gray900, style = NekiTheme.typography.title20SemiBold, + maxLines = 1, + overflow = TextOverflow.Ellipsis, ) HorizontalSpacer(6.dp) Text( text = photoBooth.branchName, color = NekiTheme.colorScheme.gray600, style = NekiTheme.typography.body14Medium, + maxLines = 1, + overflow = TextOverflow.Ellipsis, )feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/AnchoredDraggablePanel.kt (2)
248-264: 빈 상태(Empty State) UI 개선 — 전반적으로 좋습니다.Column으로 변경하고 이미지와 줄바꿈 텍스트를 추가한 것은 UX 개선에 적합합니다. 다만,
top = 123.dp하드코딩 값은 다양한 화면 크기에서 의도한 위치에 표시되지 않을 수 있습니다. 가능하면 비율 기반이나 가용 공간 내 중앙 정렬 등을 고려해 보세요.
272-272: 매직 넘버128.dp— 상수화를 권장합니다.이전에는
MapConst.BOTTOM_NAVIGATION_BAR_HEIGHT.dp를 사용하여 의미가 명확했지만,128.dp로 변경하면서 이 값의 의도를 파악하기 어렵습니다. 바텀 네비게이션 높이 + 여백 등의 조합이라면 해당 계산을 상수로 정의하거나 주석을 추가하여 의미를 명확히 해주세요.✏️ 상수 활용 제안 예시
예를 들어
MapConst에 새 상수를 추가하거나, 기존 상수를 조합하는 방식:// MapConst.kt const val PANEL_LIST_BOTTOM_PADDING = 128- contentPadding = PaddingValues(bottom = 128.dp), + contentPadding = PaddingValues(bottom = MapConst.PANEL_LIST_BOTTOM_PADDING.dp),feature/mypage/impl/src/main/java/com/neki/android/feature/mypage/impl/profile/component/EditProfileImage.kt (1)
57-61:Color.Unspecifiedtint 사용 시 아이콘 색상 확인 필요.
tint = Color.Unspecified로 설정하면 벡터 드로어블의 원본 색상이 그대로 표시됩니다.icon_camera_filled리소스의 원본 색상이gray700배경 위에서 충분한 대비를 제공하는지 확인해 주세요.
ikseong00
left a comment
ikseong00
left a comment
There was a problem hiding this comment.
Icon 쪽에서 의견을 여쭙고자 합니다!
현재 피그마 상 사이즈, 색상이 다른 리소스더라도 하나로 통일해서 사이즈와 색상을 조정하는데,
의식적으로 Icon 에 사이즈와 틴트를 부여하는 건 어떨까요??
추후 같은 아이콘을 재사용해야할 때, 수정 및 관리에 어려움이 있지 않을까합니다!
Ojongseok
left a comment
Ojongseok
left a comment
There was a problem hiding this comment.
현재 피그마 상 사이즈, 색상이 다른 리소스더라도 하나로 통일해서 사이즈와 색상을 조정하는데,
의식적으로 Icon 에 사이즈와 틴트를 부여하는 건 어떨까요??
완전완전 동의합니다. 리소스를 교체하는 과정에서 size가 지정되어있지 않은 컴포넌트의 경우 따로 확인을 해서 지정해줄 필요가 있더라구요.
앞으로 Icon 컴포넌트 사용 시 size, tint 부여하시죠-!
2 participants
🔗 관련 이슈
📙 작업 설명
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
릴리스 노트
새로운 기능
스타일
기타