-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
android(i18n): externalize hardcoded strings and add Chinese translations #395
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
base: main
Are you sure you want to change the base?
Conversation
…ions - Extract hardcoded strings to strings.xml for Onboarding, TroubleshootingScreen, AppSettingsScreen, HearingAidScreen, and services - Add corresponding Chinese Simplified translations
📝 WalkthroughWalkthroughReplaces hardcoded UI text with resource lookups across multiple Compose screens, Quick Settings and services, adds English and Simplified Chinese string resources, introduces debounced navigation and safe popBackStack utilities, and applies status bar top padding to a back button. Changes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
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
🧹 Nitpick comments (7)
android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsQSService.kt (1)
252-260: Remaining hardcoded string: "Unknown"The fallback string
"Unknown"on line 258 should also be externalized for consistency with the localization effort.🔎 Suggested fix
private fun getModeLabel(mode: Int): String { return when (mode) { NoiseControlMode.OFF.ordinal + 1 -> getString(R.string.off) NoiseControlMode.TRANSPARENCY.ordinal + 1 -> getString(R.string.transparency) NoiseControlMode.ADAPTIVE.ordinal + 1 -> getString(R.string.adaptive) NoiseControlMode.NOISE_CANCELLATION.ordinal + 1 -> getString(R.string.noise_cancellation) - else -> "Unknown" + else -> getString(R.string.unknown) } }android/app/src/main/java/me/kavishdevar/librepods/screens/AppSettingsScreen.kt (1)
786-786: Consider using string format placeholders for error messages.Concatenating localized strings (
errorText + " " + (e.message ?: unknownErrorText)) may produce awkward results in languages with different word order. Consider using a format string with a placeholder (e.g.,R.string.error_converting_hex_with_detailwith%s).This is a minor issue and can be addressed in a future iteration.
android/app/src/main/java/me/kavishdevar/librepods/QuickSettingsDialogActivity.kt (1)
601-601: Fragile string manipulation for layout.Using
.replace(" ", "\n")to force line breaks may not work well for all locales (e.g., Chinese doesn't use spaces as word separators). Consider using a separate string resource with explicit newline or adjusting the layout constraints.android/app/src/main/java/me/kavishdevar/librepods/screens/TroubleshootingScreen.kt (1)
243-337: Finish localizing remaining Troubleshooting UI stringsWithin
TroubleshootingScreen, a few user‑visible strings remain hardcoded in English even though you already added matching resources:
- Line 278:
"Total Logs: ${savedLogs.size}"could usestringResource(R.string.total_logs, savedLogs.size).- Line 592: chooser title
"Share log file"can usecontext.getString(R.string.share_log_file)for theIntent.createChoosertitle.- Lines 884–905: bottom‑sheet buttons still use
"Share"/"Save"instead of the existingR.string.share/R.string.save.Refactoring these to use the new resources will make the troubleshooting flow fully localizable and consistent with the rest of the PR.
Proposed diffs
- Text( - text = "Total Logs: ${savedLogs.size}", + Text( + text = stringResource(R.string.total_logs, savedLogs.size), fontSize = 16.sp, fontWeight = FontWeight.Medium, color = textColor )- context.startActivity( - Intent.createChooser( - shareIntent, - "Share log file" - ) - ) + context.startActivity( + Intent.createChooser( + shareIntent, + context.getString(R.string.share_log_file) + ) + )- Text("Share") + Text(stringResource(R.string.share)) ... - Text("Save") + Text(stringResource(R.string.save))Also applies to: 408-747, 845-906
android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt (1)
1552-1559: Localize remaining inline notification text (Error: …, “Reconnect”)Two small pieces of user‑visible text are still hardcoded:
- Line 1557–1558: BigText appends
"Error: $errorMessage"to a localized description, so the “Error:” label remains English in all locales.- Line 1841: the action label
"Reconnect"is a literal.Consider moving these into string resources, e.g.:
- Add a formatted string like
l2cap_connection_failed_detail_error="… Error: %s"and usegetString(R.string.l2cap_connection_failed_detail_error, errorMessage).- Add a
reconnectstring for the notification action label.This keeps logs/error details intact while making all notification chrome localizable.
Also applies to: 1838-1850
android/app/src/main/res/values-zh-rCN/strings.xml (1)
218-319: zh‑CN additions align with English keys and formattingThe new Simplified Chinese strings correctly mirror the English keys (permissions, onboarding, troubleshooting, notifications, etc.), and all formatted entries (
%d,%s) preserve the same placeholders, so runtime formatting will be safe. Only minor wording/spacing nits remain (e.g., consider adding a space in “蓝牙低功耗广播的ENC_KEY 值” → “…的 ENC_KEY 值”), but nothing blocking.android/app/src/main/res/values/strings.xml (1)
217-318: New English string resources are technically soundAll added keys (permissions, onboarding, troubleshooting, notifications, etc.) are well‑formed, match usage in Kotlin (including the
%s/%dplaceholders), and line up with the zh‑CN translations. Only minor wording polish is left (e.g., “Checking if radare2 are already installed” → “is already installed”), which you can adjust later without impacting behavior.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt(8 hunks)android/app/src/main/java/me/kavishdevar/librepods/QuickSettingsDialogActivity.kt(4 hunks)android/app/src/main/java/me/kavishdevar/librepods/screens/AppSettingsScreen.kt(10 hunks)android/app/src/main/java/me/kavishdevar/librepods/screens/HearingAidScreen.kt(1 hunks)android/app/src/main/java/me/kavishdevar/librepods/screens/Onboarding.kt(15 hunks)android/app/src/main/java/me/kavishdevar/librepods/screens/TroubleshootingScreen.kt(18 hunks)android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsQSService.kt(2 hunks)android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt(4 hunks)android/app/src/main/res/values-zh-rCN/strings.xml(1 hunks)android/app/src/main/res/values/strings.xml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: abc0922001
Repo: kavishdevar/librepods PR: 365
File: linux/translations/librepods_zh_TW.ts:97-99
Timestamp: 2025-12-11T00:55:21.542Z
Learning: In Traditional Chinese translations for this project (librepods), the localization style prefers using singular forms even when the English source uses plural forms (e.g., "Magic Cloud Keys" is translated as "Magic Cloud Key" without the "s").
🧬 Code graph analysis (1)
android/app/src/main/java/me/kavishdevar/librepods/screens/AppSettingsScreen.kt (1)
android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt (1)
validateHexInput(248-251)
🔇 Additional comments (22)
android/app/src/main/java/me/kavishdevar/librepods/screens/HearingAidScreen.kt (1)
266-269: LGTM!The dialog strings are properly externalized using
stringResource, which is correctly used within the Composable context.android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsQSService.kt (1)
217-217: LGTM!Correctly uses
getString()for the tile subtitle in this non-ComposableTileServicecontext.android/app/src/main/java/me/kavishdevar/librepods/screens/AppSettingsScreen.kt (6)
658-658: LGTM!Dialog title properly externalized.
764-769: LGTM!String resources for validation error and unknown error are properly pre-fetched before the onClick callback to avoid accessing
stringResourceoutside Composable scope.
791-802: LGTM!Save and Cancel button texts are properly externalized.
857-879: LGTM!Encryption key dialog follows the same pattern as the IRK dialog - string resources are properly pre-fetched and used.
884-895: LGTM!Save and Cancel button texts for encryption key dialog are properly externalized.
964-975: LGTM!Camera dialog button texts are properly externalized.
android/app/src/main/java/me/kavishdevar/librepods/QuickSettingsDialogActivity.kt (1)
617-617: LGTM!Loading text properly externalized.
android/app/src/main/java/me/kavishdevar/librepods/screens/Onboarding.kt (10)
159-159: LGTM!Scaffold title properly externalized.
263-263: LGTM!Button text properly externalized.
277-300: LGTM!AnimatedContent targets properly use the new composable helper functions for localized status text.
329-329: LGTM!Start setup button text properly externalized.
384-384: LGTM!Module enabled confirmation button text properly externalized.
404-404: LGTM!Bluetooth toggled confirmation button text properly externalized.
428-428: LGTM!Continue to settings button text properly externalized.
478-478: LGTM!Try again button text properly externalized.
489-526: LGTM!Skip setup dialog with title, description, confirm and dismiss button texts are all properly externalized.
585-637: LGTM!The
getStatusTitleandgetStatusDescriptionhelper functions are correctly annotated with@Composableto enablestringResourceusage. The error state appropriately falls back tostate.messagesince system error messages are typically not user-localizable.android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt (1)
553-704: PermissionsScreen i18n refactor looks correctAll permission titles, descriptions, and button labels now use
stringResource(...)with consistent keys; there are no formatting parameters to mismatch, and behavior is unchanged.android/app/src/main/java/me/kavishdevar/librepods/screens/TroubleshootingScreen.kt (1)
168-202: Log save toasts and step instructions use resources correctlyThe success/error toasts (
log_saved,failed_save_log) and troubleshooting step texts (troubleshooting_step_0–4) correctly usegetStringwith matching format arguments; no behavioral changes beyond localization.android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsService.kt (1)
1484-1511: Notification i18n wiring is correctThe background, connection‑issue, and “not connected” notifications now pull titles and bodies from string resources (
background_service_running,useless_notification,airpods_connection_issue,l2cap_connection_failed,l2cap_connection_failed_detail,airpods_not_connected,tap_to_open_app), keeping behavior identical while enabling localization.Also applies to: 1523-1527, 1552-1563, 1858-1868
| @Composable | ||
| private fun getModeLabel(mode: NoiseControlMode): String { | ||
| return when (mode) { | ||
| NoiseControlMode.OFF -> "Off" | ||
| NoiseControlMode.TRANSPARENCY -> "Transparency" | ||
| NoiseControlMode.ADAPTIVE -> "Adaptive" | ||
| NoiseControlMode.NOISE_CANCELLATION -> "Noise Cancel" | ||
| NoiseControlMode.OFF -> stringResource(R.string.off) | ||
| NoiseControlMode.TRANSPARENCY -> stringResource(R.string.transparency) | ||
| NoiseControlMode.ADAPTIVE -> stringResource(R.string.adaptive) | ||
| NoiseControlMode.NOISE_CANCELLATION -> stringResource(R.string.noise_cancel) | ||
| } |
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:
#!/bin/bash
# Check if both noise_cancel and noise_cancellation string resources exist
echo "=== Checking strings.xml for noise_cancel and noise_cancellation ==="
rg -n "noise_cancel" android/app/src/main/res/values/strings.xml
rg -n "noise_cancellation" android/app/src/main/res/values/strings.xmlRepository: kavishdevar/librepods
Length of output: 1071
🏁 Script executed:
#!/bin/bash
# Check the actual usage in AirPodsQSService.kt around line 257
echo "=== AirPodsQSService.kt around line 257 ==="
rg -n "noise_cancellation\|noise_cancel" android/app/src/main/java/me/kavishdevar/librepods/AirPodsQSService.kt
echo ""
# Check if the function in QuickSettingsDialogActivity.kt exists and verify line numbers
echo "=== QuickSettingsDialogActivity.kt - getModeLabel function ==="
rg -n "getModeLabel\|noise_cancel" android/app/src/main/java/me/kavishdevar/librepods/QuickSettingsDialogActivity.kt | head -20Repository: kavishdevar/librepods
Length of output: 300
🏁 Script executed:
#!/bin/bash
# Find the actual file locations
echo "=== Finding AirPodsQSService.kt ==="
fd "AirPodsQSService.kt" android/
echo ""
echo "=== Finding QuickSettingsDialogActivity.kt ==="
fd "QuickSettingsDialogActivity.kt" android/Repository: kavishdevar/librepods
Length of output: 310
🏁 Script executed:
#!/bin/bash
# Check usage in AirPodsQSService.kt around line 257
echo "=== AirPodsQSService.kt - searching for noise_cancel/noise_cancellation ==="
rg -n "noise_cancel|noise_cancellation" android/app/src/main/java/me/kavishdevar/librepods/services/AirPodsQSService.kt
echo ""
# Check usage in QuickSettingsDialogActivity.kt
echo "=== QuickSettingsDialogActivity.kt - getModeLabel function ==="
rg -n "getModeLabel|noise_cancel|noise_cancellation" android/app/src/main/java/me/kavishdevar/librepods/QuickSettingsDialogActivity.kt | head -30Repository: kavishdevar/librepods
Length of output: 962
Consolidate string resource keys for Noise Cancellation mode.
AirPodsQSService.kt (line 257) uses R.string.noise_cancellation ("Noise Cancellation"), while QuickSettingsDialogActivity.kt (line 639) uses R.string.noise_cancel ("Noise Cancel") for the same NoiseControlMode.NOISE_CANCELLATION. Both resources exist but with different text. Standardize on a single resource key across both files for consistency.
🤖 Prompt for AI Agents
In
android/app/src/main/java/me/kavishdevar/librepods/QuickSettingsDialogActivity.kt
around lines 633 to 640, the NoiseControlMode.NOISE_CANCELLATION case returns
stringResource(R.string.noise_cancel) which is inconsistent with
AirPodsQSService.kt that uses R.string.noise_cancellation; change the
QuickSettingsDialogActivity usage to stringResource(R.string.noise_cancellation)
so both files reference the same resource key (or alternatively update both to a
single chosen key), and run a quick build/strings search to ensure no other
mismatched keys remain.
- Add NavigationExtensions.kt with navigateDebounced() and popBackStackSafely() - Replace navigate() with navigateDebounced() to prevent rapid repeated clicks - Replace popBackStack() with popBackStackSafely() to prevent empty stack crashes - Fix status bar padding calculation using WindowInsets
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/src/main/java/me/kavishdevar/librepods/screens/AppSettingsScreen.kt (1)
765-787: Consider localizing exception error messages.Lines 787 and 880 concatenate localized error text with exception messages:
errorText + " " + (e.message ?: unknownErrorText). Sincee.messageis typically in English and won't be localized, this creates mixed-language error messages for non-English users.Consider one of these approaches:
- Use only
unknownErrorTextwithout the exception message:irkError.value = errorText + " " + unknownErrorText- Log the exception details separately and show only localized messages to users
🔎 Proposed fix to use only localized strings
} catch (e: Exception) { - irkError.value = errorText + " " + (e.message ?: unknownErrorText) + irkError.value = "$errorText $unknownErrorText" + e.printStackTrace() // Log the detailed error for debugging }Apply the same pattern at line 880 for the encryption key dialog.
Also applies to: 858-880
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt(12 hunks)android/app/src/main/java/me/kavishdevar/librepods/composables/NavigationButton.kt(2 hunks)android/app/src/main/java/me/kavishdevar/librepods/screens/AirPodsSettingsScreen.kt(3 hunks)android/app/src/main/java/me/kavishdevar/librepods/screens/AppSettingsScreen.kt(12 hunks)android/app/src/main/java/me/kavishdevar/librepods/screens/Onboarding.kt(17 hunks)android/app/src/main/java/me/kavishdevar/librepods/utils/NavigationExtensions.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- android/app/src/main/java/me/kavishdevar/librepods/screens/Onboarding.kt
🧰 Additional context used
🧬 Code graph analysis (1)
android/app/src/main/java/me/kavishdevar/librepods/screens/AppSettingsScreen.kt (1)
android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt (1)
validateHexInput(252-255)
🔇 Additional comments (8)
android/app/src/main/java/me/kavishdevar/librepods/composables/NavigationButton.kt (1)
54-54: LGTM! Navigation debouncing correctly applied.The import and usage of
navigateDebouncedprevents accidental double-tap navigation issues, aligning with the project-wide navigation safety improvements.Also applies to: 99-99
android/app/src/main/java/me/kavishdevar/librepods/screens/AirPodsSettingsScreen.kt (1)
71-71: LGTM! Debounced navigation consistently applied.Both navigation calls now use the debounced variant, preventing rapid repeated navigations to app settings and troubleshooting screens.
Also applies to: 231-231, 384-384
android/app/src/main/java/me/kavishdevar/librepods/screens/AppSettingsScreen.kt (2)
76-76: LGTM! Debounced navigation with options correctly implemented.The navigation to onboarding properly uses
navigateDebouncedwith a builder to configurepopUpTo, maintaining both debounce protection and custom navigation behavior.Also applies to: 682-684
659-659: LGTM! Dialog strings properly localized.All dialog titles and button labels now use
stringResource, supporting the internationalization effort across the app.Also applies to: 710-710, 792-792, 803-803, 885-885, 896-896, 965-965, 976-976
android/app/src/main/java/me/kavishdevar/librepods/utils/NavigationExtensions.kt (2)
47-64: LGTM! Clean extension API design.Both
navigateDebouncedoverloads provide a consistent interface with boolean return values indicating whether navigation occurred, allowing callers to respond appropriately to debounced actions.
70-76: LGTM! Safe back navigation pattern.The
popBackStackSafelyextension prevents crashes by checking for a previous destination before popping, providing a defensive pattern for back navigation throughout the app.android/app/src/main/java/me/kavishdevar/librepods/MainActivity.kt (2)
94-96: LGTM! Status bar padding and safe navigation correctly implemented.The back button now:
- Respects status bar insets for proper positioning
- Uses
popBackStackSafely()to prevent crashes when the back stack is emptyBoth changes improve the user experience and app stability.
Also applies to: 112-112, 431-445
560-560: LGTM! Permission screen fully localized.All permission screen text (titles, descriptions, button labels) now uses
stringResource, ensuring a consistent localized experience for users.Also applies to: 574-575, 588-589, 600-601, 612-613, 624-625, 646-646, 677-677, 709-709
| private object NavigationDebounce { | ||
| private var lastNavigationTime: Long = 0 | ||
| private var lastRoute: String? = null | ||
| private const val DEBOUNCE_INTERVAL_MS = 300L | ||
|
|
||
| fun shouldNavigate(route: String): Boolean { | ||
| val currentTime = System.currentTimeMillis() | ||
| if (route == lastRoute && currentTime - lastNavigationTime < DEBOUNCE_INTERVAL_MS) { | ||
| return false | ||
| } | ||
| lastNavigationTime = currentTime | ||
| lastRoute = route | ||
| return 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.
Add synchronization to prevent race conditions.
The NavigationDebounce object uses mutable state (lastNavigationTime, lastRoute) without synchronization. In Compose, navigation can be triggered from different threads (e.g., coroutines, callbacks), potentially causing race conditions where multiple navigations occur simultaneously or debounce state is inconsistent.
🔎 Proposed fix with synchronization
private object NavigationDebounce {
+ private val lock = Any()
private var lastNavigationTime: Long = 0
private var lastRoute: String? = null
private const val DEBOUNCE_INTERVAL_MS = 300L
fun shouldNavigate(route: String): Boolean {
+ synchronized(lock) {
val currentTime = System.currentTimeMillis()
if (route == lastRoute && currentTime - lastNavigationTime < DEBOUNCE_INTERVAL_MS) {
return false
}
lastNavigationTime = currentTime
lastRoute = route
return 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.
| private object NavigationDebounce { | |
| private var lastNavigationTime: Long = 0 | |
| private var lastRoute: String? = null | |
| private const val DEBOUNCE_INTERVAL_MS = 300L | |
| fun shouldNavigate(route: String): Boolean { | |
| val currentTime = System.currentTimeMillis() | |
| if (route == lastRoute && currentTime - lastNavigationTime < DEBOUNCE_INTERVAL_MS) { | |
| return false | |
| } | |
| lastNavigationTime = currentTime | |
| lastRoute = route | |
| return true | |
| } | |
| } | |
| private object NavigationDebounce { | |
| private val lock = Any() | |
| private var lastNavigationTime: Long = 0 | |
| private var lastRoute: String? = null | |
| private const val DEBOUNCE_INTERVAL_MS = 300L | |
| fun shouldNavigate(route: String): Boolean { | |
| synchronized(lock) { | |
| val currentTime = System.currentTimeMillis() | |
| if (route == lastRoute && currentTime - lastNavigationTime < DEBOUNCE_INTERVAL_MS) { | |
| return false | |
| } | |
| lastNavigationTime = currentTime | |
| lastRoute = route | |
| return true | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
android/app/src/main/java/me/kavishdevar/librepods/utils/NavigationExtensions.kt
around lines 27 to 41, the NavigationDebounce object mutates lastNavigationTime
and lastRoute without thread-safety which can cause race conditions; make
shouldNavigate atomic by synchronizing access — either annotate the function
with @Synchronized or wrap its body in a synchronized(this) block (or use a
Mutex/atomic types) so the check and update of lastNavigationTime/lastRoute
happen as a single thread-safe operation, ensuring consistent debounce behavior
across threads.
Summary
This PR extracts hardcoded strings from Kotlin source files into
strings.xmlfor better internationalization support, and adds corresponding Chinese Simplified translations.Changes
Extract hardcoded strings to
strings.xmlfor:Onboarding.ktTroubleshootingScreen.ktAppSettingsScreen.ktHearingAidScreen.ktMainActivity.ktQuickSettingsDialogActivity.ktAirPodsService.ktAirPodsQSService.ktAdd Chinese Simplified translations in
values-zh-rCN/strings.xmlType of Change
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.