-
-
Notifications
You must be signed in to change notification settings - Fork 84
feat: Localization for english, french, chinese, japanese, spanish and russian #99
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
This commit introduces a `TextUi` sealed interface to abstract string resource handling, allowing for the use of both static `String` values and dynamic `StringResource` references. This refactoring enables more flexible and centralized text management within the UI layer. Key changes: - Created a `TextUi` sealed interface with two data classes: `Static` for raw strings and `Dynamic` for resource IDs. - Added a composable function `asString()` to resolve the `TextUi` instance into a displayable string.
This commit introduces internationalization (i18n) to the application by replacing hardcoded strings with string resources. It also adds translations for Spanish, French, Russian, Japanese, and Chinese (Simplified). Key changes: - All user-facing text in the "Installed Apps" feature is now sourced from `strings.xml` resources. - Added translation files for Spanish (`es`), French (`fr`), Russian (`ru`), Japanese (`ja`), and Chinese (`zh-rCN`). - Replaced hardcoded strings in `AppsRoot.kt` (UI) and `AppsViewModel.kt` (logic) with Compose resource accessors (`stringResource(Res.string.some_string)`). - The `TextUi.kt` sealed interface has been removed as it's no longer needed with the new resource-based approach. - Removed the `errorMessage` property from `AppsState`, as error messages are now handled directly via events with localized strings.
This commit refactors the authentication feature to use string resources, enabling localization and improving code maintainability. All hardcoded text in the authentication UI and related view models has been replaced with string resources. This includes text for states like "Waiting for authorization", "Signed in", error messages, and button labels. Additionally, translations for these new strings have been added for Spanish, French, Japanese, Russian, and Chinese.
This commit fully internationalizes the search screen by replacing hardcoded strings with Compose resources. Key changes: - Added new string resources for the search screen, including labels for filters, sorting, results count, and error messages. - Provided translations for Japanese, French, Spanish, Russian, and Chinese. - Refactored `ProgrammingLanguage` and `SortBy` enums to use `StringResource` for their display names, making them localizable. - Updated `SearchRoot.kt`, `LanguageFilterBottomSheet.kt`, and `SortByBottomSheet.kt` to use the new `stringResource` APIs. - The `SearchViewModel` now uses resource strings for error messages like "No repositories found" and "Search failed".
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds comprehensive i18n resource bundles (EN, ES, FR, JA, RU, ZH-CN) and replaces hardcoded UI strings with resource lookups across the app; refactors several enums to provide resource-backed labels; introduces a typed Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
Comment |
This commit replaces hardcoded strings on the settings screen and its related components with string resources from `composeResources` to support internationalization. Key changes: - All static text in the `SettingsRoot`, `Appearance`, `About`, `Account`, and `LogoutDialog` composables now uses the `stringResource()` function. - The `AppTheme` enum was refactored to provide theme display names via a composable `displayName()` function, which fetches the appropriate localized string. - Added new string resources for all text related to the settings screen, including section headers, item titles, dialog text, and theme names. - Provided translations for the new strings in Chinese (`zh-rCN`), Russian (`ru`), Spanish (`es`), French (`fr`), and Japanese (`ja`).
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 (4)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/presentation/components/SortByBottomSheet.kt (2)
29-29: Replace hardcoded string with resource.The "Close" text should use
stringResourcefor localization. The resource keycloseis available in strings.xml.🔎 Proposed fix
Add the required import at the top if not already present:
import githubstore.composeapp.generated.resources.closeThen update the text:
TextButton(onClick = onDismissRequest) { - Text(text = "Close") + Text(text = stringResource(Res.string.close)) }
34-34: Replace hardcoded string with resource.The "Sort by" text should use
stringResourcefor localization. The resource keysort_byis available in strings.xml.🔎 Proposed fix
Add the required import at the top if not already present:
import githubstore.composeapp.generated.resources.sort_byThen update the text:
Text( - text = "Sort by", + text = stringResource(Res.string.sort_by), style = MaterialTheme.typography.titleMedium )composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/auth/presentation/AuthenticationViewModel.kt (1)
127-139: Inconsistent localization: hardcoded clipboard label.Line 136 still uses a hardcoded clipboard label
"GitHub Code", while line 98 correctly usesgetString(Res.string.enter_code_on_github)for the same purpose. This inconsistency should be addressed for complete localization coverage.🔎 Suggested fix
private fun copyCode(start: DeviceStart) { _state.update { it.copy( loginState = AuthLoginState.DevicePrompt(start), copied = true ) } clipboardHelper.copy( - label = "GitHub Code", + label = getString(Res.string.enter_code_on_github), text = start.userCode ) }composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/domain/model/SortBy.kt (1)
15-25: Remove only the unusedgetStringimport from this file.The
getStringimport on line 8 is not used inSortBy.kt. However,displayText()is actively used inSearchRoot.ktand should be kept. BothdisplayText()andlabel()serve different purposes and are not redundant.
🧹 Nitpick comments (4)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/core/presentation/model/AppTheme.kt (1)
7-7: Remove unused import.The
logoutresource is imported but never used in this file. Only theme-related resources are referenced in thedisplayName()function.🔎 Proposed fix
-import githubstore.composeapp.generated.resources.logoutcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/settings/presentation/components/LogoutDialog.kt (1)
22-28: Unused import can be removed.The
navigate_backresource is imported on line 26 but doesn't appear to be used in this file.🔎 Suggested cleanup
import githubstore.composeapp.generated.resources.Res import githubstore.composeapp.generated.resources.close import githubstore.composeapp.generated.resources.logout import githubstore.composeapp.generated.resources.logout_confirmation -import githubstore.composeapp.generated.resources.navigate_back import githubstore.composeapp.generated.resources.warning import org.jetbrains.compose.resources.stringResourcecomposeApp/src/commonMain/composeResources/values-es/strings-es.xml (1)
1-121: LGTM! Spanish localization resource file is well-structured.The translations appear accurate, and placeholders are properly formatted for runtime substitution. The coverage is comprehensive across all UI elements.
One minor note: The filename
strings-es.xmlinvalues-es/directory is slightly redundant since the directory suffix already indicates the locale. Consider using juststrings.xmlwithin locale-specific directories for consistency with Android resource conventions (e.g.,values-es/strings.xml).composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/presentation/SearchRoot.kt (1)
271-303: Inconsistent string handling in disabled sorting feature.While this code block is disabled (
if (false)), line 282 mixesstringResourcewith the hardcodeddisplayText()method. When re-enabling this feature, update to uselabel()for consistency:🔎 Suggested fix for when feature is enabled
Text( - text = stringResource(Res.string.sort_by) + ": ${state.selectedSortBy.displayText()}", + text = stringResource(Res.string.sort_by) + ": " + stringResource(state.selectedSortBy.label()), style = MaterialTheme.typography.titleMedium,
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
composeApp/src/androidMain/res/values/strings.xmlcomposeApp/src/commonMain/composeResources/values-es/strings-es.xmlcomposeApp/src/commonMain/composeResources/values-fr/strings-fr.xmlcomposeApp/src/commonMain/composeResources/values-ja/values-ja.xmlcomposeApp/src/commonMain/composeResources/values-ru/strings-ru.xmlcomposeApp/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xmlcomposeApp/src/commonMain/composeResources/values/strings.xmlcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/core/presentation/model/AppTheme.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsRoot.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsState.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsViewModel.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/auth/presentation/AuthenticationRoot.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/auth/presentation/AuthenticationViewModel.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/domain/model/ProgrammingLanguage.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/domain/model/SortBy.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/presentation/SearchRoot.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/presentation/SearchViewModel.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/presentation/components/LanguageFilterBottomSheet.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/presentation/components/SortByBottomSheet.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/settings/presentation/SettingsRoot.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/settings/presentation/components/LogoutDialog.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/settings/presentation/components/sections/About.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/settings/presentation/components/sections/Account.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/settings/presentation/components/sections/Appearance.kt
💤 Files with no reviewable changes (1)
- composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsState.kt
🧰 Additional context used
🧬 Code graph analysis (2)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/auth/presentation/AuthenticationRoot.kt (1)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/core/presentation/components/GithubStoreButton.kt (1)
GithubStoreButton(17-52)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsRoot.kt (1)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsViewModel.kt (1)
onAction(107-146)
🪛 detekt (1.23.8)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsViewModel.kt
[warning] 338-338: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (26)
composeApp/src/androidMain/res/values/strings.xml (1)
2-2: LGTM! Branding correction applied.The capitalization change from "Github Store" to "GitHub Store" correctly reflects GitHub's official branding.
composeApp/src/commonMain/composeResources/values-fr/strings-fr.xml (1)
1-121: LGTM! Comprehensive French localization added.The French resource file is well-structured with proper placeholder formatting (%1$s, %2$d) and covers all UI features including apps, authentication, search, settings, and themes.
composeApp/src/commonMain/composeResources/values-ja/values-ja.xml (1)
1-121: LGTM! Comprehensive Japanese localization added.The Japanese resource file follows the same well-structured pattern as other locale files with proper placeholder formatting.
composeApp/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xml (1)
1-121: LGTM! Comprehensive Simplified Chinese localization added.The Simplified Chinese resource file is properly structured and consistent with other localization files.
composeApp/src/commonMain/composeResources/values-ru/strings-ru.xml (1)
1-121: LGTM! Comprehensive Russian localization added.The Russian resource file follows the established pattern with proper formatting and placeholder usage.
composeApp/src/commonMain/composeResources/values/strings.xml (1)
1-148: LGTM! Well-organized base English resource file.The base resource file is comprehensive and well-structured with clear section comments, proper placeholder formatting, and correct HTML entity escaping (&). This serves as an excellent foundation for all locale-specific translations.
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/core/presentation/model/AppTheme.kt (1)
38-48: LGTM! Excellent localization refactor.The new
displayName()composable function properly usesstringResourceto provide localized theme names. This approach correctly separates display logic from data and enables runtime locale switching.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/presentation/components/SortByBottomSheet.kt (1)
53-53: Good localization implementation for sort options.The sort option labels correctly use
stringResource(option.label())to enable localized text.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/settings/presentation/components/sections/Account.kt (1)
29-31: LGTM! Clean localization implementation.The logout string has been correctly migrated to use
stringResource, maintaining consistency with the broader localization effort across the codebase.Also applies to: 53-53
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/presentation/components/LanguageFilterBottomSheet.kt (1)
22-24: LGTM! Proper resource-based localization.Both the bottom sheet title and language labels have been correctly migrated to use
stringResource. Thelanguage.label()pattern effectively delegates display name resolution to the resource system.Also applies to: 48-48, 70-70
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/settings/presentation/components/LogoutDialog.kt (1)
48-48: LGTM! Comprehensive dialog localization.All user-facing strings in the logout dialog have been properly migrated to use
stringResource, ensuring the dialog can be displayed in multiple languages.Also applies to: 54-54, 70-70, 84-84
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/auth/presentation/AuthenticationViewModel.kt (1)
98-100: LGTM! Proper error message localization.Error messages and the primary clipboard label have been correctly migrated to use resource-based strings via
getString.Also applies to: 108-108, 113-113
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/settings/presentation/components/sections/About.kt (1)
31-35: LGTM! Complete section localization.All user-facing strings in the About section have been correctly migrated to
stringResource, maintaining consistency with the settings UI localization pattern.Also applies to: 45-45, 63-63, 77-77
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/settings/presentation/SettingsRoot.kt (1)
29-36: LGTM! Correct context-aware resource usage.The implementation properly uses
getStringin the coroutine context (line 59) andstringResourcein composable contexts (lines 123, 130), following the correct pattern for Compose resource handling.Also applies to: 59-59, 123-123, 130-130
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/presentation/SearchViewModel.kt (1)
6-8: LGTM! Proper ViewModel localization.Error messages have been correctly migrated to use
getString(appropriate for non-composable ViewModel context), with smart fallback to localized strings when specific error messages aren't available.Also applies to: 21-21, 146-146, 163-163
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/settings/presentation/components/sections/Appearance.kt (1)
36-42: LGTM! Comprehensive appearance section localization.All user-facing strings have been properly migrated to
stringResource. The refactoring oftheme.displayName(property) totheme.displayName()(composable function) is a good architectural choice that enables resource-based theme names. The formatted string usage in the content description (lines 128-131) correctly combines the localized template with the dynamic theme name.Also applies to: 55-55, 75-75, 128-131, 139-139, 172-172, 180-180
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/domain/model/ProgrammingLanguage.kt (1)
21-54: LGTM! Clean refactor to resource-based labels.The enum properly separates the API query values from UI display concerns. The exhaustive
whenexpression ensures all enum values are mapped, and adding a new language will produce a compile-time error if the mapping is forgotten.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsViewModel.kt (2)
68-105: LGTM! Error handling updated for resource-based messaging.The load failure path correctly sets
isLoading = falseand logs the error. The removal oferrorMessagefrom state aligns with the event-driven messaging pattern used elsewhere.
148-166: LGTM! Resource-based error messages for app operations.The
openAppfunction correctly usesgetStringwith formatted resources for error messages. The nullable handling witharrayOf(app.appName)is appropriate here sinceappNameis non-null.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/auth/presentation/AuthenticationRoot.kt (2)
127-148: LGTM! Authentication states properly localized.The pending, logged-in, and error states now use resource-based strings. The parameterized error message correctly passes
authState.messageto the format string.
259-326: LGTM! Logged-out state UI fully localized.The
StateLoggedOutcomposable properly usesstringResourcefor all user-facing text including the marketing copy and button labels.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/search/presentation/SearchRoot.kt (2)
202-253: LGTM! Language filter UI properly localized.The language label, selected language display via
label(), and filter chip interactions correctly use resource-based strings. SettingcontentDescription = nullfor the decorative close icon is appropriate.
381-462: LGTM! Search topbar and input field localized.Navigation back button, title, and search placeholder all properly use
stringResourcefor localization.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsRoot.kt (3)
65-84: LGTM! Comprehensive resource imports for localization.All required string resources are imported for the various UI states and actions in this screen.
419-509: LGTM! Update state displays properly localized.All update states (Downloading, Installing, Checking, Success, Error) use resource-based strings with appropriate parameterization for progress percentages and error messages.
295-348: LGTM! Update progress card properly localized.The
UpdateAllProgressCardcorrectly uses parameterized resources for displaying progress counts and current app name.
| } catch (e: CancellationException) { | ||
| Logger.d { "Update all cancelled" } | ||
| } catch (e: Exception) { | ||
| Logger.e { "Update all failed: ${e.message}" } | ||
| _events.send(AppsEvent.ShowError("Update all failed: ${e.message}")) | ||
| _events.send(AppsEvent.ShowError(getString(Res.string.update_all_failed, arrayOf(e.message)))) |
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.
Potential null value in string format argument.
The static analysis hint about the swallowed CancellationException is a false positive—logging and returning early is the correct pattern for coroutine cancellation.
However, on line 342, e.message can be null, and passing it directly to arrayOf() may result in displaying "null" in the UI if the exception has no message. Consider providing a fallback:
🔎 Proposed fix
- _events.send(AppsEvent.ShowError(getString(Res.string.update_all_failed, arrayOf(e.message))))
+ _events.send(AppsEvent.ShowError(getString(Res.string.update_all_failed, arrayOf(e.message ?: ""))))🧰 Tools
🪛 detekt (1.23.8)
[warning] 338-338: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
In
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsViewModel.kt
around lines 338 to 342, the exception message e.message can be null and passing
it directly to arrayOf(...) may display "null" in the UI; change the code to
provide a safe fallback (e.g., use e.message ?:
getString(Res.string.unknown_error) or e.message ?: "") when building the format
arguments and use that fallback both in the log and in the _events.send call so
the UI shows a sensible message instead of "null".
This commit refactors the Details screen and related components to use Compose multiplatform string resources instead of hardcoded strings. This enhances internationalization (i18n) by providing translations for Chinese (zh-rCN), Russian (ru), Spanish (es), French (fr), and Japanese (ja). Additionally, the logic for installation and download logs has been improved by introducing a `LogResult` sealed class. This makes log messages type-safe and also ties them into the new resource string system for easier translation. Key changes: - Replaced hardcoded text in the Details screen UI (`Header`, `About`, `Stats`, `Owner`, `WhatsNew`, `SmartInstallButton`, `ErrorState`, etc.) with `stringResource()` calls. - Added extensive new string resources for the details screen, including install states, download progress, error messages, and section titles. - Created `values-zh-rCN`, `values-ru`, `values-es`, `values-fr`, and `values-ja` resource directories with corresponding translations. - Introduced `LogResult` sealed class to represent different installation outcomes (e.g., `Downloaded`, `Installed`, `Error`), replacing simple `String` results. - Updated `DetailsViewModel` to use the new `LogResult` types when appending logs. - Created a `LogResult.asText()` composable function to provide localized string representations of log statuses. - Removed the now-unused `Badge.kt` composable. - Minor fix in `DetailsRepositoryImpl` to always search for an English README if the detected one isn't English, regardless of the primary language.
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: 2
🧹 Nitpick comments (4)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/AppHeader.kt (1)
43-43: Unused import.The
verifyingresource is imported but not used in this file.🔎 Proposed fix
import githubstore.composeapp.generated.resources.no_description import githubstore.composeapp.generated.resources.update_available -import githubstore.composeapp.generated.resources.verifying import org.jetbrains.compose.resources.stringResourcecomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Owner.kt (1)
33-33: Unused import.
error_loading_detailsis imported but not used in this file.🔎 Proposed fix
import githubstore.composeapp.generated.resources.author -import githubstore.composeapp.generated.resources.error_loading_details import githubstore.composeapp.generated.resources.ic_githubcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/WhatsNew.kt (1)
24-24: Unused import.
error_loading_detailsis imported but not used in this file.🔎 Proposed fix
import githubstore.composeapp.generated.resources.Res -import githubstore.composeapp.generated.resources.error_loading_details import githubstore.composeapp.generated.resources.no_release_notescomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Logs.kt (1)
14-14: Unused import.
error_loading_detailsis imported but not used in this file.🔎 Proposed fix
import githubstore.composeapp.generated.resources.Res -import githubstore.composeapp.generated.resources.error_loading_details import githubstore.composeapp.generated.resources.install_logs
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
composeApp/src/commonMain/composeResources/values-es/strings-es.xmlcomposeApp/src/commonMain/composeResources/values-fr/strings-fr.xmlcomposeApp/src/commonMain/composeResources/values-ja/values-ja.xmlcomposeApp/src/commonMain/composeResources/values-ru/strings-ru.xmlcomposeApp/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xmlcomposeApp/src/commonMain/composeResources/values/strings.xmlcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/data/repository/DetailsRepositoryImpl.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/DetailsRoot.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/DetailsState.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/DetailsViewModel.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/AppHeader.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/Badge.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/SmartInstallButton.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/About.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Header.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Logs.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Owner.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Stats.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/WhatsNew.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/states/ErrorState.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/model/LogResult.kt
💤 Files with no reviewable changes (1)
- composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/Badge.kt
✅ Files skipped from review due to trivial changes (1)
- composeApp/src/commonMain/composeResources/values-fr/strings-fr.xml
🚧 Files skipped from review as they are similar to previous changes (4)
- composeApp/src/commonMain/composeResources/values-ru/strings-ru.xml
- composeApp/src/commonMain/composeResources/values-es/strings-es.xml
- composeApp/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xml
- composeApp/src/commonMain/composeResources/values-ja/values-ja.xml
🧰 Additional context used
🧬 Code graph analysis (1)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Stats.kt (1)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/StatItem.kt (1)
StatItem(14-45)
🔇 Additional comments (20)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Header.kt (2)
21-29: LGTM! Clean resource imports for localization.The imports correctly bring in the generated resource object and string resolution function following Compose Multiplatform conventions. This properly supports the i18n refactor described in the PR objectives.
82-82: LGTM! Proper string resource integration.The hardcoded strings have been correctly replaced with
stringResource()calls referencing appropriate resource keys. The implementation is clean and consistent across both dropdown menu items.Also applies to: 87-87, 112-112, 117-117
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/data/repository/DetailsRepositoryImpl.kt (1)
187-190: LGTM - Removed redundant condition.The
&& detectedLang != "en"check was redundant since the prior condition at lines 182-185 already returns early whendetectedLang == primaryLang. If execution reaches line 187 withprimaryLang == "en", we knowdetectedLangcannot be"en". This simplification improves readability.composeApp/src/commonMain/composeResources/values/strings.xml (1)
1-221: LGTM - Comprehensive localization resource file.The string resources are well-organized with clear categories, properly typed format specifiers (
%1$dfor integers,%1$sfor strings), and correctly escaped HTML entities (&). This establishes a solid foundation for internationalization.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/SmartInstallButton.kt (1)
35-50: LGTM - Consistent localization implementation.All hardcoded strings have been properly replaced with
stringResourcelookups, including button text, download stage messages, and accessibility descriptions. Format specifiers are used correctly for dynamic values.Also applies to: 93-105, 157-185, 273-273, 319-319, 344-344
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/DetailsRoot.kt (1)
36-43: LGTM - Accessibility strings localized.Icon
contentDescriptionattributes now use localized strings, improving accessibility for international users.Also applies to: 132-132, 147-147
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Stats.kt (1)
11-17: LGTM - Stat labels localized.Repository statistics labels (Forks, Stars, Issues) now pull from string resources, enabling translation across all supported languages.
Also applies to: 35-35, 43-43, 51-51
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/DetailsState.kt (1)
10-18: LGTM - Type-safe result representation.Changing
resultfromStringtoLogResultimproves type safety and enables compile-time verification of log result states. This pairs well with the resource-based localization strategy.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/About.kt (1)
21-23: LGTM - Section heading localized.The "About this app" heading now uses a localized string resource, consistent with the broader i18n effort.
Also applies to: 51-51
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/states/ErrorState.kt (1)
12-16: LGTM - Error state localized.Error messages and action buttons now display in the user's preferred language, improving the error handling experience for international users.
Also applies to: 31-31, 44-44
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/AppHeader.kt (1)
167-173: LGTM!Good improvements: the null-safe
letwrapper fortagNamedisplay and the resource-based strings for install status badges are correctly implemented.Also applies to: 219-223
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Owner.kt (1)
56-56: LGTM!The localization changes for the "Author" title and "Profile" link text are correctly implemented.
Also applies to: 137-137
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/WhatsNew.kt (1)
44-44: LGTM!The localization for the "What's New" section title and "No release notes" fallback is correctly implemented.
Also applies to: 93-93
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Logs.kt (1)
40-52: LGTM!Good improvements: using the typed
LogResultwithis LogResult.Errorcheck is cleaner and type-safe compared to string prefix matching. TheasText()composable extension is correctly invoked within the composition context.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/model/LogResult.kt (2)
18-35: LGTM!Well-designed sealed class with clear separation of log result types. Using
data objectfor singleton states anddata classfor those carrying payloads is idiomatic Kotlin.
38-71: LGTM!The
asText()extension correctly maps each variant to its localized string. The nullable message handling forError(lines 62-65) is well implemented. Note thatInforeturns the raw message directly, which is appropriate for dynamic/programmatic messages that don't need localization.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/DetailsViewModel.kt (4)
873-878: LGTM!The signature change from
result: Stringtoresult: LogResultis clean. The function correctly stores the typed result in the state, allowing the UI layer to render it with proper localization viaasText().
631-638: LGTM!Good use of conditional
LogResultassignment for distinguishing between fresh installs and updates. The ternary-style expression is clear and correctly appliesUpdateStarted/DownloadStartedandUpdated/Installedvariants.Also applies to: 693-700
526-531: LGTM!All
appendLogcall sites have been consistently updated to useLogResultvariants with named arguments for clarity. The variant choices are semantically appropriate for each scenario.Also applies to: 552-557, 573-578, 590-595, 660-665, 710-715, 830-835, 849-854, 862-867
684-686:getStringis correctly imported from Compose Resources.The import on line 24 confirms
getStringcomes fromorg.jetbrains.compose.resources. Using this suspend function withinviewModelScope.launchis appropriate and correct.
|
|
||
| Text( | ||
| text = "by ${author?.login}", | ||
| text = stringResource(Res.string.by_author, author?.login.toString()), |
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.
Avoid displaying literal "null" when author is missing.
Using author?.login.toString() will produce the string "null" when author or login is null. Consider using the Elvis operator to provide a meaningful fallback:
🔎 Proposed fix
- text = stringResource(Res.string.by_author, author?.login.toString()),
+ text = stringResource(Res.string.by_author, author?.login ?: ""),📝 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.
| text = stringResource(Res.string.by_author, author?.login.toString()), | |
| text = stringResource(Res.string.by_author, author?.login ?: ""), |
🤖 Prompt for AI Agents
In
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/AppHeader.kt
at line 148, the code uses author?.login.toString() which will render the
literal "null" when author or login is null; replace that expression with a safe
non-null fallback (e.g., use the Elvis operator like author?.login ?:
"<unknown>" or an empty string) so the stringResource call receives a meaningful
value and does not display "null".
| Icon( | ||
| imageVector = Icons.AutoMirrored.Filled.KeyboardArrowRight, | ||
| contentDescription = "Navigate Back", | ||
| contentDescription = stringResource(Res.string.navigate_back), | ||
| modifier = Modifier.size(24.dp) | ||
| ) |
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.
Semantic mismatch: forward arrow with "navigate back" description.
The icon uses Icons.AutoMirrored.Filled.KeyboardArrowRight (a forward arrow) but the content description references navigate_back. This is misleading for accessibility users. Consider using a more appropriate resource key like navigate_forward or open_author.
🤖 Prompt for AI Agents
In
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/details/presentation/components/sections/Owner.kt
around lines 157 to 161, the icon shown is a forward arrow (KeyboardArrowRight)
but its contentDescription uses the string resource Res.string.navigate_back;
update the contentDescription to a semantically correct string (e.g.,
Res.string.navigate_forward or Res.string.open_author) that matches the visual
arrow, or change the icon to a back arrow if the action is “navigate back,”
ensuring the contentDescription and icon match.
This commit replaces hardcoded UI strings with internationalized string resources (`Res.string.*`). This is a significant refactoring to support localization and improve code maintainability. Key changes: - Added new string resources for the home screen, rate limit dialog, and date formatting. - Created translations for Chinese (Simplified), Russian, Spanish, French, and Japanese. - Updated composables (`HomeRoot`, `RateLimitDialog`, `RepositoryCard`, etc.) to use `stringResource()` instead of hardcoded text. - Refactored `HomeCategory` enum to provide a composable `displayText()` function for retrieving localized category names. - Updated `formatUpdatedAt` to be a `@Composable` function that uses string resources for relative time formatting (e.g., "updated yesterday"). - Removed the unused `TrendingPeriod` enum as it was no longer needed.
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/core/presentation/utils/UpdatedAtFormmater.kt (1)
32-35: Hardcoded string breaks localization.The
elsebranch uses a hardcoded English string"updated on $date"while the rest of the function correctly uses localized resources. Theupdated_on_dateresource key exists instrings.xmland should be used here.🔎 Proposed fix
else -> { val date = updated.toLocalDateTime(TimeZone.currentSystemDefault()).date - "updated on $date" + stringResource(Res.string.updated_on_date, date.toString()) }You'll also need to add the import:
import githubstore.composeapp.generated.resources.updated_on_date
🧹 Nitpick comments (3)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/app_state/components/RateLimitDialog.kt (1)
137-138: Consider adding mock data to the preview.The preview function passes
nullforrateLimitInfo, which will cause the dialog to display "0" for limit and reset time values. Consider using mockRateLimitInfodata to make the preview more visually representative.Example mock data for preview
rateLimitInfo = RateLimitInfo( limit = 60, remaining = 0, reset = System.currentTimeMillis() + 15 * 60 * 1000 // 15 minutes from now ),composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/core/presentation/components/RepositoryCard.kt (1)
36-36: Unused import.
updated_yesterdayis imported but not used in this file. Consider removing it.🔎 Proposed fix
import githubstore.composeapp.generated.resources.installed import githubstore.composeapp.generated.resources.update_available -import githubstore.composeapp.generated.resources.updated_yesterday import org.jetbrains.compose.resources.stringResourcecomposeApp/src/commonMain/composeResources/values/strings.xml (1)
232-234: Consider using plural resources for better localization.The
hour(s)andday(s)patterns don't translate well to languages with complex plural rules (e.g., Russian has three plural forms). Android/Compose resources support<plurals>for proper pluralization. This is acceptable for now but consider improving in a follow-up.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
composeApp/src/commonMain/composeResources/values-es/strings-es.xmlcomposeApp/src/commonMain/composeResources/values-fr/strings-fr.xmlcomposeApp/src/commonMain/composeResources/values-ja/values-ja.xmlcomposeApp/src/commonMain/composeResources/values-ru/strings-ru.xmlcomposeApp/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xmlcomposeApp/src/commonMain/composeResources/values/strings.xmlcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/app_state/components/RateLimitDialog.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/core/presentation/components/RepositoryCard.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/core/presentation/utils/UpdatedAtFormmater.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/data/repository/HomeRepositoryImpl.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/domain/model/TrendingPeriod.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/domain/repository/HomeRepository.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/presentation/HomeRoot.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/presentation/HomeViewModel.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/presentation/components/HomeFilterChips.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/presentation/model/HomeCategory.kt
💤 Files with no reviewable changes (3)
- composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/data/repository/HomeRepositoryImpl.kt
- composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/domain/repository/HomeRepository.kt
- composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/domain/model/TrendingPeriod.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- composeApp/src/commonMain/composeResources/values-fr/strings-fr.xml
- composeApp/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xml
- composeApp/src/commonMain/composeResources/values-ja/values-ja.xml
- composeApp/src/commonMain/composeResources/values-es/strings-es.xml
🔇 Additional comments (7)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/app_state/components/RateLimitDialog.kt (2)
20-29: Localization imports look good.All necessary resource imports and the
stringResourcefunction are correctly added to support the localization refactor.
56-124: String resource replacements are correctly implemented.The hardcoded strings have been properly replaced with
stringResource()calls throughout the dialog. Format parameters forrate_limit_used_all,rate_limit_used_all_free, andrate_limit_resets_in_minutesare correctly passed with safe null fallbacks.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/presentation/components/HomeFilterChips.kt (1)
26-26: LGTM!The change from property access to function call correctly aligns with the refactored
HomeCategory.displayText()composable function for localized text.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/core/presentation/components/RepositoryCard.kt (1)
163-167: Localization changes look good.The button label and status badge texts are correctly migrated to use
stringResourcefor localization support.Also applies to: 195-199
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/presentation/HomeViewModel.kt (1)
147-149: LGTM!The error messages are correctly migrated to use
getString()(the suspend variant for non-composable contexts) within the coroutine scope. The fallback pattern on line 171 appropriately uses the localized string when the exception message is null.Also applies to: 170-172
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/presentation/HomeRoot.kt (1)
51-62: LGTM!Comprehensive localization migration for all UI text elements including the app bar title, loading states, retry button, end-of-list message, and accessibility content descriptions. The changes are consistent and well-structured.
Also applies to: 153-153, 209-209, 235-235, 288-288, 300-300, 338-338, 359-359, 388-388
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/home/presentation/model/HomeCategory.kt (1)
10-22: LGTM!Clean refactor from a value-bearing enum to a composable function approach for localized display text. The
whenexpression is exhaustive, ensuring all enum values are handled. This pattern is consistent with the broader localization strategy in this PR.
This commit corrects a formatting issue in internationalization string resources across multiple languages (English, Russian, Spanish, French, and Japanese). The `percent` string resource was missing a second percentage sign, which is required to properly escape it in Android XML format strings. Additionally, new strings for the app details screen have been added to the Russian localization file (`strings-ru.xml`): - `no_release_notes` - `open_in_browser` - `show_install_options`
This commit introduces a
TextUisealed interface to abstract string resource handling, allowing for the use of both staticStringvalues and dynamicStringResourcereferences.This refactoring enables more flexible and centralized text management within the UI layer.
Key changes:
TextUisealed interface with two data classes:Staticfor raw strings andDynamicfor resource IDs.asString()to resolve theTextUiinstance into a displayable string.Summary by CodeRabbit
New Features
Bug Fixes
UX Improvements
✏️ Tip: You can customize this high-level summary in your review settings.