Add Open-source Licenses screen to Settings#141
Conversation
Integrate mikepenz/AboutLibraries 14.0.1 so the app can display the licenses of every runtime dependency. The Gradle plugin generates aboutlibraries.json at build time, so the list stays in sync with the version catalog automatically — no manual upkeep per library change. A new clickable "Open-source Licenses" row is added just above About in Settings. It navigates to LicensesScreen, which loads the generated data via produceLibraries() and renders it with the Material 3 LibrariesContainer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 50 minutes and 16 seconds. ⌛ 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)
📝 WalkthroughWalkthroughThis pull request adds AboutLibraries integration to enable displaying open-source licenses in the app. The implementation includes updating the Gradle build configuration with the AboutLibraries plugin and dependencies, creating a new LicensesScreen composable to display libraries, integrating it into the navigation system via HackersPubApp, adding a "Licenses" menu item to SettingsScreen, and adding corresponding string resources. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/src/main/java/pub/hackers/android/ui/screens/settings/SettingsScreen.kt (1)
397-401: Icon duplicates the "My Drafts" row.
Icons.AutoMirrored.Outlined.Articleis already used for the "My Drafts" row at lines 222-225. Using the same icon for "Open-source Licenses" may confuse users scanning the Settings list. Consider a more distinctive icon such asIcons.Outlined.Description,Icons.Outlined.Gavel, orIcons.AutoMirrored.Outlined.LibraryBooks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/pub/hackers/android/ui/screens/settings/SettingsScreen.kt` around lines 397 - 401, The "Open-source Licenses" row in SettingsScreen.kt is using the same icon as "My Drafts" (Icons.AutoMirrored.Outlined.Article) which can confuse users; locate the Icon composable within the Open-source Licenses row (the Icon call using Icons.AutoMirrored.Outlined.Article) and replace it with a more distinctive icon such as Icons.Outlined.Description, Icons.Outlined.Gavel, or Icons.AutoMirrored.Outlined.LibraryBooks, keeping the same contentDescription and tint (colors.textSecondary) so only the imageVector changes.app/src/main/java/pub/hackers/android/ui/screens/licenses/LicensesScreen.kt (1)
28-51: Consider scroll behavior and edge-to-edge insets on the TopAppBar.Minor polish suggestions for consistency with other detail screens:
TopAppBaris not wired to ascrollBehavior, so it won't collapse/elevate on scroll. If other detail screens (e.g.,WebViewScreen) useTopAppBarDefaults.pinnedScrollBehavior()or similar, matching that here keeps the UX consistent.Scaffolduses defaultcontentWindowInsets; other screens in this app (e.g.,SettingsScreenat line 133) passcontentWindowInsets = WindowInsets(0). Worth verifying the licenses list doesn't double-pad under system bars.LibrariesContainerhandles its ownLazyColumn; passingModifier.fillMaxSize().padding(innerPadding)plus a separatecontentPaddingof 8dp vertical is fine, but you may want horizontal content padding too so license cards don't hug the screen edges.None of these are blocking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/pub/hackers/android/ui/screens/licenses/LicensesScreen.kt` around lines 28 - 51, The TopAppBar in LicensesScreen isn't using a scroll behavior and Scaffold is using default window insets which can cause inconsistent edge-to-edge padding; update LicensesScreen to create a scrollBehavior (e.g., val scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior()) and pass it into TopAppBar(scrollBehavior = scrollBehavior) and Scaffold (topBar = { ... }, contentWindowInsets = WindowInsets(0)) so the app bar collapses/elevates like other detail screens; also adjust the LibrariesContainer invocation (or its Modifier) to add horizontal padding (or set contentPadding = PaddingValues(horizontal = 16.dp, vertical = 8.dp)) so license cards don't hug screen edges while keeping innerPadding applied from Scaffold and using produceLibraries() as the data source.app/build.gradle.kts (1)
9-9: Consider configuring the AboutLibraries plugin explicitly if stricter license metadata validation is desired.The default settings are already reasonable:
fetchRemoteLicensedefaults to false in plugin v14.0.1, so build determinism and offline-friendliness are already protected. If you want stricter license checks during CI builds, add anaboutLibraries { }extension block withstrictMode = StrictMode.FAILunder thelicenseblock to catch missing metadata before shipping.Optional; the current defaults are generally fine for 238 libraries detected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/build.gradle.kts` at line 9, The AboutLibraries plugin is applied via alias(libs.plugins.aboutlibraries) but not configured; to enable stricter license metadata validation add an aboutLibraries { license { strictMode = StrictMode.FAIL } } extension so CI fails on missing/invalid license info. Update the Gradle Kotlin DSL to declare the aboutLibraries extension and set license.strictMode to StrictMode.FAIL (referencing aboutLibraries, license, and StrictMode) so missing metadata are caught during builds.
🤖 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/pub/hackers/android/ui/screens/licenses/LicensesScreen.kt`:
- Line 43: produceLibraries() is asynchronous so `libraries` can be null
initially and after config changes—update LicensesScreen to guard against null
by showing a loading indicator or an explicit empty-state and only call
LibrariesContainer when `libraries` is non-null (reference the val libraries by
produceLibraries() and the LibrariesContainer composable); additionally add an
R8/resource-keep entry to prevent release builds stripping the generated
aboutlibraries.json by adding the rule `-keepresources `@raw/aboutlibraries.json``
to app/proguard-rules.pro so the libraries resource can be loaded in release
builds.
In `@app/src/main/res/values/strings.xml`:
- Line 158: The string resource named "licenses" is currently English-only;
either provide localized translations for it to match the app's other locales or
explicitly mark it as non-translatable to avoid lint warnings — update the
<string name="licenses"> entry (or add corresponding localized versions in the
locale-specific values folders) or add translatable="false" to the "licenses"
string to indicate it should not be translated.
---
Nitpick comments:
In `@app/build.gradle.kts`:
- Line 9: The AboutLibraries plugin is applied via
alias(libs.plugins.aboutlibraries) but not configured; to enable stricter
license metadata validation add an aboutLibraries { license { strictMode =
StrictMode.FAIL } } extension so CI fails on missing/invalid license info.
Update the Gradle Kotlin DSL to declare the aboutLibraries extension and set
license.strictMode to StrictMode.FAIL (referencing aboutLibraries, license, and
StrictMode) so missing metadata are caught during builds.
In `@app/src/main/java/pub/hackers/android/ui/screens/licenses/LicensesScreen.kt`:
- Around line 28-51: The TopAppBar in LicensesScreen isn't using a scroll
behavior and Scaffold is using default window insets which can cause
inconsistent edge-to-edge padding; update LicensesScreen to create a
scrollBehavior (e.g., val scrollBehavior =
TopAppBarDefaults.pinnedScrollBehavior()) and pass it into
TopAppBar(scrollBehavior = scrollBehavior) and Scaffold (topBar = { ... },
contentWindowInsets = WindowInsets(0)) so the app bar collapses/elevates like
other detail screens; also adjust the LibrariesContainer invocation (or its
Modifier) to add horizontal padding (or set contentPadding =
PaddingValues(horizontal = 16.dp, vertical = 8.dp)) so license cards don't hug
screen edges while keeping innerPadding applied from Scaffold and using
produceLibraries() as the data source.
In `@app/src/main/java/pub/hackers/android/ui/screens/settings/SettingsScreen.kt`:
- Around line 397-401: The "Open-source Licenses" row in SettingsScreen.kt is
using the same icon as "My Drafts" (Icons.AutoMirrored.Outlined.Article) which
can confuse users; locate the Icon composable within the Open-source Licenses
row (the Icon call using Icons.AutoMirrored.Outlined.Article) and replace it
with a more distinctive icon such as Icons.Outlined.Description,
Icons.Outlined.Gavel, or Icons.AutoMirrored.Outlined.LibraryBooks, keeping the
same contentDescription and tint (colors.textSecondary) so only the imageVector
changes.
🪄 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: b3b0ea75-a993-4b29-9a38-9b731af30755
📒 Files selected for processing (7)
app/build.gradle.ktsapp/src/main/java/pub/hackers/android/ui/HackersPubApp.ktapp/src/main/java/pub/hackers/android/ui/screens/licenses/LicensesScreen.ktapp/src/main/java/pub/hackers/android/ui/screens/settings/SettingsScreen.ktapp/src/main/res/values/strings.xmlbuild.gradle.ktsgradle/libs.versions.toml
- Switch the Licenses row icon from Icons.AutoMirrored.Outlined.Article (shared with My Drafts) to Icons.AutoMirrored.Outlined.LibraryBooks so the row is distinguishable at a glance. - Set contentWindowInsets = WindowInsets(0) on LicensesScreen's Scaffold to match the rest of the detail screens and avoid double-padding under system bars. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the review! Addressed the reasonable ones in ff28a65 and documented why the rest are skipped. Applied
Skipped — with reasons
|
Summary
aboutlibraries.jsonat build time, so the license list stays in sync with the version catalog automatically.LicensesScreenloads the generated data viaproduceLibraries()and renders it with the Material 3LibrariesContainer.Screens / navigation
Icons.AutoMirrored.Outlined.Article) →LicensesScreen.LicensesScreenis registered on the NavHost asDetailScreen.Licensesand has a standardTopAppBarwith back navigation, matchingWebViewScreen.Notes
values-ko/exists in this repo yet).aboutlibraries.jsonis emitted underapp/build/generated/aboutLibraries/…/res/raw/and is bundled as a raw resource, so there is no runtime network dependency.Test plan
./gradlew :app:assembleDebugsucceeds./gradlew :app:lintDebugsucceeds with no new warnings./gradlew :app:testDebugUnitTestpasses🤖 Generated with Claude Code