-
Notifications
You must be signed in to change notification settings - Fork 0
[PC-1549] OSS 라이센스 추가 #197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds Google OSS Licenses support: declares plugin and library, registers OSS activities and a dedicated theme, exposes an OSS entry in Settings with intent/side-effect flow to launch the OSS activity, and applies lifecycle callbacks to adjust window attributes for OSS activities. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant UI as SettingScreen (Compose)
participant VM as SettingViewModel
participant Collector as LaunchedEffect (lifecycle)
participant Activity as OssLicensesMenuActivity
User->>UI: Tap "오픈소스 라이센스"
UI->>VM: onIntent(OnOssClick)
VM->>VM: emit OpenOssLicense (Channel -> Flow)
Collector->>VM: collect(sideEffects)
VM-->>Collector: OpenOssLicense
Collector->>Activity: startActivity(Intent with title)
Activity-->>User: Render OSS licenses UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @app/build.gradle.kts:
- Line 82: Remove the explicit implementation(libs.google.oss.licenses)
declaration from the app module and instead declare the dependency only in the
module that directly uses it (feature/setting) if the oss.licenses plugin still
functions; verify by removing the line, running the build and the OSS Licenses
plugin tasks, and if the plugin fails, re-add the dependency to the module where
the plugin requires it—ensure the plugin application (oss.licenses) and the
implementation(libs.google.oss.licenses) presence are consistent so the plugin
can resolve the library.
🧹 Nitpick comments (2)
app/src/main/res/values/themes.xml (1)
5-12: Consider extracting hard-coded colors to color resources.The theme uses inline hex values (
#FAFAFA,#000000). For consistency with the design system, consider defining these incolors.xmland referencing them here. This is optional since this theme is isolated for the OSS licenses UI.app/src/main/AndroidManifest.xml (1)
63-69: Add explicitandroid:exported="false"attribute.For security best practices, explicitly declare
android:exported="false"for these internal-only activities, even though they lack intent-filters.🔎 Proposed fix
<activity android:name="com.google.android.gms.oss.licenses.OssLicensesMenuActivity" + android:exported="false" android:theme="@style/Theme.PieceOss" /> <activity android:name="com.google.android.gms.oss.licenses.OssLicensesActivity" + android:exported="false" android:theme="@style/Theme.PieceOss" />
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/res/values/themes.xmlbuild.gradle.ktscore/designsystem/src/main/res/values/strings.xmlfeature/setting/build.gradle.ktsfeature/setting/src/main/java/com/puzzle/setting/graph/main/SettingScreen.ktfeature/setting/src/main/java/com/puzzle/setting/graph/main/SettingViewModel.ktfeature/setting/src/main/java/com/puzzle/setting/graph/main/contract/SettingIntent.ktfeature/setting/src/main/java/com/puzzle/setting/graph/main/contract/SettingSideEffect.ktgradle/libs.versions.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (15)
feature/setting/src/main/java/com/puzzle/setting/graph/main/contract/SettingSideEffect.kt (1)
1-5: LGTM!Clean implementation of the side effect pattern using a sealed class with a data object. This correctly models the OSS license navigation event.
feature/setting/build.gradle.kts (1)
45-45: LGTM!The OSS licenses library dependency is correctly added to enable launching
OssLicensesMenuActivityfrom the settings screen.gradle/libs.versions.toml (3)
222-222: LGTM!Library alias correctly references the
play-services-oss-licensesartifact with the specified version.
240-241: LGTM!Plugin alias properly configured with the correct plugin ID
com.google.android.gms.oss-licenses-plugin.
119-122: OSS Licenses versions are current and compatible.The plugin version (
0.10.10) and library version (17.3.0) are both the latest stable releases and are compatible with each other, as confirmed by official Google documentation.build.gradle.kts (1)
15-15: LGTM!Correct pattern for declaring the OSS Licenses plugin at the root level with
apply false, allowing it to be applied in specific modules (app module).core/designsystem/src/main/res/values/strings.xml (1)
223-223: LGTM!String resource properly added following the existing naming convention (
setting_*) and placed within the Settings section.app/build.gradle.kts (1)
9-9: LGTM!The OSS Licenses plugin is correctly applied in the app module. This plugin generates the license metadata at build time that the library uses at runtime.
feature/setting/src/main/java/com/puzzle/setting/graph/main/contract/SettingIntent.kt (1)
15-15: LGTM!The new intent follows the established pattern and integrates cleanly with the existing intent hierarchy.
feature/setting/src/main/java/com/puzzle/setting/graph/main/SettingViewModel.kt (2)
38-39: LGTM!The side effect channel pattern is implemented correctly with proper Flow exposure. The BUFFERED capacity is reasonable for UI side effects.
96-96: LGTM!The OSS intent handling correctly emits the side effect through the channel. The suspend function call is appropriate within the suspending
processIntentcontext.feature/setting/src/main/java/com/puzzle/setting/graph/main/SettingScreen.kt (3)
84-98: LGTM!The side effect handling is implemented correctly with proper lifecycle awareness using
repeatOnStarted. Setting the activity title before launching provides good UX consistency.
122-122: LGTM!The OSS click handling is properly wired through the component hierarchy from route to UI, with correct callback propagation and preview support.
Also applies to: 138-138, 197-197, 458-458, 643-643
530-549: LGTM!The OSS UI entry follows the established pattern for other announcement rows and maintains visual consistency. The
setting_ossstring resource is properly defined in the design system.app/src/main/AndroidManifest.xml (1)
65-65: Theme resource is properly defined.The
@style/Theme.PieceOsstheme is correctly defined inapp/src/main/res/values/themes.xmlwithTheme.AppCompat.Lightas its parent. No action required.
| implementation(libs.kakao.user) | ||
| implementation(libs.firebase.messaging) | ||
| implementation(libs.androidx.profileinstaller) | ||
| implementation(libs.google.oss.licenses) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Check if feature/setting is a dependency of app module
rg -n "projects\.feature\.setting|:feature:setting" app/build.gradle.ktsRepository: Piece-Puzzly/Piece-Android
Length of output: 52
🏁 Script executed:
# Check if google.oss.licenses is declared in feature/setting/build.gradle.kts
find . -path "*feature/setting/build.gradle.kts" -type f | head -1 | xargs -I {} sh -c 'echo "File: {}"; rg -n "google.oss.licenses" "{}"'Repository: Piece-Puzzly/Piece-Android
Length of output: 158
🏁 Script executed:
# Get the full app/build.gradle.kts to understand dependency management
wc -l app/build.gradle.ktsRepository: Piece-Puzzly/Piece-Android
Length of output: 93
🏁 Script executed:
cat -n app/build.gradle.ktsRepository: Piece-Puzzly/Piece-Android
Length of output: 3444
🏁 Script executed:
# Check if feature/setting is a dependency of presentation
find . -path "*presentation/build.gradle.kts" -type f | head -1 | xargs -I {} sh -c 'echo "File: {}"; rg -n "feature.*setting|:feature:setting" "{}"'Repository: Piece-Puzzly/Piece-Android
Length of output: 155
🏁 Script executed:
# Check the module structure - see if there's a settings feature
find . -path "*/feature/setting/build.gradle.kts" -o -path "*/feature/settings/build.gradle.kts" | head -5Repository: Piece-Puzzly/Piece-Android
Length of output: 104
Consider removing the explicit google.oss.licenses dependency if the plugin provides it transitively.
The google.oss.licenses library is already provided transitively through feature/setting (app → presentation → feature/setting). However, since the oss.licenses plugin is applied at the app module level (line 9), the explicit dependency may be necessary for proper plugin functionality. Verify if removing this declaration breaks the plugin's operation; if not, declare the dependency only where it's directly used (feature/setting).
🤖 Prompt for AI Agents
In @app/build.gradle.kts around line 82, Remove the explicit
implementation(libs.google.oss.licenses) declaration from the app module and
instead declare the dependency only in the module that directly uses it
(feature/setting) if the oss.licenses plugin still functions; verify by removing
the line, running the build and the OSS Licenses plugin tasks, and if the plugin
fails, re-add the dependency to the module where the plugin requires it—ensure
the plugin application (oss.licenses) and the
implementation(libs.google.oss.licenses) presence are consistent so the plugin
can resolve the library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @app/src/main/java/com/puzzle/piece/PieceApplication.kt:
- Line 27: The current fragile check uses
activity.javaClass.name.contains("com.google.android.gms.oss.licenses"); replace
it with a robust runtime type check: attempt to load the OSS activity class via
Class.forName("com.google.android.gms.oss.licenses.OssLicensesMenuActivity")
(and optionally OssLicensesActivity) and use Class.isInstance(activity) to
detect the OSS activity, wrapping Class.forName in a try/catch for
ClassNotFoundException so the app behaves correctly when the OSS library is
absent; if Class.forName is unavailable or you prefer simpler checks, compare
activity.javaClass.name against the exact fully-qualified class names instead of
using contains.
- Around line 26-35: The code in onActivityResumed forcibly sets
WindowInsetsControllerCompat(...).isAppearanceLightStatusBars = true which
ignores the current dark/light theme; change this to compute the current night
mode (e.g. via activity.resources.configuration.uiMode &
Configuration.UI_MODE_NIGHT_MASK or AppCompatDelegate.getDefaultNightMode()) and
set isAppearanceLightStatusBars to true only when the UI is in light mode (false
for night/dark mode), keeping the rest of the block
(WindowCompat.setDecorFitsSystemWindows, addFlags) intact and scoped to the same
license activity check.
In @app/src/main/res/values/themes.xml:
- Around line 5-17: The Theme.PieceOss style currently uses hard-coded hex
colors—extract each hex (windowBackground, colorPrimary, colorPrimaryDark,
android:textColorPrimary, colorControlNormal, titleTextColor) into named color
resources (e.g., oss_background, oss_primary, oss_primary_dark,
oss_text_primary, oss_control_normal, oss_title_text) in colors.xml and update
Theme.PieceOss to reference those @color/... values; then add a
values-night/themes.xml variant defining Theme.PieceOss with dark-mode color
values and set the appropriate android:windowLightStatusBar there (e.g., false)
instead of keeping android:windowLightStatusBar hard-coded in the base
Theme.PieceOss so the status bar and colors adapt to dark mode.
🧹 Nitpick comments (1)
app/src/main/java/com/puzzle/piece/PieceApplication.kt (1)
39-44: Consider removing empty callback implementations.These empty function blocks add visual noise without providing functionality. Per static analysis warnings, they can be safely removed since the interface doesn't require all methods to have non-empty implementations.
Consolidate the refactor comment with simpler note
override fun onActivityResumed(activity: Activity) { if (activity.javaClass.name.contains("com.google.android.gms.oss.licenses")) { val window = activity.window val decorView = window.decorView window.addFlags(WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS) WindowInsetsControllerCompat(window, decorView).isAppearanceLightStatusBars = true WindowCompat.setDecorFitsSystemWindows(window, true) } - } - -override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) {} -override fun onActivityStarted(activity: Activity) {} -override fun onActivityPaused(activity: Activity) {} -override fun onActivityStopped(activity: Activity) {} -override fun onActivitySaveInstanceState(activity: Activity, outState: Bundle) {} -override fun onActivityDestroyed(activity: Activity) {}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/AndroidManifest.xmlapp/src/main/java/com/puzzle/piece/PieceApplication.ktapp/src/main/res/values/themes.xmlcore/designsystem/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- core/designsystem/src/main/res/values/strings.xml
- app/src/main/AndroidManifest.xml
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/puzzle/piece/PieceApplication.kt (1)
app/src/main/java/com/puzzle/piece/notification/NotificationService.kt (1)
createNotificationChannel(97-111)
🪛 detekt (1.23.8)
app/src/main/java/com/puzzle/piece/PieceApplication.kt
[warning] 39-39: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 40-40: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 41-41: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 42-42: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 43-43: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 44-44: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
app/src/main/java/com/puzzle/piece/PieceApplication.kt (2)
3-8: LGTM!All new imports are appropriately used in the lifecycle callback implementation.
18-18: LGTM!Good placement—registering lifecycle callbacks early in
onCreateensures they're active before any activities launch.app/src/main/res/values/themes.xml (1)
5-5: No action needed—the AppCompat theme is required by the Google OSS Licenses library.The Google OSS Licenses activities (
OssLicensesMenuActivityandOssLicensesActivity) extendAppCompatActivity, which requires an AppCompat-compatible theme. UsingTheme.Material.Light.NoActionBarinstead would cause a runtime error. TheTheme.AppCompat.Lighttheme choice is necessary for the OSS screen to function correctly, not an inconsistency to resolve.Likely an incorrect or invalid review comment.
| override fun onActivityResumed(activity: Activity) { | ||
| if (activity.javaClass.name.contains("com.google.android.gms.oss.licenses")) { | ||
| val window = activity.window | ||
| val decorView = window.decorView | ||
|
|
||
| window.addFlags(WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS) | ||
|
|
||
| WindowInsetsControllerCompat(window, decorView).isAppearanceLightStatusBars = true | ||
| WindowCompat.setDecorFitsSystemWindows(window, true) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded light status bar contradicts dark/light mode unification.
Line 33 forces isAppearanceLightStatusBars = true, which applies dark icons to the status bar regardless of the current theme. In dark mode, this creates dark icons on a dark background, resulting in poor visibility. The commit message claims to unify status bar icon colors across dark and light modes, but this hardcoded value prevents proper theme adaptation.
🔎 Proposed fix to respect the current theme
override fun onActivityResumed(activity: Activity) {
if (activity.javaClass.name.contains("com.google.android.gms.oss.licenses")) {
val window = activity.window
val decorView = window.decorView
window.addFlags(WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS)
- WindowInsetsControllerCompat(window, decorView).isAppearanceLightStatusBars = true
+ val insetsController = WindowInsetsControllerCompat(window, decorView)
+ val isLightTheme = (activity.resources.configuration.uiMode and
+ android.content.res.Configuration.UI_MODE_NIGHT_MASK) !=
+ android.content.res.Configuration.UI_MODE_NIGHT_YES
+ insetsController.isAppearanceLightStatusBars = isLightTheme
WindowCompat.setDecorFitsSystemWindows(window, true)
}
}📝 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.
| override fun onActivityResumed(activity: Activity) { | |
| if (activity.javaClass.name.contains("com.google.android.gms.oss.licenses")) { | |
| val window = activity.window | |
| val decorView = window.decorView | |
| window.addFlags(WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS) | |
| WindowInsetsControllerCompat(window, decorView).isAppearanceLightStatusBars = true | |
| WindowCompat.setDecorFitsSystemWindows(window, true) | |
| } | |
| override fun onActivityResumed(activity: Activity) { | |
| if (activity.javaClass.name.contains("com.google.android.gms.oss.licenses")) { | |
| val window = activity.window | |
| val decorView = window.decorView | |
| window.addFlags(WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS) | |
| val insetsController = WindowInsetsControllerCompat(window, decorView) | |
| val isLightTheme = (activity.resources.configuration.uiMode and | |
| android.content.res.Configuration.UI_MODE_NIGHT_MASK) != | |
| android.content.res.Configuration.UI_MODE_NIGHT_YES | |
| insetsController.isAppearanceLightStatusBars = isLightTheme | |
| WindowCompat.setDecorFitsSystemWindows(window, true) | |
| } | |
| } |
🤖 Prompt for AI Agents
In @app/src/main/java/com/puzzle/piece/PieceApplication.kt around lines 26-35,
The code in onActivityResumed forcibly sets
WindowInsetsControllerCompat(...).isAppearanceLightStatusBars = true which
ignores the current dark/light theme; change this to compute the current night
mode (e.g. via activity.resources.configuration.uiMode &
Configuration.UI_MODE_NIGHT_MASK or AppCompatDelegate.getDefaultNightMode()) and
set isAppearanceLightStatusBars to true only when the UI is in light mode (false
for night/dark mode), keeping the rest of the block
(WindowCompat.setDecorFitsSystemWindows, addFlags) intact and scoped to the same
license activity check.
| private fun registerOssLicenseLifecycleCallbacks() { | ||
| registerActivityLifecycleCallbacks(object : ActivityLifecycleCallbacks { | ||
| override fun onActivityResumed(activity: Activity) { | ||
| if (activity.javaClass.name.contains("com.google.android.gms.oss.licenses")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String matching on package name is fragile.
Relying on activity.javaClass.name.contains("com.google.android.gms.oss.licenses") couples this code to Google's internal package structure. If the package name changes in future library updates, this check will silently fail.
🔎 More robust alternatives
Option 1: Check the actual activity class type
try {
val ossActivityClass = Class.forName("com.google.android.gms.oss.licenses.OssLicensesMenuActivity")
if (ossActivityClass.isInstance(activity)) {
// apply window configurations
}
} catch (e: ClassNotFoundException) {
// OSS library not available
}Option 2: Use a more specific package check
if (activity.javaClass.name == "com.google.android.gms.oss.licenses.OssLicensesMenuActivity" ||
activity.javaClass.name == "com.google.android.gms.oss.licenses.OssLicensesActivity") {
// apply window configurations
}🤖 Prompt for AI Agents
In @app/src/main/java/com/puzzle/piece/PieceApplication.kt around line 27, The
current fragile check uses
activity.javaClass.name.contains("com.google.android.gms.oss.licenses"); replace
it with a robust runtime type check: attempt to load the OSS activity class via
Class.forName("com.google.android.gms.oss.licenses.OssLicensesMenuActivity")
(and optionally OssLicensesActivity) and use Class.isInstance(activity) to
detect the OSS activity, wrapping Class.forName in a try/catch for
ClassNotFoundException so the app behaves correctly when the OSS library is
absent; if Class.forName is unavailable or you prefer simpler checks, compare
activity.javaClass.name against the exact fully-qualified class names instead of
using contains.
| <style name="Theme.PieceOss" parent="Theme.AppCompat.Light"> | ||
| <item name="android:fitsSystemWindows">true</item> | ||
|
|
||
| <item name="android:windowBackground">#FFFFFF</item> | ||
| <item name="colorPrimary">#FFFFFF</item> | ||
| <item name="colorPrimaryDark">#FFFFFF</item> | ||
|
|
||
| <item name="android:textColorPrimary">#484B4D</item> | ||
| <item name="colorControlNormal">#E8EBF0</item> | ||
| <item name="titleTextColor">#1B1A2A</item> | ||
|
|
||
| <item name="android:windowLightStatusBar">true</item> | ||
| </style> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extract hard-coded colors to color resources and add dark mode support.
The theme uses hard-coded hex values which prevents proper dark mode support. The commit message mentions unifying status bar icons across dark/light modes, but without a values-night variant, the OSS screen will always display with a white background and light theme attributes, even when the system is in dark mode.
Consider:
- Extract all color values to
colors.xml(e.g.,@color/oss_background,@color/oss_primary) - Create a
values-night/themes.xmlvariant with appropriate dark mode colors - Remove the hard-coded
android:windowLightStatusBarvalue so it can adapt based on the theme variant
🔎 Proposed refactor
In values/themes.xml:
<style name="Theme.PieceOss" parent="Theme.AppCompat.Light">
<item name="android:fitsSystemWindows">true</item>
- <item name="android:windowBackground">#FFFFFF</item>
- <item name="colorPrimary">#FFFFFF</item>
- <item name="colorPrimaryDark">#FFFFFF</item>
+ <item name="android:windowBackground">@color/oss_background</item>
+ <item name="colorPrimary">@color/oss_primary</item>
+ <item name="colorPrimaryDark">@color/oss_primary_dark</item>
- <item name="android:textColorPrimary">#484B4D</item>
- <item name="colorControlNormal">#E8EBF0</item>
- <item name="titleTextColor">#1B1A2A</item>
+ <item name="android:textColorPrimary">@color/oss_text_primary</item>
+ <item name="colorControlNormal">@color/oss_control_normal</item>
+ <item name="titleTextColor">@color/oss_title_text</item>
<item name="android:windowLightStatusBar">true</item>
</style>Then create values-night/themes.xml:
<?xml version="1.0" encoding="utf-8"?>
<resources>
<style name="Theme.PieceOss" parent="Theme.AppCompat">
<item name="android:fitsSystemWindows">true</item>
<item name="android:windowBackground">@color/oss_background</item>
<item name="colorPrimary">@color/oss_primary</item>
<item name="colorPrimaryDark">@color/oss_primary_dark</item>
<item name="android:textColorPrimary">@color/oss_text_primary</item>
<item name="colorControlNormal">@color/oss_control_normal</item>
<item name="titleTextColor">@color/oss_title_text</item>
<item name="android:windowLightStatusBar">false</item>
</style>
</resources>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @app/src/main/res/values/themes.xml around lines 5-17, The Theme.PieceOss
style currently uses hard-coded hex colors—extract each hex (windowBackground,
colorPrimary, colorPrimaryDark, android:textColorPrimary, colorControlNormal,
titleTextColor) into named color resources (e.g., oss_background, oss_primary,
oss_primary_dark, oss_text_primary, oss_control_normal, oss_title_text) in
colors.xml and update Theme.PieceOss to reference those @color/... values; then
add a values-night/themes.xml variant defining Theme.PieceOss with dark-mode
color values and set the appropriate android:windowLightStatusBar there (e.g.,
false) instead of keeping android:windowLightStatusBar hard-coded in the base
Theme.PieceOss so the status bar and colors adapt to dark mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @app/src/main/java/com/puzzle/piece/PieceApplication.kt:
- Around line 30-33: The action bar background is hardcoded to Color.WHITE in
the supportActionBar.apply block, which breaks dark mode; change the code in the
supportActionBar apply (the block starting with (activity as?
androidx.appcompat.app.AppCompatActivity)?.supportActionBar?.apply) to resolve
and use a theme attribute color (e.g.,
com.google.android.material.R.attr.colorSurface or your app's surface/primary
color) via android.util.TypedValue and activity.theme.resolveAttribute, then
setBackgroundDrawable using the resolved typedValue.data and keep elevation = 0f
so the action bar respects light/dark themes.
♻️ Duplicate comments (1)
app/src/main/res/values/themes.xml (1)
5-19: Previous review concern about hard-coded colors and dark mode support remains unaddressed.The hard-coded hex color values and lack of a
values-nighttheme variant still prevent proper dark mode support, as flagged in the previous review. This means the OSS licenses screen will always display with a light theme regardless of the user's system theme preference.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/puzzle/piece/PieceApplication.ktapp/src/main/res/values/themes.xml
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/puzzle/piece/PieceApplication.kt (1)
app/src/main/java/com/puzzle/piece/notification/NotificationService.kt (1)
createNotificationChannel(97-111)
🪛 detekt (1.23.8)
app/src/main/java/com/puzzle/piece/PieceApplication.kt
[warning] 39-39: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 40-40: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 41-41: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 42-42: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 43-43: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 44-44: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
| (activity as? androidx.appcompat.app.AppCompatActivity)?.supportActionBar?.apply { | ||
| setBackgroundDrawable(android.graphics.drawable.ColorDrawable(android.graphics.Color.WHITE)) | ||
| elevation = 0f | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded white action bar breaks dark mode.
The action bar background is forced to Color.WHITE regardless of the app's current theme. In dark mode, this will display a jarring white action bar, breaking visual consistency and potentially causing poor user experience.
🔎 Proposed fix to respect the current theme
(activity as? androidx.appcompat.app.AppCompatActivity)?.supportActionBar?.apply {
- setBackgroundDrawable(android.graphics.drawable.ColorDrawable(android.graphics.Color.WHITE))
+ val backgroundColor = if ((activity.resources.configuration.uiMode and
+ android.content.res.Configuration.UI_MODE_NIGHT_MASK) ==
+ android.content.res.Configuration.UI_MODE_NIGHT_YES) {
+ android.graphics.Color.parseColor("#121212") // Material dark surface
+ } else {
+ android.graphics.Color.WHITE
+ }
+ setBackgroundDrawable(android.graphics.drawable.ColorDrawable(backgroundColor))
elevation = 0f
}Alternatively, consider using a theme attribute instead of hardcoding colors:
(activity as? androidx.appcompat.app.AppCompatActivity)?.supportActionBar?.apply {
val typedValue = android.util.TypedValue()
activity.theme.resolveAttribute(
com.google.android.material.R.attr.colorSurface,
typedValue,
true
)
setBackgroundDrawable(android.graphics.drawable.ColorDrawable(typedValue.data))
elevation = 0f
}📝 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.
| (activity as? androidx.appcompat.app.AppCompatActivity)?.supportActionBar?.apply { | |
| setBackgroundDrawable(android.graphics.drawable.ColorDrawable(android.graphics.Color.WHITE)) | |
| elevation = 0f | |
| } | |
| (activity as? androidx.appcompat.app.AppCompatActivity)?.supportActionBar?.apply { | |
| val backgroundColor = if ((activity.resources.configuration.uiMode and | |
| android.content.res.Configuration.UI_MODE_NIGHT_MASK) == | |
| android.content.res.Configuration.UI_MODE_NIGHT_YES) { | |
| android.graphics.Color.parseColor("#121212") // Material dark surface | |
| } else { | |
| android.graphics.Color.WHITE | |
| } | |
| setBackgroundDrawable(android.graphics.drawable.ColorDrawable(backgroundColor)) | |
| elevation = 0f | |
| } |
🤖 Prompt for AI Agents
In @app/src/main/java/com/puzzle/piece/PieceApplication.kt around lines 30-33,
The action bar background is hardcoded to Color.WHITE in the
supportActionBar.apply block, which breaks dark mode; change the code in the
supportActionBar apply (the block starting with (activity as?
androidx.appcompat.app.AppCompatActivity)?.supportActionBar?.apply) to resolve
and use a theme attribute color (e.g.,
com.google.android.material.R.attr.colorSurface or your app's surface/primary
color) via android.util.TypedValue and activity.theme.resolveAttribute, then
setBackgroundDrawable using the resolved typedValue.data and keep elevation = 0f
so the action bar respects light/dark themes.
1. ⭐️ 변경된 내용
2. 🖼️ 스크린샷(선택)
3. 💡 알게된 부분
4. 📌 이 부분은 꼭 봐주세요!
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.