MM-541 savings account details doesnt render properly the values in the button table#3082
Conversation
📝 WalkthroughWalkthroughA Compose UI layout modifier was updated on the SavingsAccountDetailsScreen, changing a fixed height constraint to a minimum height constraint on the MifosLabelValueCard component within AccountDetailsGrid. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested reviewers
Poem
🚥 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. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/confirmDetails/ConfirmDetailsViewModel.kt (1)
287-292: Add a localized fallback for empty/technical server messages.
status.messagecan be blank or too technical, which can lead to an empty/non-localized subtitle. Consider falling back to the existing tip when the message is blank to preserve UX consistency.💡 Suggested fix
+import mifos_mobile.feature.loan_application.generated.resources.feature_apply_loan_status_failure_tip ... - subtitle = status.message, + subtitle = status.message.takeIf { it.isNotBlank() } + ?: getString(Res.string.feature_apply_loan_status_failure_tip),feature/third-party-transfer/src/commonMain/kotlin/org/mifos/mobile/feature/third/party/transfer/thirdPartyTransfer/TptViewModel.kt (1)
8-8: Potential typo in license URL.Same issue as in the strings file - the URL references
mobile-mobilebut should likely bemifos-mobile.- * See https://github.com/openMF/mobile-mobile/blob/master/LICENSE.md + * See https://github.com/openMF/mifos-mobile/blob/master/LICENSE.mdcore/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/component/MifosTextField.kt (1)
157-166: InconsistentsupportingTextbehavior compared toMifosOutlinedTextField.
MifosOutlinedTextField(lines 98-107) conditionally renderssupportingTextonly whenerrorTextis non-null, butMifosTextFieldalways renders it with an empty string fallback. This could cause layout differences between the two components.🔧 Suggested fix for consistency
- supportingText = config.errorText.let { - { - Text( - modifier = Modifier.testTag("errorTag"), - text = it ?: "", - style = MifosTypography.bodySmall, - color = KptTheme.colorScheme.error, - ) - } - }, + supportingText = config.errorText?.let { + { + Text( + modifier = Modifier.testTag("errorTag"), + text = it, + style = MifosTypography.bodySmall, + color = KptTheme.colorScheme.error, + ) + } + },
🤖 Fix all issues with AI agents
In `@cmp-android/src/main/res/values-es/strings.xml`:
- Line 488: The string resource key "seleted_qr_image" is misspelled and must be
renamed to "selected_qr_image" everywhere; update the base strings.xml entry and
all localized variants that currently declare the "seleted_qr_image" key to use
"selected_qr_image" instead, preserving each locale's translated value ("Imagen
QR seleccionada" in Spanish) and ensuring you also update any references in code
or XML that reference the old key (search for usages of seleted_qr_image and
replace with selected_qr_image).
In `@feature/settings/src/commonMain/composeResources/values-te/strings.xml`:
- Line 101: The resource key feature_settings_appinfo_mifos_electonic_banking
contains a typo ("electonic") and must be renamed to
feature_settings_appinfo_mifos_electronic_banking across all locales and code;
update the key in every locale file (all 18 string XMLs) and then update all
references/imports/usages in Kotlin (e.g., where AppInfoScreen.kt imports or
accesses the resource) to the new key so builds and lookups remain consistent;
ensure you run a project-wide search/replace for
feature_settings_appinfo_mifos_electonic_banking and update any generated
R/Resource usages or string resource accessors accordingly.
In
`@feature/share-application/src/commonMain/kotlin/org/mifos/mobile/feature/share/application/fillApplication/FillApplicationViewModel.kt`:
- Around line 655-658: The subtitle currently concatenates response.message
directly, which can render the literal "null" (because DataState.Error.message
is exception.message.toString()) and uses a hardcoded ", " separator; update the
assignment in FillApplicationViewModel.kt to use a localized format string via
getString (e.g., Res.string.feature_apply_share_status_failure_with_tip) that
accepts the error message and product name, and pass response.message.takeIf {
it != "null" } ?: "" (or similar null/placeholder-safe value) and
state.shareProductName so translators control punctuation/order and the literal
"null" is never shown.
♻️ Duplicate comments (1)
core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/MifosHiddenTextRow.kt (1)
95-100: Duplicate of the theme-provider check above.
🟡 Minor comments (24)
feature/accounts/src/commonMain/composeResources/values-es/strings.xml-56-58 (1)
56-58: Fix typo in Spanish label ("Icon de Filtro").
Likely intended “Icono de Filtro”.🐛 Proposed fix
- <string name="feature_transaction_filter_icon_description">Icon de Filtro</string> + <string name="feature_transaction_filter_icon_description">Icono de Filtro</string>cmp-navigation/src/commonMain/composeResources/values-es/strings.xml-9-9 (1)
9-9: Fix license header repository link.
The URL points toopenMF/mobile-mobile, which looks like a typo for this repo (openMF/mifos-mobile). Please update to avoid a broken/stale link.✅ Suggested fix
- See https://github.com/openMF/mobile-mobile/blob/master/LICENSE.md + See https://github.com/openMF/mifos-mobile/blob/master/LICENSE.mdfeature/client-charge/src/commonMain/composeResources/values-es/strings.xml-35-38 (1)
35-38: Align “due” vs “outstanding” translations to avoid ambiguity (Line 35–38).Both
dueandoutstandingare translated as “Pendiente,” which makes two distinct labels indistinguishable. Consider aligningduewith the existingamount_due(“Vencido”) or using another distinct term.💡 Suggested adjustment
- <string name="due">Pendiente</string> + <string name="due">Vencido</string>feature/home/src/commonMain/composeResources/values-es/strings.xml-48-49 (1)
48-49: Standardize “Ahorro” vs “Ahorros” terminology.“Cuentas de Ahorros” conflicts with “Cuenta de Ahorro” below. Consider aligning the plural to “Cuentas de Ahorro” (and review related strings if you want full consistency).
💡 Suggested tweak
- <string name="feature_home_saving_accounts">Cuentas de Ahorros</string> + <string name="feature_home_saving_accounts">Cuentas de Ahorro</string>Also applies to: 57-58
feature/home/src/commonMain/composeResources/values-es/strings.xml-67-67 (1)
67-67: Fix casing in Spanish sentence.“Por Favor” should be “Por favor”.
💡 Suggested tweak
- <string name="feature_server_error">Ocurrió un error. Por Favor intenta más tarde.</string> + <string name="feature_server_error">Ocurrió un error. Por favor intenta más tarde.</string>feature/third-party-transfer/src/commonMain/composeResources/values-es/strings.xml-9-9 (1)
9-9: Potential typo in license URL.The URL references
mobile-mobilebut should likely bemifos-mobileto match the repository name.- See https://github.com/openMF/mobile-mobile/blob/master/LICENSE.md + See https://github.com/openMF/mifos-mobile/blob/master/LICENSE.mdfeature/savings-account/src/commonMain/composeResources/values-es/strings.xml-78-78 (1)
78-78: Incorrect translation: "loan products" should be "savings products".The string name indicates this is for savings products (
feature_savings_savings_product_empty), but the Spanish text says "productos de préstamo" (loan products). This should refer to savings products instead.Proposed fix
- <string name="feature_savings_savings_product_empty">Actualmente los productos de préstamo no están disponibles</string> + <string name="feature_savings_savings_product_empty">Actualmente los productos de ahorro no están disponibles</string>feature/savings-account/src/commonMain/composeResources/values-es/strings.xml-91-91 (1)
91-91: Inconsistent translation: withdraw action says "update".The string name
feature_savings_withdraw_request_withdrawsuggests a withdrawal action, but the translation "Solicitar actualización" means "Request update". This appears to be a copy-paste error from the update workflow. Consider using "Solicitar retiro" (Request withdrawal) instead.Proposed fix
- <string name="feature_savings_withdraw_request_withdraw">Solicitar actualización</string> + <string name="feature_savings_withdraw_request_withdraw">Solicitar retiro</string>feature/savings-account/src/commonMain/composeResources/values-es/strings.xml-9-9 (1)
9-9: Typo in repository URL.The URL references
mobile-mobilebut should likely bemifos-mobileto match the actual repository name.Proposed fix
- See https://github.com/openMF/mobile-mobile/blob/master/LICENSE.md + See https://github.com/openMF/mifos-mobile/blob/master/LICENSE.mdfeature/transfer-process/src/commonMain/composeResources/values-es/strings.xml-9-9 (1)
9-9: Typo in license URL.The URL contains
mobile-mobilewhich should likely bemifos-mobileto match the actual repository name.- See https://github.com/openMF/mobile-mobile/blob/master/LICENSE.md + See https://github.com/openMF/mifos-mobile/blob/master/LICENSE.mdfeature/transfer-process/src/commonMain/composeResources/values-es/strings.xml-13-13 (1)
13-13: Remove the unusedtransferred_successfullystring key.
transferred_successfullyis defined but never used in this module—onlytransfer_successful(line 27) is imported and used in the code. Remove line 13 to eliminate dead code and maintenance burden.feature/loan-account/src/commonMain/composeResources/values-es/strings.xml-9-9 (1)
9-9: Typo in license header URL:mobile-mobileshould bemifos-mobile.The repository name in the license URL appears incorrect based on the PR's repository.
Proposed fix
- See https://github.com/openMF/mobile-mobile/blob/master/LICENSE.md + See https://github.com/openMF/mifos-mobile/blob/master/LICENSE.mdfeature/settings/src/commonMain/composeResources/values-ur/strings.xml-161-162 (1)
161-162: Trim leading space in Urdu “Contact Us” string.The value starts with a leading space, which can misalign the rendered text.
🔧 Proposed fix
- <string name="feature_settings_faq_contact_us"> ہم سے رابطہ کریں</string> + <string name="feature_settings_faq_contact_us">ہم سے رابطہ کریں</string>feature/settings/src/commonMain/composeResources/values/strings.xml-89-90 (1)
89-90: Remove the trailing space in the logout tip.The current value ends with an extra space, which can render awkwardly in UI and complicate string comparisons.
🔧 Proposed fix
- <string name="feature_settings_action_logout_tip">Are You Sure You Want To Logout? </string> + <string name="feature_settings_action_logout_tip">Are You Sure You Want To Logout?</string>feature/settings/src/commonMain/composeResources/values-es/strings.xml-85-96 (1)
85-96: Add missing Spanish translations for logout description/action.
feature_settings_logout_descriptionandfeature_settings_logout_actionare used by the logout dialog but aren’t defined here, so Spanish users will see English text in that section.🌍 Suggested additions (please validate with localization)
<string name="feature_settings_action_logout">Salir</string> <string name="feature_settings_action_logout_tip">Cerrar sesión en todos los dispositivos</string> + <string name="feature_settings_logout_description">Cerrar sesión finalizará tu sesión actual y todas las sesiones activas por motivos de seguridad. Tendrás que iniciar sesión de nuevo para acceder a tu cuenta.</string> + <string name="feature_settings_logout_action">Volver al inicio</string>cmp-android/src/main/res/values-es/strings.xml-533-552 (1)
533-552:translatable="false"on translated language names is contradictory.The
languagesstring-array hastranslatable="false"but contains Spanish translations like "Inglés", "Español", "Francés". If language display names should be localized (showing "English" in English, "Inglés" in Spanish), removetranslatable="false". If they should remain constant, use consistent names across all locales.feature/auth/src/commonMain/composeResources/values-es/strings.xml-51-56 (1)
51-56: Minor Spanish accent marks missing.Several words are missing accent marks which affects proper Spanish orthography:
- Line 51:
invalido→inválido- Line 53:
ID invalido de correo→Correo electrónico inválido(also awkward phrasing)- Line 56:
digitos→dígitosfeature/qr/src/commonMain/composeResources/values-es/strings.xml-9-9 (1)
9-9: Same license URL typo as auth file.The URL references
mobile-mobileinstead ofmifos-mobile.Suggested fix
- See https://github.com/openMF/mobile-mobile/blob/master/LICENSE.md + See https://github.com/openMF/mifos-mobile/blob/master/LICENSE.mdfeature/auth/src/commonMain/composeResources/values-es/strings.xml-81-81 (1)
81-81: Inconsistent accent on "Electrónico".Line 26 uses "Correo Electrónico" (correct), but line 81 uses "Correo Electronico" (missing accent). Maintain consistency.
Suggested fix
- <string name="feature_signup_email_label">Correo Electronico</string> + <string name="feature_signup_email_label">Correo Electrónico</string>feature/auth/src/commonMain/composeResources/values-es/strings.xml-9-9 (1)
9-9: Typo in license URL.The URL references
mobile-mobileinstead ofmifos-mobile.Suggested fix
- See https://github.com/openMF/mobile-mobile/blob/master/LICENSE.md + See https://github.com/openMF/mifos-mobile/blob/master/LICENSE.mdcmp-android/src/main/res/values-es/strings.xml-12-15 (1)
12-15: Contradictorytranslatable="false"on translated strings.These strings have
translatable="false"but contain Spanish translations. If a string should not be translated, it should remain in the base language (English) and not appear in locale-specific files, or the attribute should be removed.Suggested fix: Remove the attribute to allow translation
- <string name="app_name" translatable="false">Mifos Móvil</string> - <string name="feature_about_app_name" translatable="false">Mifos Móvil</string> + <string name="app_name">Mifos Móvil</string> + <string name="feature_about_app_name">Mifos Móvil</string> <string name="login">Ingresar</string> - <string name="toast_welcome" translatable="false">Bienvenido %1$s</string> + <string name="toast_welcome">Bienvenido %1$s</string>core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/MFStepProcess.kt-49-50 (1)
49-50: Stale or misleading TODO comment.The comment says "use DesignToken, currently it throws error," yet DesignToken is actively used in this function (lines 57, 77, 90). Either remove this TODO if it's no longer relevant, or clarify which specific token usage causes the error.
🧹 Suggested fix
- // TODO use DesignToken, currently it throws error var barHeight by remember { mutableStateOf(0.dp) }core/ui/src/commonMain/composeResources/values-es/strings.xml-16-44 (1)
16-44: Fix minor Spanish typos/formatting for consistency.
These are small UX text issues (accent, capitalization, trailing space).✏️ Proposed fix
- <string name="something_went_wrong">Ocurrio un error</string> + <string name="something_went_wrong">Ocurrió un error</string> @@ - <string name="password_strength_level_1">débil</string> + <string name="password_strength_level_1">Débil</string> @@ - <string name="minimum_characters_requirement">Mínimo de caracteres: %1$d </string> + <string name="minimum_characters_requirement">Mínimo de caracteres: %1$d</string>core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/PasswordStrengthIndicator.kt-208-215 (1)
208-215: Align GOOD/VERY_STRONG colors with the main indicator.
InPasswordStrengthIndicator, GOOD uses primary and VERY_STRONG uses magenta, but this mapping is swapped here. If that’s unintentional, align them or centralize the mapping to prevent drift.💡 Possible alignment (match PasswordStrengthIndicator)
- PasswordStrengthState.GOOD -> Color.Magenta - PasswordStrengthState.STRONG -> strongColor - PasswordStrengthState.VERY_STRONG -> KptTheme.colorScheme.primary + PasswordStrengthState.GOOD -> KptTheme.colorScheme.primary + PasswordStrengthState.STRONG -> strongColor + PasswordStrengthState.VERY_STRONG -> Color.Magenta
🧹 Nitpick comments (24)
feature/passcode/src/commonMain/composeResources/values-es/strings.xml (1)
14-25: Unify formal vs. informal tone across Spanish strings.These strings mix usted (“Introduzca”) with tú (“Ingresa”, “inténtalo”), which reads inconsistent in UX. Consider standardizing to one register across the file.
feature/accounts/src/commonMain/composeResources/values-es/strings.xml (1)
48-52: Consider plural‑safe wording for filter count.
If%1$sis a number, “Seleccionada” may be grammatically off for plural; consider neutral phrasing or plural resources.feature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/fillApplication/FillApplicationViewModel.kt (1)
583-598: Trim server message and broaden punctuation check before appending “.”
Ifresponse.messagehas trailing whitespace or ends with “?”/“…” the current logic can yield awkward punctuation/spacing. Consider trimming and checking a wider punctuation set.♻️ Suggested tweak
- subtitle = buildString { - val serverMessage = response.message.takeIf { it.isNotBlank() } + subtitle = buildString { + val serverMessage = response.message.trim().takeIf { it.isNotBlank() } if (serverMessage != null) { - append(serverMessage) - if (!serverMessage.endsWith(".") && !serverMessage.endsWith("!")) { + append(serverMessage) + if (serverMessage.last() !in listOf('.', '!', '?')) { append(".") } append(" ") } append( getString( Res.string.feature_apply_savings_status_failure_tip, state.fieldOfficerName, ), ) },feature/third-party-transfer/src/commonMain/kotlin/org/mifos/mobile/feature/third/party/transfer/thirdPartyTransfer/TptViewModel.kt (1)
476-478: Consider usingvalinstead ofvarfor state properties.In a data class representing immutable state, using
varforaccountOptionsTemplate,fromAccountOptions, andtoAccountOptionsbreaks the immutability contract. State updates should create new instances viacopy()rather than mutating existing properties.♻️ Suggested fix
- var accountOptionsTemplate: AccountOptionsTemplate = AccountOptionsTemplate(), - var fromAccountOptions: List<AccountOption> = emptyList(), - var toAccountOptions: List<AccountOption> = emptyList(), + val accountOptionsTemplate: AccountOptionsTemplate = AccountOptionsTemplate(), + val fromAccountOptions: List<AccountOption> = emptyList(), + val toAccountOptions: List<AccountOption> = emptyList(),core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/FaqItemHolder.kt (1)
63-70: Consider unifying typography source within the component.The question text still uses
MifosTypography, while the answer text now usesKptTheme.typography. If the intent is a full migration, aligning both toKptThemewill avoid mixed typography sources.♻️ Suggested tweak
- style = MifosTypography.bodyMedium, + style = KptTheme.typography.bodyMedium,feature/savings-account/src/commonMain/composeResources/values-es/strings.xml (1)
97-102: Inconsistent indentation and trailing whitespace.Line 98 has extra indentation compared to the rest of the file, and there are unnecessary blank lines before the closing tag.
Proposed fix
<string name="feature_savings_no_accounts_found">No se encontraron Cuentas de Ahorros</string> - <string name="feature_generic_error_server">Error del servidor. No se puede completar la operación. Por favor, inténtalo de nuevo.</string> - - - - </resources> + <string name="feature_generic_error_server">Error del servidor. No se puede completar la operación. Por favor, inténtalo de nuevo.</string> +</resources>core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/BeneficiaryCard.kt (1)
63-75: Inconsistent migration from DesignToken to KptTheme.The migration appears incomplete within this file:
- Line 63: Migrated to
KptTheme.spacing.md- Line 71: Still uses
DesignToken.sizes.iconExtraLarge- Line 75: Still uses
DesignToken.padding.mediumAdditionally, the semantic change from
DesignToken.padding.largetoKptTheme.spacing.md("large" → "md") may alter the actual spacing value. Please verify this is intentional.Consider completing the migration for consistency, or if partial migration is intentional, add a comment explaining the approach.
♻️ Suggested consistent migration
modifier = Modifier .clip(CircleShape) - .size(DesignToken.sizes.iconExtraLarge) + .size(KptTheme.sizes.iconExtraLarge) // if available in KptTheme .background(Color.LightGray), bitmap = null, ) - Spacer(modifier = Modifier.width(DesignToken.padding.medium)) + Spacer(modifier = Modifier.width(KptTheme.spacing.sm)) // or appropriate KptTheme equivalentcore/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/MifosDashboardCard.kt (1)
108-109: Remove commented-out code.These commented lines appear to be remnants from the migration process. Either complete the migration to use
KptTheme.colorScheme.secondaryor remove the comments to avoid confusion.🧹 Remove dead code
Text( text = stringResource(loanAccount), style = MifosTypography.bodySmall, -// color = KptTheme.colorScheme.secondary.copy(alpha = 0.7f), color = AppColors.customWhite.copy(alpha = 0.5f), )Text( text = stringResource(savingsAccount), style = MifosTypography.bodySmall, -// color = KptTheme.colorScheme.secondary, color = AppColors.customWhite.copy(alpha = 0.5f), )Also applies to: 132-133
feature/settings/src/commonMain/composeResources/values-te/strings.xml (1)
14-14: Remove non-translatable string from locale file.Strings marked
translatable="false"should only be defined in the defaultvalues/strings.xml, not in locale-specific files. This entry is redundant here.♻️ Suggested fix
- <string name="password_key" translatable="false">Password</string>feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryDetail/BeneficiaryDetailScreen.kt (1)
165-166: Consider handling or documenting the empty else branch.The empty
elsebranch could silently ignore newScreenUiStatesubtypes if added in the future. If thewhenis meant to be exhaustive, consider removingelseand handling all sealed subtypes explicitly, or add a comment explaining why this is intentional.feature/loan-account/src/commonMain/composeResources/values-es/strings.xml (1)
103-103: String keyrepayment_scheduleis inconsistent with thefeature_loan_*naming convention used throughout this file.However, this key is actively referenced in code (
RepaymentScheduleScreen.kt). If renamed tofeature_loan_repayment_schedule, you'll also need to update the import and usage in the RepaymentScheduleScreen.kt file, plus all locale files. The consistent naming across all locales suggests this may be intentional design for a top-level screen string rather than a detail section string.core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/MifosOutlineDropDown.kt (3)
93-97: Consider differentiating focused vs unfocused border colors.Both
focusedBorderColorandunfocusedBorderColorare set to the same value (KptTheme.colorScheme.secondaryContainer). This removes the visual distinction when the dropdown gains focus, which may impact usability and accessibility.If this is intentional design, no action needed. Otherwise, consider using a more prominent color (e.g.,
primary) forfocusedBorderColor.
105-113: Shape mismatch between border and container.The border uses
KptTheme.shapes.medium(line 108) while the container usesKptTheme.shapes.large(line 113). This inconsistency could cause visual misalignment where the border doesn't properly follow the container's corners.Consider using the same shape for both:
♻️ Proposed fix
.border( width = DesignToken.strokes.thin, color = KptTheme.colorScheme.secondaryContainer, - shape = KptTheme.shapes.medium, + shape = KptTheme.shapes.large, ),
45-45: Remove duplicate@OptInannotation.
ExperimentalMaterial3Api::classappears twice in the@OptInannotation.♻️ Proposed fix
-@OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterial3Api::class) +@OptIn(ExperimentalMaterial3Api::class)cmp-android/src/main/res/values-es/strings.xml (2)
14-16: Translation inconsistencies with feature/auth strings.The same string keys have different translations across modules:
toast_welcome: "Bienvenido %1$s" (here) vs "Hola %1$s" (auth module)login_failed: Different phrasing between filesno_internet_connection: "Sin conexión a Internet" vs "Sin conexión a internet" (capitalization)Consider aligning translations to ensure consistent user experience.
218-218: Trailing space before closing tag.Minor formatting issue - there's a trailing space before the closing
</string>tag.Suggested fix
- <string name="no_beneficiary_found_please_add">Actualmente no tiene ningún beneficiario. Por favor, agréguelo.</string> + <string name="no_beneficiary_found_please_add">Actualmente no tiene ningún beneficiario. Por favor, agréguelo.</string>core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/NoInternet.kt (1)
64-68: Hard-coded "Retry" string should use a string resource.For i18n/l10n consistency (especially given the PR includes localization additions for Spanish/Telugu/Urdu), consider extracting "Retry" to a string resource.
🌐 Suggested fix
- FilledTonalButton(onClick = { retry.invoke() }) { - Text(text = "Retry") + FilledTonalButton(onClick = { retry.invoke() }) { + Text(text = stringResource(Res.string.retry)) }core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/navigation/MifosBottomBar.kt (1)
31-43: Redundant.background()modifier.The
containerColorparameter on line 32 already sets the surface background forBottomAppBar. The additional.background(KptTheme.colorScheme.surface)modifier on line 42 is redundant and can be removed.Also, consider removing or addressing the commented-out shadow code (lines 37-41) to avoid dead code accumulation.
♻️ Suggested cleanup
BottomAppBar( containerColor = KptTheme.colorScheme.surface, contentColor = KptTheme.colorScheme.onSurface, windowInsets = windowInsets, modifier = modifier - .fillMaxWidth() -// .shadow( -// elevation = DesignToken.elevation.elevation, -// spotColor = Color(0xFF5D5D5D), -// ambientColor = Color.Black, -// ) - .background(KptTheme.colorScheme.surface), + .fillMaxWidth(), tonalElevation = 0.dp, ) {core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/MifosDropDownPayFromComponent.kt (1)
174-180: Consider using the semantically correct text color for tertiary surfaces.
KptTheme.colorScheme.onPrimaryis used for text displayed on atertiarybackground (see line 198). Typically,onTertiarywould be the semantically appropriate color for text on tertiary surfaces to ensure proper contrast and theme consistency.💡 Suggested semantic color usage
Text( text = accountNumber, style = MifosTypography.titleMediumEmphasized, - color = KptTheme.colorScheme.onPrimary, + color = KptTheme.colorScheme.onTertiary, ) Spacer(modifier = Modifier.height(KptTheme.spacing.xs)) Text( text = balance, style = MifosTypography.bodySmall, - color = KptTheme.colorScheme.onPrimary, + color = KptTheme.colorScheme.onTertiary, )core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/MifosDropDownTextField.kt (1)
58-60: Simplify the boolean expression.
expanded = true && isEnabledis equivalent toexpanded = isEnabled. Thetrue &&is redundant.♻️ Suggested simplification
LaunchedEffect(key1 = isPressed) { - if (isPressed) expanded = true && isEnabled + if (isPressed) expanded = isEnabled }The same redundant pattern exists at line 134 in
MifosDropDownDoubleTextField.core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/component/MifosBottomSheet.kt (1)
74-77: Attach the preview height modifier to a composable.
Modifier.height(...)is currently unused, so the preview won’t reflect the intended height. Consider applying it to theBox(or using aSpacer).♻️ Proposed fix
- content = { - Box { - Modifier.height(DesignToken.sizes.boxDp100) - } - }, + content = { + Box(modifier = Modifier.height(DesignToken.sizes.boxDp100)) { + } + },core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/component/MifosButton.kt (1)
241-242: Track the TODO with a follow-up issue.
This TODO references a known error path; linking it to a task will help ensure it doesn’t get lost.If you want, I can draft a short issue description for this follow-up.
core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/component/scrollbar/AppScrollbars.kt (1)
185-185: Track the TODO for KptTheme shapes.
Please add a follow-up task/issue to resolve the KptTheme shape error or document the workaround. I can help investigate a safe KptTheme-based fallback if needed.core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/TransactionScreenItem.kt (1)
75-82: Inconsistent spacing token usage.The code mixes
KptTheme.spacing(lines 75, 82) withDesignToken.spacing(line 78). Consider using one approach consistently for maintainability.♻️ Suggested consistency fix
- .padding(KptTheme.spacing.sm), + .padding(DesignToken.spacing.small), ) Spacer(modifier = Modifier.width(DesignToken.spacing.medium)) Column( modifier = Modifier.weight(1f), - verticalArrangement = Arrangement.spacedBy(KptTheme.spacing.xs), + verticalArrangement = Arrangement.spacedBy(DesignToken.spacing.extraSmall), )Or alternatively, migrate all to
KptTheme.spacing:- Spacer(modifier = Modifier.width(DesignToken.spacing.medium)) + Spacer(modifier = Modifier.width(KptTheme.spacing.md))
| <string name="please_wait">Espere por favor...</string> | ||
| <string name="message">mensaje</string> | ||
| <string name="no_image_selected_or_something_went_wrong">No se ha seleccionado ninguna imagen o algo ha salido mal</string> | ||
| <string name="seleted_qr_image">Imagen QR seleccionada</string> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for the typo in all XML files
rg -n "seleted_qr_image" --type xml
# Also search for the correct spelling to see if it's used elsewhere
rg -n "selected_qr_image" --type xml
# Check the base strings file
fd "strings.xml$" -not -path "*/values-*" | head -20Repository: openMF/mifos-mobile
Length of output: 2809
🏁 Script executed:
# Search for code references to both key names
rg "seleted_qr_image" --type java --type kt
rg "selected_qr_image" --type java --type kt
# Also check Compose files
rg "seleted_qr_image" --type rs
rg "selected_qr_image" --type rsRepository: openMF/mifos-mobile
Length of output: 170
🏁 Script executed:
# Search for code references without file type restrictions
rg "seleted_qr_image" --type-list | head -5
rg "seleted_qr_image"
rg "selected_qr_image"Repository: openMF/mifos-mobile
Length of output: 2909
🏁 Script executed:
# Search for code references to the key name - looking for R.string or string resource references
rg "seleted_qr_image|selected_qr_image" | grep -E "\.(kt|java|xml|compose)" | grep -v "string name=" | head -20Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
# Search for string references in code (looking for R.string pattern)
rg "R\.string\." | grep -i "qr\|selected"
# Also search for context.getString or stringResource patterns
rg "getString.*qr|stringResource.*qr" -i
# Find Kotlin/Java source files in cmp-android QR-related code
fd "\.kt$|\.java$" cmp-android | xargs grep -l "qr\|QR" -i | head -10
</parameter>Repository: openMF/mifos-mobile
Length of output: 2468
🏁 Script executed:
# Search for code that references the QR image string
rg "seleted_qr_image" --no-heading
# Check if the feature/qr module is the current active code
rg "selected_qr_image" --no-heading
# Look for any activity or fragment that uses QR image functionality
fd "\.kt$" feature/qr | head -5Repository: openMF/mifos-mobile
Length of output: 2910
Key name typo: "seleted_qr_image" should be "selected_qr_image".
This typo exists in the base file cmp-android/src/main/res/values/strings.xml and is inherited by all translations including the Spanish variant at this location. Fix the key name in the base file and all language variants for consistency. Note: The newer feature/qr module uses the correct spelling "selected_qr_image".
🤖 Prompt for AI Agents
In `@cmp-android/src/main/res/values-es/strings.xml` at line 488, The string
resource key "seleted_qr_image" is misspelled and must be renamed to
"selected_qr_image" everywhere; update the base strings.xml entry and all
localized variants that currently declare the "seleted_qr_image" key to use
"selected_qr_image" instead, preserving each locale's translated value ("Imagen
QR seleccionada" in Spanish) and ensuring you also update any references in code
or XML that reference the old key (search for usages of seleted_qr_image and
replace with selected_qr_image).
| <string name="feature_settings_appinfo_mobile">మొబైల్</string> | ||
| <string name="feature_settings_appinfo_logo_content_description">మిఫోస్ చిత్రం</string> | ||
| <string name="feature_settings_appinfo_app_name">మిఫోస్ మొబైల్</string> | ||
| <string name="feature_settings_appinfo_mifos_electonic_banking">మిఫోస్ ఎలక్ట్రానిక్ బ్యాంకింగ్</string> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking all strings.xml files for this key ==="
rg -n "feature_settings_appinfo_mifos_elect" --type xml
echo ""
echo "=== Checking code references ==="
rg -n "feature_settings_appinfo_mifos_elect" --type kotlinRepository: openMF/mifos-mobile
Length of output: 3792
Typo in string resource key must be fixed across all locale files and code: "electonic" → "electronic".
The key feature_settings_appinfo_mifos_electonic_banking has a typo missing the 'r' in "electronic". This misspelling is consistent across all 18 locale files (default, en, ar, te, gu, hi, hu, kn, mr, ms, pt, si, sw, es, fr, de, in, ur) and referenced in the Kotlin code at AppInfoScreen.kt (import and usage). The string values themselves are correctly translated, but the resource key name needs to be corrected everywhere.
🤖 Prompt for AI Agents
In `@feature/settings/src/commonMain/composeResources/values-te/strings.xml` at
line 101, The resource key feature_settings_appinfo_mifos_electonic_banking
contains a typo ("electonic") and must be renamed to
feature_settings_appinfo_mifos_electronic_banking across all locales and code;
update the key in every locale file (all 18 string XMLs) and then update all
references/imports/usages in Kotlin (e.g., where AppInfoScreen.kt imports or
accesses the resource) to the new key so builds and lookups remain consistent;
ensure you run a project-wide search/replace for
feature_settings_appinfo_mifos_electonic_banking and update any generated
R/Resource usages or string resource accessors accordingly.
| subtitle = "${response.message}, ${getString( | ||
| Res.string.feature_apply_share_status_failure_tip, | ||
| state.shareProductName, | ||
| ), | ||
| )}", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find DataState definition
find . -type f -name "*.kt" | xargs rg -l "sealed.*DataState|data class Error"Repository: openMF/mifos-mobile
Length of output: 5331
🏁 Script executed:
#!/bin/bash
# Search for DataState.Error definition more broadly
rg -n "class Error.*DataState|sealed class DataState" --type kotlinRepository: openMF/mifos-mobile
Length of output: 286
🏁 Script executed:
#!/bin/bash
# Read the specific lines in question
head -n 660 feature/share-application/src/commonMain/kotlin/org/mifos/mobile/feature/share/application/fillApplication/FillApplicationViewModel.kt | tail -n 20Repository: openMF/mifos-mobile
Length of output: 1217
🏁 Script executed:
#!/bin/bash
# Read the DataState definition to check message nullability
cat core/common/src/commonMain/kotlin/org/mifos/mobile/core/common/DataState.ktRepository: openMF/mifos-mobile
Length of output: 1199
🏁 Script executed:
#!/bin/bash
# Check if there are similar error message concatenation patterns in the codebase
rg -n "response\.message.*getString|exception\.message.*getString" --type kotlin -A 2Repository: openMF/mifos-mobile
Length of output: 680
🏁 Script executed:
#!/bin/bash
# Check how other parts of the codebase handle error messages
rg -n "DataState\.Error" feature/share-application/src/commonMain/kotlin/org/mifos/mobile/feature/share/application/fillApplication/FillApplicationViewModel.kt -A 10 | head -60Repository: openMF/mifos-mobile
Length of output: 1671
Prevent "null" string from appearing in error messages and use localized formatting.
The code concatenates response.message directly, but since DataState.Error.message is defined as exception.message.toString(), a null exception message will render as the literal string "null" in the subtitle. Additionally, the hardcoded ", " separator is inappropriate for locale-aware formatting.
♻️ Suggested improvement
- subtitle = "${response.message}, ${getString(
+ val errorPrefix = response.message.takeIf { it != "null" }?.let { "$it, " } ?: ""
+ subtitle = "$errorPrefix${getString(
Res.string.feature_apply_share_status_failure_tip,
state.shareProductName,
- )}",
+ )}",Alternatively, use a localized format string that accepts both the error message and the tip as parameters:
subtitle = getString(
Res.string.feature_apply_share_status_failure_with_tip,
response.message.takeIf { it != "null" } ?: "",
state.shareProductName,
)This allows translators to control punctuation and word order.
🤖 Prompt for AI Agents
In
`@feature/share-application/src/commonMain/kotlin/org/mifos/mobile/feature/share/application/fillApplication/FillApplicationViewModel.kt`
around lines 655 - 658, The subtitle currently concatenates response.message
directly, which can render the literal "null" (because DataState.Error.message
is exception.message.toString()) and uses a hardcoded ", " separator; update the
assignment in FillApplicationViewModel.kt to use a localized format string via
getString (e.g., Res.string.feature_apply_share_status_failure_with_tip) that
accepts the error message and product name, and pass response.message.takeIf {
it != "null" } ?: "" (or similar null/placeholder-safe value) and
state.shareProductName so translators control punctuation/order and the literal
"null" is never shown.
Naman-kr404
force-pushed
the
MM-541-savings-account-details-doesnt-render-properly-the-values-in-the-button-table
branch
from
4406148 to
0302000
Compare
1 participant
Fixes - Jira-#541
Savings account details doesn't render properly the values in the button/table
Before
After
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.