[FIX#133] 1차 큐에이 수정사항 반영 - #139
Merged
Merged
Conversation
Closed
Walkthrough여러 화면에서 Loading 상태를 렌더링하도록 추가하고, 사이드 이펙트 타입들을 상태 파일로 이동·재정의하며, 절차 관련 컴포넌트의 다운타임 날짜 표현을 문자열에서 월/일 정수로 변경하고, 게이지 컴포넌트에서 step == 0(LEVEL0)을 필터링·지원하도록 변경했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 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.github.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 `@app/src/main/java/com/cherrish/android/presentation/main/MainAppState.kt`:
- Around line 111-116: The navOptions builder for refreshNavOptions is wrong:
change the property access launchSingleTop to an assignment (launchSingleTop =
true) and, instead of duplicating behavior, reuse the existing
clearStackNavOptions defined at class level; update references to use
clearStackNavOptions or initialize refreshNavOptions by delegating to
clearStackNavOptions to remove duplication while ensuring launchSingleTop is set
to true.
In
`@app/src/main/java/com/cherrish/android/presentation/onboarding/OnboardingScreen.kt`:
- Around line 223-238: The Text composable is forcing a custom fontSize by
dividing by fontScale (see the Text using
CherrishTheme.typography.title2M16.copy(fontSize = with(density) {
CherrishTheme.typography.title2M16.fontSize / fontScale })) which disables
system font scaling and breaks accessibility; remove the manual division and use
the theme typography directly (e.g., style = CherrishTheme.typography.title2M16)
or, if a fixed scale is absolutely required, wrap the composable in a
CompositionLocalProvider(LocalDensity or LocalFontScale override) with clear
justification. Also find and remove similar patterns (e.g., the fixedFontSize
variable around the other occurrence) so system fontScale is honored throughout.
🧹 Nitpick comments (3)
app/src/main/java/com/cherrish/android/presentation/home/component/UpcomingPlanSection.kt (1)
82-85: LGTM! 디자인 QA 반영 패딩 조정하단 여백이 5.dp에서 16.dp로 증가하여 "다가오는 일정" 텍스트와 HorizontalDivider 사이의 간격이 넓어졌습니다. 디자인 의도에 맞는 변경으로 보입니다.
♻️ (선택사항) 패딩 호출 통합
두 개의 연속
.padding()호출을 하나로 통합할 수 있습니다:modifier = Modifier - .padding(horizontal = 4.dp) - .padding(bottom = 16.dp) + .padding(start = 4.dp, end = 4.dp, bottom = 16.dp)app/src/main/java/com/cherrish/android/core/designsystem/component/gaugebar/CherrishGaugeBar.kt (1)
37-44: LGTM - LEVEL0 필터링 로직이 적절합니다.
step == 0인 게이지를 필터링하고coerceAtLeast(1)로 edge case를 처리하는 방식이 좋습니다.한 가지 선택적 개선사항:
reversedGauges의 remember key를displayGauges대신gauges로 통일하면 의존성 체인이 더 명확해집니다.displayGauges가 이미gauges를 key로 remember되어 있어 현재 코드도 정상 동작하지만, 동일한 key를 사용하면 가독성이 향상됩니다.♻️ 선택적 개선안
- val displayGauges = remember(gauges) { gauges.filter { it.step != 0 } } - val reversedGauges = remember(displayGauges) { displayGauges.asReversed() } + val displayGauges = remember(gauges) { gauges.filter { it.step != 0 } } + val reversedGauges = remember(gauges) { displayGauges.asReversed() }app/src/main/java/com/cherrish/android/presentation/calendar/procedure/component/DowntimeBottomSheet.kt (1)
3-3: DowntimeGuideBubble의 modifier 파라미터가 적용되지 않습니다.
외부에서 전달한 modifier가 무시되어 확장성이 떨어집니다.♻️ 제안 수정
- Box() { + Box(modifier = modifier) {Also applies to: 242-263
Comment on lines
+111
to
+116
| val refreshNavOptions = navOptions { | ||
| popUpTo(0) { | ||
| inclusive = true | ||
| } | ||
| launchSingleTop | ||
| } |
There was a problem hiding this comment.
launchSingleTop 할당 누락 버그
Line 115에서 launchSingleTop은 property access일 뿐, 값을 true로 설정하지 않습니다. launchSingleTop = true로 수정해야 합니다.
또한, 이 refreshNavOptions는 클래스 레벨에 정의된 clearStackNavOptions (lines 41-45)와 거의 동일합니다. 중복 코드를 제거하고 기존 property를 재사용하는 것을 권장합니다.
🐛 제안된 수정
옵션 1: 버그 수정만
val refreshNavOptions = navOptions {
popUpTo(0) {
inclusive = true
}
- launchSingleTop
+ launchSingleTop = true
}옵션 2: 기존 property 재사용 (권장)
- val refreshNavOptions = navOptions {
- popUpTo(0) {
- inclusive = true
- }
- launchSingleTop
- }
-
when (tab) {
- MainTab.HOME -> navController.navigateToHome(navOptions = refreshNavOptions)
+ MainTab.HOME -> navController.navigateToHome(navOptions = clearStackNavOptions)
MainTab.CALENDAR -> navController.navigateToCalendar(navOptions = navOptions)
MainTab.MYPAGE -> navController.navigateToMyPage(navOptions = navOptions)
MainTab.CHALLENGE -> {}
}📝 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
| val refreshNavOptions = navOptions { | |
| popUpTo(0) { | |
| inclusive = true | |
| } | |
| launchSingleTop | |
| } | |
| val refreshNavOptions = navOptions { | |
| popUpTo(0) { | |
| inclusive = true | |
| } | |
| launchSingleTop = true | |
| } |
🤖 Prompt for AI Agents
In `@app/src/main/java/com/cherrish/android/presentation/main/MainAppState.kt`
around lines 111 - 116, The navOptions builder for refreshNavOptions is wrong:
change the property access launchSingleTop to an assignment (launchSingleTop =
true) and, instead of duplicating behavior, reuse the existing
clearStackNavOptions defined at class level; update references to use
clearStackNavOptions or initialize refreshNavOptions by delegating to
clearStackNavOptions to remove duplication while ensuring launchSingleTop is set
to true.
Merged
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
2 participants
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 ✏️
Screenshot 📸
Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit
New Features
UI
Chores
✏️ Tip: You can customize this high-level summary in your review settings.