Open
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
개요상단바 아이콘 색상 관리 로직을 홈 화면 한정에서 모든 화면으로 확장하기 위해 중앙집중식 변경사항
시퀀스 다이어그램sequenceDiagram
participant NavController
participant NavStatusBarEffect
participant LifecycleOwner
participant StatusBarAppearanceManager
participant Window
NavController->>NavStatusBarEffect: 현재 NavBackStackEntry 제공
NavStatusBarEffect->>LifecycleOwner: repeatOnLifecycle(RESUMED) 설정
LifecycleOwner->>NavStatusBarEffect: RESUMED 상태 진입
NavStatusBarEffect->>Window: Activity의 window 접근
NavStatusBarEffect->>StatusBarAppearanceManager: applyStatusBarColorByLuminance(window) 호출
StatusBarAppearanceManager->>Window: 상태바 영역 비트맵 캡처 (PixelCopy)
StatusBarAppearanceManager->>StatusBarAppearanceManager: 중앙 픽셀 광도 계산
StatusBarAppearanceManager->>Window: WindowInsetsControllerCompat를 통해 아이콘 색상 업데이트
코드 리뷰 예상 난이도🎯 3 (Moderate) | ⏱️ ~22 분 래빗의 축하 시
🚥 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🧪 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: 3
🧹 Nitpick comments (4)
presentation/src/main/java/com/threegap/bitnagil/presentation/util/statusbar/StatusBarAppearanceManager.kt (2)
16-17: 상태가 없는 유틸리티이므로object로 선언 권장.이 클래스는 인스턴스 상태를 보관하지 않고 모든 메서드가 순수 함수처럼 동작합니다.
class대신object또는 최상위 함수로 노출하면 호출부의 인스턴스 생성을 줄이고 의도를 더 명확히 표현할 수 있습니다.-class StatusBarAppearanceManager { +object StatusBarAppearanceManager {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/com/threegap/bitnagil/presentation/util/statusbar/StatusBarAppearanceManager.kt` around lines 16 - 17, StatusBarAppearanceManager is a stateless utility class; change its declaration from a class to an object (or convert applyStatusBarColorByLuminance into a top-level function) so callers don't need to instantiate it; update any call sites that do StatusBarAppearanceManager() to reference the singleton (StatusBarAppearanceManager.applyStatusBarColorByLuminance) or the new top-level function name, and ensure visibility (public) and imports are adjusted accordingly.
48-53: 중심 픽셀 1개만으로 휘도를 계산하는 정확도 한계.
calculateLuminance가 비트맵 정중앙의 단일 픽셀만 샘플링합니다. 상단바 영역에 그라디언트, 좌측 상태 아이콘 영역과 우측 노치/캐리어 영역이 다른 색상을 가질 경우 중앙 픽셀이 전체 영역을 대표하지 못해 아이콘 가독성이 저하될 수 있습니다. 또한150이라는 임계값이 0.299·R + 0.587·G + 0.114·B 기준이라면 실제 BT.601의 일반 임계값과 비교해 명시적인 근거가 부족합니다.여러 지점(예: 좌/중/우)의 평균 또는 최소 휘도를 사용하는 것을 권장합니다.
♻️ 제안 예시
- private fun calculateLuminance(bitmap: Bitmap): Int { - val x = bitmap.width / 2 - val y = bitmap.height / 2 - val px = bitmap[x, y] - return (0.299 * Color.red(px) + 0.587 * Color.green(px) + 0.114 * Color.blue(px)).toInt() - } + private fun calculateLuminance(bitmap: Bitmap): Int { + val y = bitmap.height / 2 + val xs = intArrayOf(bitmap.width / 4, bitmap.width / 2, bitmap.width * 3 / 4) + val avg = xs.map { x -> + val px = bitmap[x, y] + 0.299 * Color.red(px) + 0.587 * Color.green(px) + 0.114 * Color.blue(px) + }.average() + return avg.toInt() + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/com/threegap/bitnagil/presentation/util/statusbar/StatusBarAppearanceManager.kt` around lines 48 - 53, calculateLuminance currently samples only the single center pixel (bitmap[x, y]) which is unreliable for varied status bar regions; update calculateLuminance to sample multiple points (e.g., left/center/right or a small grid across the status bar area) and compute either the average or the minimum luminance using the same BT.601 formula (0.299*R + 0.587*G + 0.114*B) to better represent overall brightness, replace the single-pixel read with that multisample result, and extract the hard-coded threshold (150) into a named constant (e.g., STATUS_BAR_LUMINANCE_THRESHOLD) with a comment referencing the BT.601 basis so it’s easy to tune or document.presentation/src/main/java/com/threegap/bitnagil/presentation/util/statusbar/NavStatusBarEffect.kt (1)
19-21:StatusBarAppearanceManager인스턴스화 비용.
StatusBarAppearanceManager는 상태가 없는 유틸리티 클래스입니다. navigation 변경마다 새 인스턴스를 생성하기보다object선언으로 변경하거나, 호출부에서 한 번만 만들어 재사용하는 편이 적절합니다.♻️ 제안
StatusBarAppearanceManager를object로 선언하면 호출부도 다음과 같이 단순해집니다.- StatusBarAppearanceManager().applyStatusBarColorByLuminance(window) + StatusBarAppearanceManager.applyStatusBarColorByLuminance(window)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/com/threegap/bitnagil/presentation/util/statusbar/NavStatusBarEffect.kt` around lines 19 - 21, StatusBarAppearanceManager is a stateless utility so avoid re-instantiating it on every navigation change; either convert the class to a Kotlin object (StatusBarAppearanceManager) so you can call StatusBarAppearanceManager.applyStatusBarColorByLuminance(window) directly, or create a single reusable instance (e.g., a private val statusBarAppearanceManager = StatusBarAppearanceManager()) in NavStatusBarEffect and call statusBarAppearanceManager.applyStatusBarColorByLuminance(window) instead of instantiating it each time; update references to applyStatusBarColorByLuminance accordingly.app/src/main/java/com/threegap/bitnagil/navigation/home/HomeNavHost.kt (1)
55-55: 홈 탭 간 전환 시 effect 위치 정리 필요.내부
navigator.navController로NavStatusBarEffect를 호출하는 의도는 홈 하위 탭(Home/RecommendRoutine/MyPage) 전환에 따라 상단바 색상을 갱신하기 위한 것으로 보입니다. 다만MainNavHost.ktLine 37의 외부NavStatusBarEffect와 함께 동작하므로Route.Home으로 진입할 때 두 effect가 동시에 발화합니다. 의도한 동작이라면 상관없지만, 외부 effect만으로는 홈 내부 탭 전환을 감지하지 못하므로 두 effect 모두 유지하는 것이 정상이라는 점을 코드 주석으로 남겨두면 추후 유지보수에 도움이 될 것 같습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/threegap/bitnagil/navigation/home/HomeNavHost.kt` at line 55, The NavStatusBarEffect call using navigator.navController in HomeNavHost is causing duplicate effects together with the outer NavStatusBarEffect in MainNavHost when entering Route.Home; add a concise code comment immediately above the NavStatusBarEffect(navController = navigator.navController) line explaining that this inner effect is intentionally present to react to home sub-tab (Home/RecommendRoutine/MyPage) navigation (which the outer effect alone cannot observe), and note that both effects will fire on Route.Home entry but must be kept to handle internal tab changes; reference NavStatusBarEffect, navigator.navController, MainNavHost, and Route.Home in the comment for future maintainers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/threegap/bitnagil/MainNavHost.kt`:
- Line 37: MainNavHost와 HomeNavHost에서 동일한 NavStatusBarEffect를 두 번 호출해 PixelCopy
캡처가 중복 실행되는 문제입니다; 해결하려면 중복 호출을 제거하거나 조건부로 실행되게 변경하세요: 보통은 MainNavHost의
NavStatusBarEffect(navController = navigator.navController)를 유지하고 HomeNavHost 쪽의
NavStatusBarEffect 호출을 삭제하거나, HomeNavHost에서
rememberHomeNavigator().navController가 부모의 navController와 동일하거나 현재 목적지가
Route.Home이 아닐 때만 효과를 실행하도록 가드(예: currentBackStackEntry 비교 또는 navController 참조
비교)하세요 so only one PixelCopy capture runs per navigation change.
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/util/statusbar/StatusBarAppearanceManager.kt`:
- Around line 21-29: The bitmap created in captureStatusBarBitmap is not being
recycled after use, causing memory pressure; update the lambda passed to
captureStatusBarBitmap (the callback that receives bmp) to ensure the Bitmap is
recycled after luminance is computed — e.g., wrap use of bmp in a try/finally
(or call bmp.recycle() at the end of the callback) so calculateLuminance(it) and
WindowInsetsControllerCompat(window,
window.decorView).isAppearanceLightStatusBars = (lum > 150) run before
bmp.recycle(), and avoid accessing the bitmap after recycling.
- Around line 38-46: In captureStatusBarBitmap, guard against callbacks running
after the Activity/Window is gone and handle PixelCopy result codes: before
calling PixelCopy.request ensure window is still valid (check
window.decorView.isAttachedToWindow and, if available, the Activity's
isFinishing/isDestroyed), and in the PixelCopy.request callback check the
copyResult == PixelCopy.SUCCESS; if success call onResult(bmp), otherwise
recycle the bmp (or let it be GC'd by explicitly bmp.recycle()) and call
onResult(null). Also avoid invoking WindowInsetsControllerCompat or other window
APIs unless the window/decorView is still attached.
---
Nitpick comments:
In `@app/src/main/java/com/threegap/bitnagil/navigation/home/HomeNavHost.kt`:
- Line 55: The NavStatusBarEffect call using navigator.navController in
HomeNavHost is causing duplicate effects together with the outer
NavStatusBarEffect in MainNavHost when entering Route.Home; add a concise code
comment immediately above the NavStatusBarEffect(navController =
navigator.navController) line explaining that this inner effect is intentionally
present to react to home sub-tab (Home/RecommendRoutine/MyPage) navigation
(which the outer effect alone cannot observe), and note that both effects will
fire on Route.Home entry but must be kept to handle internal tab changes;
reference NavStatusBarEffect, navigator.navController, MainNavHost, and
Route.Home in the comment for future maintainers.
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/util/statusbar/NavStatusBarEffect.kt`:
- Around line 19-21: StatusBarAppearanceManager is a stateless utility so avoid
re-instantiating it on every navigation change; either convert the class to a
Kotlin object (StatusBarAppearanceManager) so you can call
StatusBarAppearanceManager.applyStatusBarColorByLuminance(window) directly, or
create a single reusable instance (e.g., a private val
statusBarAppearanceManager = StatusBarAppearanceManager()) in NavStatusBarEffect
and call statusBarAppearanceManager.applyStatusBarColorByLuminance(window)
instead of instantiating it each time; update references to
applyStatusBarColorByLuminance accordingly.
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/util/statusbar/StatusBarAppearanceManager.kt`:
- Around line 16-17: StatusBarAppearanceManager is a stateless utility class;
change its declaration from a class to an object (or convert
applyStatusBarColorByLuminance into a top-level function) so callers don't need
to instantiate it; update any call sites that do StatusBarAppearanceManager() to
reference the singleton
(StatusBarAppearanceManager.applyStatusBarColorByLuminance) or the new top-level
function name, and ensure visibility (public) and imports are adjusted
accordingly.
- Around line 48-53: calculateLuminance currently samples only the single center
pixel (bitmap[x, y]) which is unreliable for varied status bar regions; update
calculateLuminance to sample multiple points (e.g., left/center/right or a small
grid across the status bar area) and compute either the average or the minimum
luminance using the same BT.601 formula (0.299*R + 0.587*G + 0.114*B) to better
represent overall brightness, replace the single-pixel read with that
multisample result, and extract the hard-coded threshold (150) into a named
constant (e.g., STATUS_BAR_LUMINANCE_THRESHOLD) with a comment referencing the
BT.601 basis so it’s easy to tune or document.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e8efa56-9a11-46ff-aea1-8a0b6425e945
📒 Files selected for processing (5)
app/src/main/java/com/threegap/bitnagil/MainNavHost.ktapp/src/main/java/com/threegap/bitnagil/navigation/home/HomeNavHost.ktapp/src/main/java/com/threegap/bitnagil/navigation/home/HomeNavigator.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/util/statusbar/NavStatusBarEffect.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/util/statusbar/StatusBarAppearanceManager.kt
💤 Files with no reviewable changes (1)
- app/src/main/java/com/threegap/bitnagil/navigation/home/HomeNavigator.kt
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
1 participant
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 Content ]
홈 화면 탭에 한해서만 적용되었던 상단바 아이콘 색상 변경 로직을 모든 화면에 적용될 수 있도록 수정했습니다.
Related issue
Screenshot 📸
기존 동작
before.mp4
수정 후 동작
after.mp4
Work Description
To Reviewers 📢
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항