홈 화면 및 GNB 구현 #39
홈 화면 및 GNB 구현 #39
Conversation
📝 WalkthroughWalkthrough홈 화면과 GNB(Global Navigation Bar)를 구현하기 위해 새로운 feature/main 모듈을 추가합니다. 이 변경사항은 MVU 아키텍처를 지원하는 BaseViewModel을 core/ui에 도입하고, 네비게이션 구조를 재정의(LoginRoute, MainGraph, MainRoute)합니다. 또한 Kermit 로깅 라이브러리를 추가하고, 홈 화면 및 메인 탭 기능을 구성하는 ViewModel, Intent, SideEffect, UI 컴포넌트를 구현합니다. 다양한 벡터 드로어블 리소스와 문자열 리소스도 추가됩니다. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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
🤖 Fix all issues with AI agents
In `@core/design-system/src/main/res/drawable/ic_drop_down_arrow.xml`:
- Around line 6-11: The path element in ic_drop_down_arrow.xml hardcodes
fillColor and strokeColor to `#464646` which breaks dark-theme contrast; update
the drawable to avoid fixed colors by replacing android:fillColor and
android:strokeColor with a theme attribute (e.g., ?attr/colorOnBackground or a
dedicated ?attr/colorIcon) or remove those attributes so the view can apply a
tint at runtime; locate the <path ... android:fillColor="#464646"
android:strokeColor="#464646" ... /> and swap to the theme attribute or null it,
and ensure consuming views call setImageTintList or use android:tint to apply
the correct themed color.
In `@core/design-system/src/main/res/drawable/ic_home_unselected.xml`:
- Around line 6-10: The path in ic_home_unselected.xml hardcodes
strokeColor="#171717" which breaks theme/dark-mode support; replace the
hardcoded android:strokeColor on the <path> with a color resource reference
(e.g., `@color/icon_default`) and add a colors.xml entry named icon_default in
core/design-system/src/main/res/values/colors.xml (and a corresponding
values-night/colors.xml for dark mode) so icons follow theme colors instead of
fixed hex values.
In `@core/design-system/src/main/res/values/strings.xml`:
- Line 19: Rename the malformed string resource key homt_empty_goal_guide to
home_empty_goal_guide in strings.xml and update all usages to match (for example
the reference in EmptyGoalGuide.kt), ensuring the string value remains unchanged
and any import/lookup (e.g., R.string.homt_empty_goal_guide) is replaced with
R.string.home_empty_goal_guide so resource lookups compile correctly.
In `@feature/main/src/main/java/com/twix/home/component/WeeklyCalendar.kt`:
- Around line 60-66: LaunchedEffect(days.first()) runs on first composition and
with dragSumPx initial 0f will call onUpdateVisibleDate(days.last()); guard the
effect so it only triggers after a user swipe by adding a check (e.g., require
dragSumPx != 0f or track a hasSwiped/initialized boolean remembered and set only
when a drag completes) before calling onUpdateVisibleDate; update the
LaunchedEffect containing LaunchedEffect(days.first()), dragSumPx, and
onUpdateVisibleDate to return early when no user swipe has occurred so initial
composition does not change visibleDate.
In `@feature/main/src/main/java/com/twix/main/component/MainBottomBar.kt`:
- Around line 68-94: The clickable area is currently applied only to the Image,
so the tab label (AppText) is not interactive; move the clickable modifier from
the Image to the parent Column (apply modifier.clickable(...) with the same
indication, interactionSource, and onClick that performs
haptic.performHapticFeedback(HapticFeedbackType.TextHandleMove) then calls
onClick()), and remove the clickable modifier from the Image so the entire
Column (icon + label) is tappable while preserving the original
interactionSource, haptic behavior, and onClick handler.
🧹 Nitpick comments (27)
core/design-system/src/main/res/drawable/ic_couple_unselected.xml (1)
9-10: 하드코딩된 색상 값이 다크 모드를 지원하지 않습니다.
#171717은 거의 검정에 가까운 색상으로, 다크 모드에서 어두운 배경 위에 표시될 경우 아이콘이 보이지 않을 수 있습니다. 디자인 시스템 가이드라인에 따라, 테마에 반응하는 색상 리소스를 참조하는 것이 좋습니다.예를 들어:
@color/icon_on_surface같은 색상 리소스를 정의하고 참조- 또는
?attr/colorOnSurface같은 테마 속성 사용다크 모드 지원 계획이 있다면 지금 색상 리소스로 분리해 두시면, 향후 테마 변경 시 일괄 적용이 훨씬 수월해집니다.
♻️ 개선 제안
core/design-system/src/main/res/values/colors.xml에 색상 정의:<color name="icon_default">#171717</color>다크 모드용
core/design-system/src/main/res/values-night/colors.xml:<color name="icon_default">#E0E0E0</color>그 후 drawable에서 참조:
- android:fillColor="#00000000" - android:strokeColor="#171717"/> + android:fillColor="@android:color/transparent" + android:strokeColor="@color/icon_default"/>core/design-system/src/main/res/drawable/ic_couple_selected.xml (1)
1-11: 아이콘 쌍의 구조가 잘 설계되었습니다.
ic_couple_unselected.xml과 동일한 pathData를 사용하면서fillColor만 다르게 설정하여 선택/미선택 상태를 표현한 점이 좋습니다. 이렇게 하면 두 상태 간 전환 시 아이콘 형태가 일관되게 유지됩니다.다만, 앞서 언급한 것처럼
#171717하드코딩 색상은 다크 모드 지원을 위해 색상 리소스로 분리하는 것을 권장드립니다. 두 파일 모두 동일한 색상을 사용하므로, 한 번 리소스로 정의해 두면 유지보수가 편리해집니다.,
core/ui/src/main/java/com/twix/ui/extension/NoRippleClickable.kt (1)
9-15: 리플 효과 제거 확장 함수가 깔끔하게 구현되었습니다.기본 구현은 정확하며,
indication = null과MutableInteractionSource를 활용한 패턴이 적절합니다.다만 코딩 가이드라인에서 요구하는 커스터마이징 포인트를 고려했을 때,
enabled와 접근성 관련 파라미터를 추가하면 다양한 사용 사례에서 재사용성이 높아질 것으로 보입니다. 예를 들어, 버튼이 비활성화된 상태나 스크린 리더를 위한 라벨이 필요한 경우에도 대응할 수 있습니다.♻️ 커스터마이징 포인트 확장 제안
`@Composable` fun Modifier.noRippleClickable( + enabled: Boolean = true, + onClickLabel: String? = null, onClick: () -> Unit, ): Modifier = clickable( indication = null, interactionSource = remember { MutableInteractionSource() }, + enabled = enabled, + onClickLabel = onClickLabel, onClick = onClick, )코딩 가이드라인 기준 - 커스터마이징 포인트가 적절한가?에 따른 제안입니다.
core/design-system/src/main/res/drawable/ic_alert.xml (1)
10-10: Dark Mode 지원을 위해 테마 속성 사용을 권장합니다.현재
#464646색상이 하드코딩되어 있어 Dark Mode에서 아이콘이 잘 보이지 않을 수 있습니다. 디자인 시스템 가이드라인에 따라 테마 속성을 참조하거나, colors.xml에 정의된 색상 리소스를 사용하면 라이트/다크 모드 전환 시 자동으로 적절한 색상이 적용됩니다.♻️ 테마 속성을 사용한 개선 예시
- android:strokeColor="#464646" + android:strokeColor="?attr/colorOnSurface"또는 colors.xml에 별도 색상 리소스를 정의하고 참조:
- android:strokeColor="#464646" + android:strokeColor="@color/icon_default"core/design-system/src/main/res/drawable/ic_plus.xml (1)
10-10: 흰색 하드코딩이 의도된 것인지 확인이 필요합니다.FAB(Floating Action Button) 등 특정 컴포넌트에서 항상 흰색 아이콘을 사용하는 것이 의도라면 현재 구현이 적절합니다. 다만, 다른 곳에서도 재사용될 가능성이 있다면
?attr/colorOnPrimary같은 테마 속성을 사용하는 것이 Dark Mode 대응에 유리합니다.현재 사용처(AddGoalButton)를 고려할 때 의도된 디자인으로 보이므로 확인 차원에서 코멘트 남깁니다.
core/design-system/src/main/res/drawable/ic_empty_goal_arrow.xml (1)
8-8: 데코레이션 요소도 테마 대응을 고려해주세요.Empty state 가이드에 사용되는 장식용 화살표이지만,
#C6C6C6색상이 하드코딩되어 있어 Dark Mode에서 배경과 대비가 낮아질 수 있습니다. colors.xml에 정의된 색상 리소스를 참조하면 테마 전환 시 자동으로 적절한 색상이 적용됩니다.♻️ 색상 리소스 참조 예시
- android:fillColor="#C6C6C6" + android:fillColor="@color/illustration_secondary"core/design-system/src/main/res/values/strings.xml (1)
1-21: 다국어 지원 구조를 확인해주세요.현재 기본
values/strings.xml에 한국어 문자열이 직접 포함되어 있습니다. 향후 다국어 지원을 고려한다면, 다음과 같은 구조가 권장됩니다:
values/strings.xml: 영어(기본값)values-ko/strings.xml: 한국어현재 앱이 한국어 전용이라면 문제없지만, 추후 영어 등 다른 언어 지원 계획이 있다면 구조 개선을 고려해보시는 것이 좋겠습니다.
core/design-system/src/main/res/drawable/ic_stats_unselected.xml (1)
10-10: Dark Mode 지원을 위해 테마 색상 참조를 고려해 보세요.현재
strokeColor가#171717로 하드코딩되어 있습니다. 이 색상은 Dark Mode에서 배경과 대비가 낮아 아이콘이 잘 보이지 않을 수 있습니다.
@color/리소스나 테마 속성(?attr/colorOnSurface등)을 참조하면 Dark Mode에서도 자동으로 적절한 색상이 적용됩니다. 다만, 디자인 시스템 전체 아이콘에서 동일한 방식을 사용하고 있다면, 일관성을 위해 현재 상태를 유지하고 추후 일괄 개선하는 방향도 괜찮습니다.이 부분은 팀의 디자인 시스템 정책에 따라 결정해 주세요.
feature/main/src/main/java/com/twix/home/model/DateItemUiModel.kt (1)
3-6: 간결하고 명확한 UI 모델입니다! 👍
dayOfMonth와isSelected만으로 날짜 아이템 표현에 필요한 최소한의 정보를 담고 있어 좋습니다.data class로 정의하여 불변성도 확보되어 있습니다.Compose에서 불필요한 recomposition을 방지하기 위해
@Immutable어노테이션을 추가하는 것도 고려해 볼 수 있습니다. 다만, 이 모델이 리스트 아이템으로만 사용되고equals()비교가 잘 동작한다면 현재 상태로도 충분합니다.선택적 개선안: `@Immutable` 추가
package com.twix.home.model +import androidx.compose.runtime.Immutable + +@Immutable data class DateItemUiModel( val dayOfMonth: Int, val isSelected: Boolean, )core/design-system/src/main/res/drawable/ic_empty_face.xml (1)
1-57: 일러스트레이션 아이콘이 잘 구성되어 있습니다.Empty 상태를 표현하는 귀여운 캐릭터 일러스트레이션입니다. 복잡한 경로들이 잘 조합되어 있네요.
Dark Mode 고려 사항:
현재#ffffff(흰색)와#464646(어두운 회색) 색상이 하드코딩되어 있습니다. Dark Mode에서는 흰색 배경이 너무 밝게 보이거나, 어두운 배경과 캐릭터의 대비가 어색해질 수 있습니다.해결 방안으로는:
res/drawable-night/폴더에 Dark Mode용 별도 아이콘 추가- 또는 색상을
@color/리소스로 분리하여 테마별 색상 적용이 부분은 디자인 가이드에 따라 결정해 주세요. 현재 앱이 Dark Mode를 지원하지 않는다면 나중에 개선해도 됩니다.
core/design-system/src/main/res/drawable/ic_stats_selected.xml (1)
8-14: 다크 모드 대응을 위해 색상 리소스 사용을 권장합니다.현재
#171717색상이 직접 하드코딩되어 있어 다크 모드에서 아이콘이 배경과 구분되지 않을 수 있습니다. 디자인 시스템의 일관성과 다크 모드 지원을 위해res/color에 정의된 색상 리소스를 참조하거나, 사용처에서tint를 동적으로 적용하는 방식을 고려해 주세요.예를 들어,
fillColor와strokeColor에?attr/colorOnSurface같은 테마 속성을 사용하면 테마 변경 시 자동으로 적절한 색상이 적용됩니다.feature/main/src/main/java/com/twix/home/component/HomeTopBar.kt (2)
58-63: contentDescription에 하드코딩된 문자열 대신 stringResource 사용을 권장합니다.현재
"drop down arrow","notification","setting"등의 contentDescription이 영어로 하드코딩되어 있습니다. 접근성(TalkBack) 사용자를 위해 다국어 지원이 필요하며, 리소스 파일에 정의된 문자열을 사용하는 것이 좋습니다.♻️ 개선 제안
Image( painter = painterResource(R.drawable.ic_drop_down_arrow), - contentDescription = "drop down arrow", + contentDescription = stringResource(R.string.content_desc_drop_down), modifier = Modifier.size(20.dp), colorFilter = ColorFilter.tint(GrayColor.C400), )
strings.xml에 다음과 같이 추가:<string name="content_desc_drop_down">월 선택 드롭다운</string> <string name="content_desc_notification">알림</string> <string name="content_desc_setting">설정</string>
105-109: 접근성을 위해 ripple indication 제거 여부를 검토해 주세요.
indication = null로 설정하면 터치 시 시각적 피드백이 사라집니다. 이는 시각 장애가 있는 사용자나 터치 확인이 필요한 사용자에게 불편할 수 있습니다.의도적으로 ripple을 제거한 것이라면 괜찮지만, 접근성 가이드라인에 따르면 인터랙티브 요소는 명확한 피드백을 제공해야 합니다. Material3의 기본 ripple을 유지하거나, 커스텀 indication을 제공하는 것을 고려해 보시겠어요?
♻️ 기본 ripple 유지 방안
Box( modifier = Modifier .size(40.dp) - .clickable( - indication = null, - interactionSource = interactionSource, - onClick = onClick, - ), + .clickable( + interactionSource = interactionSource, + indication = ripple(bounded = false, radius = 20.dp), + onClick = onClick, + ), contentAlignment = Alignment.Center, )feature/main/src/main/java/com/twix/home/component/EmptyGoalGuide.kt (2)
23-59: 재사용 가능한 Composable로 잘 설계되었습니다.
Modifier파라미터를 통해 외부에서 커스터마이징 가능하고, 순수 함수로 작성되어 있습니다. 몇 가지 개선 제안이 있습니다:1. Preview Composable 추가 필요
코딩 가이드라인에 따르면 다양한 상태에 대한 Preview가 제공되어야 합니다. 이 컴포넌트는 단일 상태이므로 기본 Preview만 추가해도 충분합니다.
2. contentDescription 국제화
"empty face","empty goal arrow"가 하드코딩되어 있습니다. 접근성을 위해 문자열 리소스로 추출하는 것이 좋습니다.3. 문자열 리소스 키 오타 확인
R.string.homt_empty_goal_guide에서homt가home의 오타인지 확인해 주세요.♻️ Preview 추가 예시
`@Preview`(showBackground = true) `@Composable` private fun EmptyGoalGuidePreview() { EmptyGoalGuide() }
32-38: 이미지 접근성을 위한 contentDescription 국제화 제안스크린 리더 사용자를 위해
contentDescription을 문자열 리소스로 관리하면 다국어 지원이 가능해집니다. 장식용 이미지라면null로 설정하여 스크린 리더가 건너뛰도록 할 수도 있습니다.♻️ 제안 코드
Image( painter = painterResource(R.drawable.ic_empty_face), - contentDescription = "empty face", + contentDescription = stringResource(R.string.empty_face_content_description), // 또는 장식용이라면: contentDescription = null, ... )Also applies to: 51-57
feature/main/src/main/java/com/twix/main/component/MainBottomBar.kt (1)
29-55: Preview Composable 추가를 고려해 주세요.코딩 가이드라인에 따르면 Preview Composable을 제공하는 것이 권장됩니다. 이를 통해 개발 중 빠른 UI 확인이 가능해집니다.
💡 Preview 추가 예시
`@Preview`(showBackground = true) `@Composable` private fun MainBottomBarPreview() { MainBottomBar( selectedTab = MainTab.HOME, onTabClick = {}, ) }feature/main/src/main/java/com/twix/home/model/HomeUiState.kt (1)
9-17: State 정의가 불변 데이터 클래스로 잘 구현되어 있습니다.
@Immutable어노테이션과data class를 사용하여 MVI 패턴의 State를 올바르게 정의하고 있습니다.몇 가지 확인 사항이 있습니다:
month속성의 용도 확인:monthYear가visibleDate에서 파생되고 있는데,month속성이 별도로 필요한 용도가 있을까요? 현재는 사용되지 않는 것처럼 보입니다.국제화(i18n) 고려:
monthYear의 "월" 문자열이 하드코딩되어 있습니다. 다국어 지원이 필요하다면 리소스 문자열로 분리하는 것을 고려해 주세요.feature/main/src/main/java/com/twix/main/MainScreen.kt (2)
41-62: Scaffold 배경색 설정 방식 개선 제안
Scaffold의modifier에.background(CommonColor.White)를 적용하고, 내부Box에도 동일한 배경색을 적용하고 있습니다.Scaffold는containerColor파라미터를 통해 배경색을 설정하는 것이 더 적절합니다.modifier.background()는Scaffold내부 컨텐츠 영역에 적용되지 않을 수 있습니다.♻️ 개선 제안
Scaffold( - modifier = - Modifier - .fillMaxSize() - .background(CommonColor.White), + modifier = Modifier.fillMaxSize(), + containerColor = CommonColor.White, bottomBar = { MainBottomBar( selectedTab = selectedTab, onTabClick = onTabClick, ) }, ) { padding -> Box( modifier = Modifier .fillMaxSize() - .background(CommonColor.White) .padding(padding), ) { content(selectedTab) } }
35-63: Preview Composable 미제공코딩 가이드라인에 따르면 Compose UI에는 Preview Composable이 제공되어야 합니다.
MainScreen에 대한 Preview 함수를 추가하면 UI 개발 및 검증이 용이해집니다.♻️ Preview 추가 예시
`@Preview`(showBackground = true) `@Composable` private fun MainScreenPreview() { MainScreen( selectedTab = MainTab.HOME, onTabClick = {}, content = { Box(modifier = Modifier.fillMaxSize()) } ) }feature/main/src/main/java/com/twix/home/HomeScreen.kt (3)
45-91: Preview Composable 미제공코딩 가이드라인에 따르면 다양한 상태에 대한 Preview가 제공되어야 합니다.
HomeScreen의 여러 상태(빈 목표, 목표 있음 등)에 대한 Preview를 추가하면 UI 검증이 용이해집니다.♻️ Preview 추가 예시
`@Preview`(showBackground = true) `@Composable` private fun HomeScreenPreview() { HomeScreen( uiState = HomeUiState(), onSelectDate = {}, onPreviousWeek = {}, onNextWeek = {}, onUpdateVisibleDate = {}, ) }
63-67: 빈 클릭 핸들러에 대한 TODO 주석 추가 권장
onNotificationClick과onSettingClick이 빈 람다로 전달되고 있습니다. 향후 구현 예정임을 명시하는 TODO 주석을 추가하면 코드 추적이 용이해집니다.♻️ 개선 제안
HomeTopBar( monthYearText = uiState.monthYear, - onNotificationClick = {}, - onSettingClick = {}, + onNotificationClick = { /* TODO: 알림 화면 연결 */ }, + onSettingClick = { /* TODO: 설정 화면 연결 */ }, )
110-116: contentDescription 국제화(i18n) 고려
contentDescription이 하드코딩된 영어 문자열("add goal")로 되어 있습니다. 접근성을 위해stringResource를 사용하여 국제화를 적용하는 것이 좋습니다.♻️ 개선 제안
Image( painter = painterResource(R.drawable.ic_plus), - contentDescription = "add goal", + contentDescription = stringResource(R.string.content_desc_add_goal), modifier = Modifier .size(40.dp), )feature/main/src/main/java/com/twix/home/component/WeeklyCalendar.kt (3)
125-144: 클릭 영역이 텍스트에만 제한됨현재
noRippleClickable이AppText에 적용되어 있어 클릭 가능 영역이 날짜 텍스트 부분으로 제한됩니다. 사용자 경험을 위해 전체 44dp 셀 영역이 클릭 가능하도록Box에 클릭 핸들러를 적용하는 것이 좋습니다.♻️ 개선 제안
Box( modifier = Modifier .size(44.dp) + .noRippleClickable { onClick() } .then( if (selected) Modifier.border(1.dp, GrayColor.C500, CircleShape) else Modifier, ), contentAlignment = Alignment.Center, ) { AppText( text = dayOfMonth.toString(), style = AppTextStyle.B1, color = GrayColor.C400, - modifier = - Modifier - .fillMaxSize() - .wrapContentSize(Alignment.Center) - .noRippleClickable { onClick() }, ) }
43-43:today값이 자정 이후에도 갱신되지 않음
remember { LocalDate.now() }는 키 없이 사용되어 앱이 자정을 넘겨 실행 중일 경우 "오늘" 라벨이 갱신되지 않습니다. 일반적인 사용 시나리오에서는 큰 문제가 아니지만, 장시간 앱 사용 시 고려해볼 사항입니다.
34-101: Preview Composable 미제공재사용 가능한 UI 컴포넌트인
WeeklyCalendar에 Preview가 없습니다. 다양한 상태(선택된 날짜, 오늘 날짜 포함 여부 등)에 대한 Preview를 추가하면 컴포넌트 검증이 용이해집니다.core/ui/src/main/java/com/twix/ui/base/BaseViewModel.kt (2)
104-109: 에러 로깅 상세도 검토 제안
handleError에서 전체 스택 트레이스(stackTraceToString())를 로깅하고 있습니다. 개발 단계에서는 유용하지만, 프로덕션 환경에서는 로그 볼륨이 과도해질 수 있습니다. 환경에 따라 로깅 상세도를 조절하거나, Crash Reporting 도구(예: Firebase Crashlytics)로 전달하는 방식을 고려해보시는 것은 어떨까요?
30-30: Channel 용량에 대한 고려
Channel.BUFFERED는 기본적으로 64개의 버퍼를 가집니다. 대부분의 UI 인터랙션에는 충분하지만, 빠른 연속 입력 시나리오(예: 빠른 버튼 연타)에서 버퍼 오버플로우가 발생할 수 있습니다. 현재trySend실패 시 로깅을 하고 있어 추적은 가능하지만, 필요에 따라Channel.UNLIMITED나 커스텀 용량을 고려해볼 수 있습니다.
| <path | ||
| android:pathData="M15,11H9L12.2,14L15,11Z" | ||
| android:strokeLineJoin="round" | ||
| android:strokeWidth="0.857143" | ||
| android:fillColor="#464646" | ||
| android:strokeColor="#464646" |
There was a problem hiding this comment.
다크 모드/테마 대응을 위해 색상 하드코딩을 피하는 게 좋아요.
왜 문제인가요: #464646 고정 색상은 다크 모드에서 대비가 깨질 수 있어 디자인 시스템 일관성과 접근성을 해칩니다.
어떻게 개선하나요: 테마 속성으로 바꾸거나, 아이콘을 무채색으로 두고 사용처에서 tint를 적용하는 방식이 더 안전합니다. 아래처럼 테마 색상으로 치환하는 건 어떨까요?
♻️ 테마 속성 사용 예시
- android:fillColor="#464646"
- android:strokeColor="#464646"
+ android:fillColor="?attr/colorOnSurface"
+ android:strokeColor="?attr/colorOnSurface"질문: 이 아이콘은 특정 색을 고정해야 하는 의도가 있었나요, 아니면 화면 테마에 따라 변화하도록 의도하셨나요?
📝 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.
| <path | |
| android:pathData="M15,11H9L12.2,14L15,11Z" | |
| android:strokeLineJoin="round" | |
| android:strokeWidth="0.857143" | |
| android:fillColor="#464646" | |
| android:strokeColor="#464646" | |
| <path | |
| android:pathData="M15,11H9L12.2,14L15,11Z" | |
| android:strokeLineJoin="round" | |
| android:strokeWidth="0.857143" | |
| android:fillColor="?attr/colorOnSurface" | |
| android:strokeColor="?attr/colorOnSurface" |
🤖 Prompt for AI Agents
In `@core/design-system/src/main/res/drawable/ic_drop_down_arrow.xml` around lines
6 - 11, The path element in ic_drop_down_arrow.xml hardcodes fillColor and
strokeColor to `#464646` which breaks dark-theme contrast; update the drawable to
avoid fixed colors by replacing android:fillColor and android:strokeColor with a
theme attribute (e.g., ?attr/colorOnBackground or a dedicated ?attr/colorIcon)
or remove those attributes so the view can apply a tint at runtime; locate the
<path ... android:fillColor="#464646" android:strokeColor="#464646" ... /> and
swap to the theme attribute or null it, and ensure consuming views call
setImageTintList or use android:tint to apply the correct themed color.
| <path | ||
| android:strokeWidth="1" | ||
| android:pathData="M2,19.714V9.429C2,9.067 2.089,8.724 2.266,8.4C2.443,8.076 2.688,7.81 3,7.6L10.5,2.457C10.938,2.152 11.438,2 12,2C12.563,2 13.063,2.152 13.5,2.457L21,7.6C21.313,7.81 21.557,8.076 21.734,8.4C21.912,8.724 22,9.067 22,9.429V19.714C22,20.343 21.755,20.881 21.266,21.329C20.776,21.776 20.188,22 19.5,22H15.75C15.396,22 15.099,21.89 14.859,21.671C14.62,21.452 14.5,21.181 14.5,20.857V15.143C14.5,14.819 14.38,14.548 14.141,14.329C13.901,14.109 13.604,14 13.25,14H10.75C10.396,14 10.099,14.109 9.859,14.329C9.62,14.548 9.5,14.819 9.5,15.143V20.857C9.5,21.181 9.38,21.452 9.141,21.671C8.901,21.89 8.604,22 8.25,22H4.5C3.813,22 3.224,21.776 2.734,21.329C2.245,20.881 2,20.343 2,19.714Z" | ||
| android:fillColor="#00000000" | ||
| android:strokeColor="#171717"/> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the file existence and content
echo "=== Checking ic_home_unselected.xml ==="
cat -n core/design-system/src/main/res/drawable/ic_home_unselected.xml 2>/dev/null || echo "File not found"
# Check for colors.xml files
echo -e "\n=== Checking for colors.xml files ==="
fd -t f "colors.xml" core/design-system/
# Check for night theme resources
echo -e "\n=== Checking for night theme resources ==="
fd -t d "values-night" core/design-system/
# Check other drawable icons for color patterns
echo -e "\n=== Checking other drawable icons for color patterns ==="
fd -t f "ic_.*\.xml" core/design-system/src/main/res/drawable/ | head -10 | xargs -I {} bash -c 'echo "File: {}"; grep -E "(fillColor|strokeColor)" {} | head -1; echo'
# Check for theme attribute usage in resources
echo -e "\n=== Checking for theme attribute usage ==="
rg -n "\?attr" core/design-system/src/main/res/ --type xml | head -20Repository: YAPP-Github/Twix-Android
Length of output: 2403
디자인 시스템의 아이콘 색상을 테마 기반으로 관리해야 합니다.
현재 strokeColor="#171717"이 하드코딩되어 있어 다크 모드 지원에 장애물이 됩니다. 다른 아이콘들도 같은 방식으로 색상을 관리하고 있는데, 이는 다크 모드 도입 시 모든 drawable을 수정해야 하는 상황을 초래합니다.
왜 개선이 필요한가:
- Material Design 3 가이드라인은 색상을 테마 속성으로 정의하여 다크/라이트 모드를 자동으로 지원하도록 권장합니다.
- 현재 모든 아이콘이 하드코딩된 색상을 사용하면, 향후 다크 모드 지원 시 drawable 파일을 개별 수정해야 합니다.
개선 방안:
core/design-system/src/main/res/values/colors.xml생성하여 색상 정의<color name="icon_default">#171717</color>
- drawable에서
@color/icon_default참조 사용 - 향후 다크 모드 지원 시
values-night/colors.xml추가하여 동일한 색상명으로 다른 값 정의
이렇게 구조화하면 다크 모드 추가 시 colors.xml만 수정하면 모든 아이콘이 자동으로 테마를 따르게 됩니다.
🤖 Prompt for AI Agents
In `@core/design-system/src/main/res/drawable/ic_home_unselected.xml` around lines
6 - 10, The path in ic_home_unselected.xml hardcodes strokeColor="#171717" which
breaks theme/dark-mode support; replace the hardcoded android:strokeColor on the
<path> with a color resource reference (e.g., `@color/icon_default`) and add a
colors.xml entry named icon_default in
core/design-system/src/main/res/values/colors.xml (and a corresponding
values-night/colors.xml for dark mode) so icons follow theme colors instead of
fixed hex values.
|
|
||
| <!-- 홈 화면 --> | ||
| <string name="home_today_goal">오늘 우리 목표</string> | ||
| <string name="homt_empty_goal_guide">첫 목표를 세워볼까요?</string> |
There was a problem hiding this comment.
오타 수정이 필요합니다: homt → home
String key 이름에 오타가 있습니다. homt_empty_goal_guide를 home_empty_goal_guide로 수정해주세요. 이 키를 참조하는 코드(EmptyGoalGuide.kt)도 함께 수정이 필요합니다.
🐛 수정 제안
- <string name="homt_empty_goal_guide">첫 목표를 세워볼까요?</string>
+ <string name="home_empty_goal_guide">첫 목표를 세워볼까요?</string>📝 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.
| <string name="homt_empty_goal_guide">첫 목표를 세워볼까요?</string> | |
| <string name="home_empty_goal_guide">첫 목표를 세워볼까요?</string> |
🤖 Prompt for AI Agents
In `@core/design-system/src/main/res/values/strings.xml` at line 19, Rename the
malformed string resource key homt_empty_goal_guide to home_empty_goal_guide in
strings.xml and update all usages to match (for example the reference in
EmptyGoalGuide.kt), ensuring the string value remains unchanged and any
import/lookup (e.g., R.string.homt_empty_goal_guide) is replaced with
R.string.home_empty_goal_guide so resource lookups compile correctly.
| LaunchedEffect(days.first()) { | ||
| if (dragSumPx > 0) { | ||
| onUpdateVisibleDate(days.first()) | ||
| } else { | ||
| onUpdateVisibleDate(days.last()) | ||
| } | ||
| } |
There was a problem hiding this comment.
LaunchedEffect 초기 실행 시 의도치 않은 동작 가능성
LaunchedEffect(days.first())는 첫 composition 시에도 실행됩니다. 이때 dragSumPx는 초기값 0f이므로 else 분기를 타서 onUpdateVisibleDate(days.last())가 호출됩니다. 이는 스와이프 없이도 visibleDate가 주의 마지막 날로 설정되는 의도치 않은 동작을 유발할 수 있습니다.
스와이프 후에만 onUpdateVisibleDate가 호출되도록 조건을 추가하거나, 초기 상태를 별도로 처리하는 것이 좋습니다.
🐛 개선 제안
+ var hasSwiped by remember { mutableStateOf(false) }
var dragSumPx by remember { mutableFloatStateOf(0f) }
LaunchedEffect(days.first()) {
+ if (!hasSwiped) return@LaunchedEffect
if (dragSumPx > 0) {
onUpdateVisibleDate(days.first())
} else {
onUpdateVisibleDate(days.last())
}
}
Column(
modifier =
Modifier
.fillMaxWidth()
.pointerInput(Unit) {
detectHorizontalDragGestures(
onDragStart = { dragSumPx = 0f },
onHorizontalDrag = { _, dragAmount ->
dragSumPx += dragAmount
},
onDragEnd = {
if (abs(dragSumPx) < 120f) return@detectHorizontalDragGestures
+ hasSwiped = true
if (dragSumPx > 0f) onPreviousWeek() else onNextWeek()
},
)
}🤖 Prompt for AI Agents
In `@feature/main/src/main/java/com/twix/home/component/WeeklyCalendar.kt` around
lines 60 - 66, LaunchedEffect(days.first()) runs on first composition and with
dragSumPx initial 0f will call onUpdateVisibleDate(days.last()); guard the
effect so it only triggers after a user swipe by adding a check (e.g., require
dragSumPx != 0f or track a hasSwiped/initialized boolean remembered and set only
when a drag completes) before calling onUpdateVisibleDate; update the
LaunchedEffect containing LaunchedEffect(days.first()), dragSumPx, and
onUpdateVisibleDate to return early when no user swipe has occurred so initial
composition does not change visibleDate.
| Column( | ||
| modifier = modifier, | ||
| verticalArrangement = Arrangement.spacedBy(4.dp), | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| ) { | ||
| Image( | ||
| painter = painterResource(icon), | ||
| contentDescription = null, | ||
| modifier = | ||
| Modifier | ||
| .size(24.dp) | ||
| .clickable( | ||
| indication = null, | ||
| interactionSource = interactionSource, | ||
| onClick = { | ||
| haptic.performHapticFeedback(HapticFeedbackType.TextHandleMove) | ||
| onClick() | ||
| }, | ||
| ), | ||
| ) | ||
|
|
||
| AppText( | ||
| text = stringResource(tab.title), | ||
| color = GrayColor.C500, | ||
| style = AppTextStyle.C2, | ||
| ) | ||
| } |
There was a problem hiding this comment.
클릭 영역이 아이콘에만 제한되어 있습니다.
현재 clickable modifier가 Image(24dp)에만 적용되어 있어, 사용자가 탭 라벨을 탭해도 반응하지 않습니다. 전체 탭 영역을 클릭 가능하게 만드는 것이 UX 관점에서 더 좋습니다.
🛠️ 클릭 영역을 Column 전체로 확장하는 제안
`@Composable`
private fun MainBottomBarItem(
modifier: Modifier = Modifier,
tab: MainTab,
selected: Boolean,
onClick: () -> Unit,
) {
val haptic = LocalHapticFeedback.current
val interactionSource = remember { MutableInteractionSource() }
val icon = if (selected) tab.selectedIcon else tab.unselectedIcon
Column(
- modifier = modifier,
+ modifier = modifier
+ .clickable(
+ indication = null,
+ interactionSource = interactionSource,
+ onClick = {
+ haptic.performHapticFeedback(HapticFeedbackType.TextHandleMove)
+ onClick()
+ },
+ ),
verticalArrangement = Arrangement.spacedBy(4.dp),
horizontalAlignment = Alignment.CenterHorizontally,
) {
Image(
painter = painterResource(icon),
contentDescription = null,
- modifier =
- Modifier
- .size(24.dp)
- .clickable(
- indication = null,
- interactionSource = interactionSource,
- onClick = {
- haptic.performHapticFeedback(HapticFeedbackType.TextHandleMove)
- onClick()
- },
- ),
+ modifier = Modifier.size(24.dp),
)
AppText(
text = stringResource(tab.title),
color = GrayColor.C500,
style = AppTextStyle.C2,
)
}
}🤖 Prompt for AI Agents
In `@feature/main/src/main/java/com/twix/main/component/MainBottomBar.kt` around
lines 68 - 94, The clickable area is currently applied only to the Image, so the
tab label (AppText) is not interactive; move the clickable modifier from the
Image to the parent Column (apply modifier.clickable(...) with the same
indication, interactionSource, and onClick that performs
haptic.performHapticFeedback(HapticFeedbackType.TextHandleMove) then calls
onClick()), and remove the clickable modifier from the Image so the entire
Column (icon + label) is tappable while preserving the original
interactionSource, haptic behavior, and onClick handler.
chanho0908
left a comment
chanho0908
left a comment
There was a problem hiding this comment.
고생했어 현수아
궁금한게 몇 가지 있어서 리뷰 남겼으니 확인해줘 ! 🥊
| .clickable( | ||
| indication = null, | ||
| onClick = onClick, | ||
| interactionSource = remember { MutableInteractionSource() }, |
There was a problem hiding this comment.
리플 효과를 제거하려는 의도였다면 현수가 만든 noRippleClickable을 사용하는건 어떨까 ?
| } | ||
|
|
||
| private fun shiftWeek(offset: Long) { | ||
| val newReference = currentState.referenceDate.plusDays(offset * 7) |
There was a problem hiding this comment.
- 7이라는 매직 넘버의 의미를 좀 더 명확히 표현할 수 있도록 상수화 하는건 어떨까 ?
| HomeIntent.NextWeek -> shiftWeek(1) | ||
| HomeIntent.PreviousWeek -> shiftWeek(-1) |
There was a problem hiding this comment.
- 1과 -1이라는 숫자가 "다음주로 이동한다" "지난주로 이동한다" 처럼 명확한
의미를 전달할 수 있도록 만들면 코드를 조금 더 읽기 쉬워질거 같다고 생각하는데 어떻게 생각해 ? 🙂
| .clickable( | ||
| indication = null, | ||
| interactionSource = interactionSource, | ||
| onClick = onClick, | ||
| ), |
There was a problem hiding this comment.
noRippleClickable을 사용할 수 있을 것 같아 !
| init { | ||
| // Intent 순차 처리 | ||
| viewModelScope.launch { | ||
| intentChannel.consumeAsFlow().collect { intent -> |
There was a problem hiding this comment.
channel에 consumeAsFlow와 receiveAsFlow가 있는걸로 아는데
여기선 어떤 기준으로 consumeAsFlow를 사용했는지 궁금해 !
| * UI에서 Intent를 발생시키는 메서드 | ||
| * */ | ||
| fun dispatch(intent: I) { | ||
| val result = intentChannel.trySend(intent) |
There was a problem hiding this comment.
요기두 send와 trySend가 있는걸로 아는데 어떤 기준으로 선택했을까 ?
| private fun TopBarButton( | ||
| icon: Painter, | ||
| contentDescription: String, | ||
| onClick: () -> Unit, | ||
| ) { |
There was a problem hiding this comment.
공식 문서에 따르면 Painter는 stable 하지 않다고 안내하고 있는 것으로 알고있는데,
imageVector를 사용하는건 어떨까 ?
이슈 번호
작업내용
리뷰어에게 추가로 요구하는 사항 (선택)
MainScreen이 컨테이너 역할로 GNB 탭을 관리하고 화면을 분기처리하고 있습니다. 이후에 MainScreen에 토스트 메시지 렌더링 로직과 바텀시트 렌더링을 추가할 예정이라 전체적인 구조를 봐주시면 좋을 것 같아요!
HomeScreen에서는 아직 목표에 대한 DTO가 정의되지 않아서 목표가 비어있는 것을 전제로, 달력 전환과 상단 날짜가 변경되는 로직만 구현해 뒀습니다.