Refac/#8 화면 변경사항 수정#11
Conversation
CHOOSLA
requested review from
CHOOSLA,
Lee-Jong-Jin,
taemin3,
vivivim and
yangjiseonn
Walkthrough의존성에 Material 및 아이콘 라이브러리를 추가하고 Compose BOM을 갱신했다. 네비게이션을 Dashboard/Delivery/Cart/Orders/Parts/Employee 기반으로 재구성하고 FAB를 Parts로 연결했다. 공통 컴포넌트(CommonButton, CommonTextField)를 추가하고 Part 화면을 Scaffold로 래핑했으며 다수의 벡터 드로어블과 런처 아이콘을 교체/추가했다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 사용자
participant App as MainScreen
participant BN as BottomNavigationBar
participant Nav as NavHostController
participant Dash as DashboardScreen
participant Del as DeliveryScreen
participant Cart as CartScreen
participant Ord as OrderScreen
participant Parts as PartsScreen
rect rgba(230,240,255,0.4)
note over App,Nav: 앱 시작 — startDestination = ROUTE_DASHBOARD
App->>Nav: setStartDestination(ROUTE_DASHBOARD)
Nav->>Dash: Compose DashboardScreen
end
rect rgba(240,255,230,0.4)
User->>BN: 하단 탭 선택
BN->>Nav: navigate(ROUTE_DELIVERY){popUpTo/saveState/launchSingleTop/restoreState}
Nav->>Del: Compose DeliveryScreen
end
rect rgba(255,245,230,0.5)
User->>App: FAB 클릭
App->>Nav: navigate(ROUTE_PARTS)
Nav->>Parts: Compose PartsScreen
end
alt 다른 탭 선택
User->>BN: Cart / Orders 탭
BN->>Nav: navigate(ROUTE_CART or ROUTE_ORDERS)
Nav->>Cart: Compose CartScreen
Nav->>Ord: Compose OrderScreen
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
♻️ Duplicate comments (5)
app/src/main/res/drawable/delivery.xml (1)
1-13: clip-path/viewport 불일치 정리 권장
- viewport가 25인데 clip-path는 24 범위(오프셋 포함)로 정의되어 있어 미세한 클리핑/정렬 오차를 유발할 수 있습니다. 동일 기준(24 또는 25)으로 맞추는 정리를 권장합니다.
- 색상/사이즈 테마화 관련 코멘트는 employee.xml에 남긴 내용과 동일합니다.
app/src/main/res/drawable/settings.xml (1)
1-13: viewport(25) 대비 clip-path(24.xxx) 불일치그룹의 clip-path가 24.077×24.333로 설정되어 viewport(25)와 불일치합니다. 가장자리 일부가 의도치 않게 잘릴 수 있으므로 동일 기준으로 정규화하는 것을 권장합니다. 색상/사이즈 테마화는 employee.xml 코멘트 참조.
app/src/main/res/drawable/cart.xml (1)
1-13: 아이콘 리소스 정규화(크기/클리핑/컬러)
- viewport 25 대비 clip-path 24.077×24.333 불일치 정리 권장.
- 색상 하드코딩(#36393F) 대신 테마 틴트/컬러 리소스 사용 권장.
- 아이콘 크기(24dp) 기준 통일 검토.
app/src/main/res/drawable/orders.xml (1)
1-9: 색상/사이즈 테마화
- 하드코딩 색상(#36393F)을 테마 틴트 또는 color 리소스로 전환 권장(다크모드, 동적 컬러 대응).
- 24dp 표준으로의 사이즈 통일 검토.
app/src/main/res/drawable/parts.xml (1)
1-9: 일관성 유지(24dp 기준·테마 색상)
- 24dp 기준(또는 적어도 모듈 내 통일)으로 사이즈 정규화 권장.
- 색상 하드코딩 대신 테마 틴트/리소스 사용 권장.
🧹 Nitpick comments (6)
app/src/main/res/drawable/employee.xml (1)
1-9: 아이콘 사이즈/테마 일관화 권장
- Material 가이드에 맞춰 아이콘을 24dp 기준으로 통일하는 것을 권장합니다(현재 25dp/viewport 25). 바텀내비/툴바 정렬 이슈를 줄일 수 있습니다.
- 하드코딩된 색상(#36393F) 대신 테마 기반 틴트 사용을 권장합니다.
- Compose: Icon(...)에 tint를 적용하고 벡터는 단색(예: #000000) 혹은 tint=Color.Unspecified 전략을 일관되게 사용.
- View/XML: 벡터 루트에 android:tint="?attr/colorControlNormal" 또는 색상을 color 리소스로 추출(+ night 리소스)해 다크모드 대응.
- 방향성 있는 아이콘의 경우 RTL 미러링이 필요하면 android:autoMirrored="true" 고려.
즉시 필수는 아니나, 지금 리소스 추가 시점에 맞춰 정리하면 이후 유지보수 비용이 크게 줄어듭니다.
app/src/main/res/drawable/search.xml (1)
1-13: 스트로크 아이콘의 틴트/다크모드 대응
- strokeColor(#36393F)도 테마 기반으로 전환 권장(Icon tint 또는 color 리소스).
- strokeWidth(2)가 다른 아이콘 대비 두꺼워 보일 수 있어, 바텀내비 등에서 균형 확인 필요.
다른 화면(특히 BottomNavigation)에서 시각적 굵기/명암이 일관적인지 한번 확인 부탁드립니다.
app/src/main/java/com/sampoom/android/core/ui/component/CommonButton.kt (1)
112-173: leadingIcon 지원 일관성 개선 권장Primary와 Secondary 변형은 leadingIcon을 지원하지만, Outlined, Ghost, Neutral 변형에서는 지원하지 않습니다. 모든 변형에서 leadingIcon을 지원하도록 일관성을 개선하면 사용성이 향상됩니다.
참고: PR 설명에서 공통 컴포넌트가 초기 버전이며 추후 수정 예정이라고 언급되었으므로, 향후 업데이트 시 고려하시기 바랍니다.
app/src/main/java/com/sampoom/android/core/ui/component/CommonTextField.kt (1)
39-45: 테마 색상 일관성 개선 권장하드코딩된 색상 값들(
0xFF1C1C1E,0xFFF3F3F3,0xFF666666,0xFFCCCCCC)이 사용되고 있습니다. MaterialTheme.colorScheme의 기본 색상들을 활용하면 테마 변경 시 더 일관된 UI를 제공할 수 있습니다.예시:
val containerColor = if (variant == TextFieldVariant.Filled) { cs.surfaceVariant } else Color.Transparent val unfocusedBorderColor = cs.outline참고: PR 설명에서 공통 컴포넌트가 초기 버전이며 추후 수정 예정이라고 언급되었으므로, 향후 업데이트 시 고려하시기 바랍니다.
gradle/libs.versions.toml (1)
27-27: androidx-material-icons-extended의 버전 관리 방식 통일 필요
현재material-icons-core에는version.ref = "materialIconsCore"를 적용하고 있지만,material-icons-extended는 BOM에만 의존해 별도 버전 참조가 없습니다. 둘 중 하나를 선택해 –
libs.versions.toml의[versions]에materialIconsExtended추가 후androidx-material-icons-extended에version.ref = "materialIconsExtended"적용
또는androidx-material-icons-core의version.ref를 제거해 둘 다 BOM으로 일괄 관리하시어 아이콘 라이브러리 간 버전 관리 일관성을 맞추세요.
app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt (1)
121-141: currentBackStackEntryAsState를 루프 밖으로 이동하고, 선택 상태는 hierarchy로 판별하세요.forEach 내부에서 상태를 매 아이템마다 구독하고 있고, route 비교는 중첩 그래프에서 부정확할 수 있습니다.
적용 diff:
NavigationBar { - bottomNavItems.forEach { item -> - val navBackStackEntry by navController.currentBackStackEntryAsState() - val currentDestination = navBackStackEntry?.destination + val navBackStackEntry by navController.currentBackStackEntryAsState() + val currentDestination = navBackStackEntry?.destination + + bottomNavItems.forEach { item -> NavigationBarItem( icon = { Icon(painterResource(id = item.icon), contentDescription = stringResource(item.title)) }, label = { Text(stringResource(item.title)) }, - selected = currentDestination?.route == item.route, + selected = currentDestination?.hierarchy?.any { it.route == item.route } == true, onClick = { navController.navigate(item.route) { popUpTo(navController.graph.startDestinationId) { saveState = true } launchSingleTop = true restoreState = true } } ) } }필요한 import 추가:
import androidx.navigation.NavDestination.Companion.hierarchy
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/src/main/ic_launcher-playstore.pngis excluded by!**/*.png
📒 Files selected for processing (17)
app/build.gradle.kts(1 hunks)app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt(5 hunks)app/src/main/java/com/sampoom/android/core/ui/component/CommonButton.kt(1 hunks)app/src/main/java/com/sampoom/android/core/ui/component/CommonTextField.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt(1 hunks)app/src/main/res/drawable-night/ic_launcher_foreground.xml(1 hunks)app/src/main/res/drawable/cart.xml(1 hunks)app/src/main/res/drawable/dashboard.xml(1 hunks)app/src/main/res/drawable/delivery.xml(1 hunks)app/src/main/res/drawable/employee.xml(1 hunks)app/src/main/res/drawable/ic_launcher_foreground.xml(1 hunks)app/src/main/res/drawable/orders.xml(1 hunks)app/src/main/res/drawable/parts.xml(1 hunks)app/src/main/res/drawable/search.xml(1 hunks)app/src/main/res/drawable/settings.xml(1 hunks)app/src/main/res/values/strings.xml(1 hunks)gradle/libs.versions.toml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (1)
app/src/main/java/com/sampoom/android/feature/part/domain/model/PartList.kt (1)
items(3-11)
app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt (1)
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (1)
PartScreen(17-85)
🔇 Additional comments (11)
app/src/main/res/drawable-night/ic_launcher_foreground.xml (1)
14-17: 앱 런처 아이콘 업데이트 확인벡터 아이콘의 pathData가 새로운 디자인으로 교체되었습니다. fillColor는 유지되며, PR 목적에 맞게 앱 아이콘이 변경되었습니다.
app/build.gradle.kts (1)
82-84: Material 라이브러리 의존성 추가 확인새로운 공통 UI 컴포넌트(CommonButton, CommonTextField) 및 Material 아이콘을 지원하기 위한 의존성이 적절히 추가되었습니다.
gradle/libs.versions.toml (1)
13-13: Compose BOM 버전 업데이트 확인Compose BOM이 최신 버전으로 업데이트되어 최신 기능과 버그 수정을 활용할 수 있습니다.
app/src/main/java/com/sampoom/android/core/ui/component/CommonButton.kt (1)
59-83: Primary 버튼 구현 확인Primary 버튼 변형이 적절히 구현되었으며, MaterialTheme의 colorScheme를 올바르게 활용하고 있습니다. leadingIcon 지원도 잘 구현되어 있습니다.
app/src/main/java/com/sampoom/android/core/ui/component/CommonTextField.kt (1)
47-57: 비밀번호 가시성 토글 기능 구현 확인비밀번호 입력 필드의 가시성 토글 기능이 적절히 구현되었습니다. IconButton과 VisualTransformation을 사용하여 사용자 경험을 개선했습니다.
app/src/main/res/drawable/dashboard.xml (1)
1-9: 대시보드 아이콘 리소스 추가 확인새로운 대시보드 네비게이션 아이콘이 적절히 추가되었습니다. 벡터 드로어블 형식으로 다양한 화면 밀도에서 선명하게 표시됩니다.
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (1)
23-29: Scaffold 래핑 및 패딩 처리 확인PartScreen이 Scaffold로 래핑되어 Material Design 구조를 따르도록 개선되었습니다.
innerPadding을 적절히 적용하여 시스템 바와의 레이아웃 충돌을 방지하고 있습니다.app/src/main/res/values/strings.xml (1)
5-8: 이전 네비게이션 문자열 키 참조 제거 확인됨이전(nav_part, nav_inventory, nav_profile, nav_setting) 키는 코드·XML에서 더 이상 참조되지 않으며, 새로운 키(nav_dashboard, nav_delivery, nav_cart, nav_order)는 AppNavHost.kt에서 올바르게 사용되고 있습니다. 변경 사항 승인합니다.
app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt (3)
53-59: 로그인 상태 하드코딩(TODO) — 릴리스 전 교체 필요
isLoggedIn = true는 임시입니다. 배포 전 반드시 AuthRepository 기반의 실제 인증 상태로 대체하세요. 라우팅 플로우(로그인→홈 popUpTo 포함) 재검증도 필요합니다.
68-69: PARTS 라우트 상위 그래프 등록과 PartScreen 연결 적절상위 NavController로 진입해야 하는 상세/파트 흐름에 맞는 구성입니다. FAB에서 parent NavController를 사용하는 구조와도 일치합니다.
84-91: 내부 NavHost startDestination과 라우트 매핑 적절
ROUTE_DASHBOARD시작과 각 화면 매핑이 일관적입니다. BottomNavigation 동작과도 호환됩니다.
| // Destructive/Neutral (dark) filled – matches the black fill example | ||
| ButtonVariant.Neutral -> { | ||
| Button( | ||
| onClick = onClick, | ||
| enabled = enabled, | ||
| shape = shape, | ||
| modifier = modifier.height(height), | ||
| colors = ButtonDefaults.buttonColors( | ||
| containerColor = Color(0xFF000000), | ||
| contentColor = Color.White, | ||
| disabledContainerColor = cs.onSurface.copy(alpha = 0.12f), | ||
| disabledContentColor = cs.onSurface.copy(alpha = 0.38f), | ||
| ) | ||
| ) { | ||
| Text( | ||
| text = text, | ||
| style = MaterialTheme.typography.labelLarge, | ||
| fontWeight = FontWeight.SemiBold | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
테마 일관성을 위해 하드코딩된 색상 제거 권장
Neutral 변형에서 Color(0xFF000000)와 Color.White를 직접 사용하고 있습니다. MaterialTheme의 colorScheme를 활용하면 다크/라이트 테마 전환 시 일관성을 유지할 수 있습니다.
다음과 같이 수정하는 것을 권장합니다:
ButtonVariant.Neutral -> {
Button(
onClick = onClick,
enabled = enabled,
shape = shape,
modifier = modifier.height(height),
colors = ButtonDefaults.buttonColors(
- containerColor = Color(0xFF000000),
- contentColor = Color.White,
+ containerColor = cs.surface,
+ contentColor = cs.onSurface,
disabledContainerColor = cs.onSurface.copy(alpha = 0.12f),
disabledContentColor = cs.onSurface.copy(alpha = 0.38f),
)
) {🤖 Prompt for AI Agents
In app/src/main/java/com/sampoom/android/core/ui/component/CommonButton.kt
around lines 152 to 173, remove the hardcoded Color(0xFF000000) and Color.White
in the Neutral branch and use MaterialTheme.colorScheme values instead: set
containerColor to an appropriate color from the theme (e.g., cs.surface or
cs.primaryContainer) and set contentColor to cs.onSurface (or cs.onPrimary if
you pick a primary container), leaving the existing
disabledContainerColor/disabledContentColor that already use cs.onSurface;
ensure you reference the existing cs = MaterialTheme.colorScheme variable (or
MaterialTheme.colorScheme directly) so the button responds to light/dark theme
changes.
There was a problem hiding this comment.
공통 컴포넌트는 초기 세팅만 진행했어요
나중에 컬러 코드 정의 후 수정할 계획이에요
There was a problem hiding this comment.
@Sangyoon98, 이해했습니다! 초기 세팅 단계에서 나중에 정리하실 계획이라면 문제없습니다.
향후 작업을 추적하기 위해 GitHub 이슈를 생성하시겠습니까? 원하신다면 제가 이슈를 생성해드릴 수 있습니다. 이슈에는 다음 내용이 포함됩니다:
- 공통 컴포넌트의 컬러 코드 정의 및 MaterialTheme 적용
- 현재 하드코딩된 색상을 테마 기반 색상으로 리팩토링
🐰✨
| android:pathData="M0,0h512v512h-512z" | ||
| android:fillColor="#ffffff"/> |
There was a problem hiding this comment.
Adaptive 아이콘 전면(포어그라운드)에 배경 사각형 제거 필요
ic_launcher_foreground는 배경 레이어에 의존해야 하며, 전면 레이어에 전체 흰색 사각형을 넣으면 배경 레이어/동적 컬러를 가려버립니다. 다양한 런처/테마에서 잘못된 표시(흰색 정사각형)로 보일 수 있습니다.
아래처럼 전체 배경 사각형 path를 제거하세요.
<path
- android:pathData="M0,0h512v512h-512z"
- android:fillColor="#ffffff"/>
+ <!-- Removed full-bleed background rectangle from foreground -->또한 전면 레이어는 투명 영역을 충분히 포함하고, 백그라운드 색/그래픽은 ic_launcher_background.xml에서 관리되어야 합니다. 필요 시 전면 아트웍의 스케일/트랜스레이트를 재조정해 안전 영역을 맞춰 주세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/src/main/res/drawable/ic_launcher_foreground.xml around lines 11-12,
remove the full white background rectangle path
(android:pathData="M0,0h512v512h-512z" with fillColor="#ffffff") from the
foreground so it no longer obliterates adaptive/background layers; ensure the
foreground drawable contains transparent areas and only the foreground artwork,
and if needed adjust the artwork’s scale/translate to fit the safe/adaptive
inset so background color/graphics are provided solely by
ic_launcher_background.xml.
There was a problem hiding this comment.
앱 아이콘 색상 컨셉이 화이트여서 하드코딩 된 것이라 상관 없을 것 같아요
There was a problem hiding this comment.
@Sangyoon98, 알겠습니다! 화이트 컬러 컨셉으로 의도된 디자인이시라면 문제없습니다. 앱 아이콘의 디자인 방향을 존중하겠습니다. 😊
✏️ Learnings added
Learnt from: Sangyoon98
PR: 33-Auto/Sampoom-Management-Android#11
File: app/src/main/res/drawable/ic_launcher_foreground.xml:11-12
Timestamp: 2025-10-10T15:24:11.370Z
Learning: In the Sampoom Management Android project, the app icon uses a white color concept, and the hardcoded white background in ic_launcher_foreground.xml is intentional and acceptable per the design requirements.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
CHOOSLA
left a comment
CHOOSLA
left a comment
There was a problem hiding this comment.
슬랙에서 온 메시지로 확인했습니다. 코드 래빗의 기능으로 자동으로 정리되는 점이 좋네요
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt (2)
53-54: TODO: 인증 로직 리팩토링이 필요합니다.임시 로그인 상태 확인 로직을
AuthRepository로 이동하는 작업이 남아있습니다. 이는 향후 보안 및 유지보수성 향상을 위해 중요한 작업입니다.이 작업을 추적하기 위한 별도의 이슈를 생성하시겠습니까?
144-167: 임시 플레이스홀더 화면 - 향후 구현 필요현재 임시 화면들이 단순 텍스트만 표시하고 있습니다. PR 목적에 명시된 대로 공통 컴포넌트 구성 작업이 진행 중이므로, 이 화면들은 각 feature 모듈에서 적절히 구현되어야 합니다.
추천 사항:
- 각 화면을 해당 feature 모듈로 분리 (
feature/dashboard,feature/delivery등)- 적절한 ViewModel과 UI 상태 관리 추가
- 공통 컴포넌트를 활용한 일관된 UI 구성
각 feature 모듈의 기본 구조를 생성하는 데 도움이 필요하시면 말씀해주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt (1)
app/src/main/java/com/sampoom/android/feature/part/ui/PartScreen.kt (1)
PartScreen(17-85)
🔇 Additional comments (4)
app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt (4)
72-93: LGTM! Scaffold 구조와 네비게이션 설정이 올바릅니다.
MainScreen의 Scaffold 구조가 Material3 가이드라인을 따르고 있으며, FAB와 하단 네비게이션 바를 적절히 배치했습니다.parentNavController를 FAB에 전달하여 상위 레벨 화면(ROUTE_PARTS)으로 정확하게 네비게이션하고 있습니다.
95-110: 이전 리뷰 피드백이 정확히 반영되었습니다!함수명을
FloatingActionButton에서PartsFab로 변경하여 Material3의FloatingActionButton과의 이름 충돌을 해결했습니다. 네비게이션 로직도 올바르게 구현되어 있습니다:
saveState와restoreState로 상태 보존launchSingleTop으로 중복 인스턴스 방지popUpTo로 백스택 관리Based on past review comments.
112-142: LGTM! 하단 네비게이션 바 구현이 우수합니다.네비게이션 바 구현이 Material3 가이드라인을 준수하며, 상태 관리가 적절합니다.
popUpTo,saveState,launchSingleTop,restoreState옵션을 사용하여 원활한 사용자 경험을 제공합니다.
43-46: 리소스 참조 검증 완료
모든 드로어블 및 문자열 리소스가 정의되어 있습니다.
4 participants
📣 Related Issue
📝 Summary
🙏 Question & PR point
공통 컴포넌트는 초기 버전으로 추 후 수정 예정입니다.
📬 Reference
Summary by CodeRabbit
New Features
Style
Chores