refactor(feature): replace MaterialTheme with KptTheme#3013
Conversation
WalkthroughMigrates UI theming across many composables from androidx.compose.material3.MaterialTheme to template.core.base.designsystem.theme.KptTheme, replaces hardcoded dp values with DesignToken/KptTheme spacing and size tokens, and adds DesignToken.strokes and size/spacing tokens. No control-flow or public-signature removals; some default parameter values updated. Changes
Sequence Diagram(s)(omitted — changes are theming/token substitutions, not new multi-component control flows) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/notification/src/commonMain/kotlin/org/mifos/mobile/feature/notification/NotificationScreen.kt (1)
1-256: Migration is incomplete across the codebase—MaterialTheme usage persists in 200+ files.While this file correctly uses
KptThemethroughout, the verification found extensive remainingMaterialThemeusage across the codebase:
- Core UI components still reference
MaterialTheme.colorScheme,MaterialTheme.typography, andMaterialTheme.shapesdirectly (e.g., PasswordStrengthIndicator.kt, MifosCard.kt, MifosTopAppBar.kt)- Mixed imports of both
MaterialThemeandKptThemeexist incore-base/designsystem/src/commonMain/kotlin/template/core/base/designsystem/KptMaterialTheme.kt- Hardcoded dimension values (8.dp, 16.dp, 24.dp) remain in 25+ files instead of using
DesignTokenequivalentsComplete the migration across all remaining modules and libraries before merging.
feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryDetail/BeneficiaryDetailContent.kt (1)
152-184: KptTheme is not provided in the composition tree — this will cause theme colors to use hardcoded defaults instead of theme values.The
ActionBarfunction usesKptTheme.colorScheme.primary, butMifosMobileThemedoes not provide the requiredKptThemecomposition locals.MifosMobileThemewraps content inMaterialThemeandDesignTokenTheme, but neither of these provideLocalKptColors,LocalKptTypography,LocalKptShapes,LocalKptSpacing, orLocalKptElevation.When
KptTheme.colorSchemeis accessed, it retrievesLocalKptColors.current, which falls back to the hardcoded defaultKptColorSchemeImpl()instead of using the active theme. This means the Delete and Update button colors will always render with the default purple (Color(0xFF6750A4)) regardless of theme changes.To fix this,
MifosMobileThemeshould either:
- Replace
MaterialTheme+DesignTokenThemewithKptMaterialTheme, which provides both Material3 and KPT composition locals, or- Wrap the content with the
KptThemecomposable to provide the necessary KPT composition locals
♻️ Duplicate comments (1)
feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessScreen.kt (1)
52-52: Verify the KptTheme package name matches project conventions.Same issue as in MakeTransferScreen.kt: the import uses
template.core.base.designsystem.theme.KptThemeinstead of matching the project'sorg.mifos.mobile.*package structure. This appears to be a systematic issue across the theming migration.
🧹 Nitpick comments (9)
feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorAdd/AddGuarantorScreen.kt (1)
190-190: Consider migrating the remaining hardcoded spacing value.Line 190 still uses a hardcoded
10.dpvalue for the Spacer height. For consistency with the PR objective of migrating to design tokens, consider replacing this with an appropriate design token (e.g.,DesignToken.padding.mediumorDesignToken.spacing.small, depending on what's available in your design system).🔎 Example refactor suggestion:
- Spacer(modifier = Modifier.height(10.dp)) + Spacer(modifier = Modifier.height(DesignToken.padding.medium))feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/LoanCard.kt (1)
54-54: Consider migrating remaining hardcoded dimensions to DesignToken.Several hardcoded dp values remain in the file that could be migrated to design tokens for consistency:
- Card heights:
128.dp(lines 54, 72) and140.dp(line 159)- Elevation:
6.dp(line 150) - note that line 64 already usesDesignToken.elevation.elevation- Corner radii:
16.dp(line 160)While these might be component-specific values, migrating them to DesignToken would provide better consistency with the PR's objective and make future theme adjustments easier.
Also applies to: 72-72, 150-150, 159-160
feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/Dialogs.kt (1)
61-61: Consider using a design token for the 6.dp spacing.For consistency with the design system migration, consider replacing the hardcoded
6.dpvertical spacing with an appropriate design token (e.g.,DesignToken.spacing.smallor similar, if available).feature/onboarding-language/src/commonMain/kotlin/org/mifos/mobile/feature/onboarding/language/SetOnboardingLanguageScreen.kt (1)
113-113: Consider using a more semantically appropriate design token.The image displays the app logo, but the height uses
DesignToken.sizes.avatarMedium. While this achieves size standardization, a token likelogoMediumoriconLargewould better convey intent. IfavatarMediumis intentionally being reused for non-avatar elements, this is fine.feature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/components/ClientChargeItem.kt (1)
122-126: Theme migration complete; consider AppColors consistency.The error color correctly uses
KptTheme.colorScheme.error. Note thatAppColors.customEnable(line 123) remains unmigrated. If the goal is full KptTheme adoption, consider whether this custom color should also be sourced from KptTheme.feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/language/LanguageScreen.kt (1)
41-41: Theme migration applied correctly for unselected state.The unselected radio button text now uses
KptTheme.colorScheme.onSurface, aligning with the design-system migration.Note: Line 140 uses
AppColors.primaryBluefor the selected text style. Consider whether this should also migrate toKptTheme.colorScheme.primaryfor consistency across the theme.Also applies to: 139-144
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt (2)
355-398: Consider using theme colors for consistency.While the shape correctly uses
DesignToken.shapes.medium, several hardcoded colors remain:
- Line 361:
Color.Gray.copy(alpha = 0.5f)for border- Line 362:
Color.Whitefor card background- Line 388:
Color.Blackfor icon tint- Line 398:
Color.Whitefor dropdown backgroundFor a complete design system migration, consider using
KptTheme.colorSchemeequivalents (e.g.,outline,surface,onSurface) to ensure proper theming support, especially for dark mode compatibility.
449-472: Consider replacing remaining hardcoded dimensions.A few hardcoded dimension values remain (40.dp on Line 449, 50.dp and 25.dp on Line 460, 24.dp on Line 472) while most of the file has been migrated to use
DesignToken.spacingandDesignToken.padding. For consistency with the design system migration, consider replacing these with appropriate design tokens.feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountDetails/SavingsAccountDetailsScreen.kt (1)
386-399: Consider updating the preview to use KptTheme for consistency.The preview function still wraps content in
MifosMobileTheme, while the production code now usesKptTheme. This inconsistency means the design-time preview won't accurately reflect the actual theme used at runtime.🔎 Consider updating the preview theme wrapper:
If
KptThemeshould be used directly, update the preview:@Preview @Composable private fun Account_Details_Overview() { - MifosMobileTheme { + KptTheme { Column(modifier = Modifier.fillMaxSize()) { SavingsAccountDetailsContent(Alternatively, verify that
MifosMobileThemeinternally delegates toKptThemeto maintain consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (55)
feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accountTransactions/TransactionScreen.kt(3 hunks)feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/component/FilterSection.kt(4 hunks)feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginScreen.kt(6 hunks)feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/otpAuthentication/OtpAuthenticationScreen.kt(7 hunks)feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/recoverPassword/RecoverPasswordScreen.kt(6 hunks)feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/registration/RegistrationScreen.kt(6 hunks)feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt(4 hunks)feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/uploadId/UploadIdScreen.kt(4 hunks)feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryApplication/BeneficiaryApplicationContent.kt(2 hunks)feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryDetail/BeneficiaryDetailContent.kt(3 hunks)feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryList/BeneficiaryListScreen.kt(6 hunks)feature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/chargeDetails/ChargeDetailScreen.kt(5 hunks)feature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/components/ClientChargeItem.kt(4 hunks)feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorAdd/AddGuarantorScreen.kt(2 hunks)feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorDetails/GuarantorDetailContent.kt(2 hunks)feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorList/GuarantorListScreen.kt(3 hunks)feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt(6 hunks)feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/components/BottomSheetContent.kt(5 hunks)feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/component/AccountSummaryCard.kt(5 hunks)feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/component/RepaymentPeriodCard.kt(5 hunks)feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccount/LoanAccountScreen.kt(3 hunks)feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountDetails/LoanAccountDetailsScreen.kt(4 hunks)feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/ConfirmDetailsCard.kt(2 hunks)feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/LoanCard.kt(7 hunks)feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/loanApplication/LoanApplyScreen.kt(2 hunks)feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/uploadDocs/UploadDocsScreen.kt(2 hunks)feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/uploadDocs/component/BottomSheetContent.kt(4 hunks)feature/location/src/commonMain/kotlin/org/mifos/mobile/feature/location/LocationScreen.kt(2 hunks)feature/notification/src/commonMain/kotlin/org/mifos/mobile/feature/notification/NotificationScreen.kt(4 hunks)feature/onboarding-language/src/commonMain/kotlin/org/mifos/mobile/feature/onboarding/language/SetOnboardingLanguageScreen.kt(5 hunks)feature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/PasscodeScreen.kt(8 hunks)feature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/verifyPasscode/VerifyPasscodeScreen.kt(7 hunks)feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/CropContent.kt(3 hunks)feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/Dialogs.kt(2 hunks)feature/qr/src/commonMain/kotlin/org/mifos/mobile/feature/qr/qrCodeDisplay/QrCodeDisplayScreen.kt(3 hunks)feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt(15 hunks)feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccount/SavingsAccountScreen.kt(3 hunks)feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountDetails/SavingsAccountDetailsScreen.kt(6 hunks)feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountUpdate/AccountUpdateScreen.kt(2 hunks)feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountWithdraw/AccountWithdrawScreen.kt(3 hunks)feature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/fillApplication/FillApplicationScreen.kt(4 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/about/AboutScreen.kt(3 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/appInfo/AppInfoScreen.kt(2 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/componenets/MifosLogoutDilaog.kt(3 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/faq/FaqScreen.kt(2 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/help/HelpScreen.kt(1 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/language/LanguageScreen.kt(2 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/passcode/UpdatePasscodeScreen.kt(2 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/settings/SettingsScreen.kt(4 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/theme/ChangeThemeScreen.kt(2 hunks)feature/share-account/src/commonMain/kotlin/org/mifos/mobile/feature/shareaccount/shareAccount/ShareAccountScreen.kt(3 hunks)feature/share-application/src/commonMain/kotlin/org/mifos/mobile/feature/share/application/fillApplication/FillApplicationScreen.kt(4 hunks)feature/third-party-transfer/src/commonMain/kotlin/org/mifos/mobile/feature/third/party/transfer/thirdPartyTransfer/TptScreen.kt(6 hunks)feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/makeTransfer/MakeTransferScreen.kt(3 hunks)feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessScreen.kt(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorDetails/GuarantorDetailContent.kt (1)
core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/MifosTexts.kt (1)
MifosTextTitleDescDoubleLine(62-83)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR Checks KMP / Static Analysis Check
🔇 Additional comments (92)
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/help/HelpScreen.kt (1)
351-351: Good refactoring to use shape tokens—verify visual consistency.This change correctly migrates from an inline
RoundedCornerShapeusing a padding value to a predefined shape token, which better aligns with design system best practices. However, since the original implementation usedDesignToken.padding.smallas the corner radius and the new implementation usesDesignToken.shapes.small, please verify that both produce visually consistent button shapes.Given that the PR template requested screenshots for UI changes but none were provided, consider adding before/after screenshots to confirm there are no unintended visual regressions in the button appearance.
Verification approach
Compare the button appearance in the Help screen before and after this change to ensure:
- Corner radius appears identical or intentionally updated
- Button visual consistency with other buttons using
DesignToken.shapes.small- Overall Help screen layout remains unchanged
feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorAdd/AddGuarantorScreen.kt (2)
49-49: Good addition of DesignToken import.The import is correctly added to support the design token usage in the padding modifications below.
152-153: Excellent migration to design tokens.Replacing hardcoded padding values with
DesignToken.padding.largealigns perfectly with the PR objective and improves consistency across the design system.feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/components/BottomSheetContent.kt (3)
47-47: Import correctly updated for theming migration.The import statement properly adds
KptThemeto support the migration from MaterialTheme to the custom design system theme.
96-100: Border colors correctly migrated to KptTheme.The border color references for both the card container and icon box have been successfully updated to use
KptTheme.colorScheme.secondaryContainer, maintaining visual consistency.Also applies to: 108-114
131-138: Text colors correctly migrated to KptTheme.The text color references have been properly updated to use
KptTheme.colorScheme.onSurfacefor the account title andKptTheme.colorScheme.secondaryfor the tip text, maintaining appropriate semantic color usage.Also applies to: 140-147
feature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/chargeDetails/ChargeDetailScreen.kt (2)
50-50: LGTM! Consistent theme migration.The migration from
MaterialThemetoKptThemeis correctly implemented across all modified lines. The import is properly added, and all typography and color scheme references are consistently updated.Also applies to: 141-141, 155-155, 164-164, 201-201
162-162: Verify if MifosTypography should also be migrated.While all other typography references were migrated to
KptTheme.typography, this line still usesMifosTypography.bodySmallEmphasized. Please confirm whether this is intentional (e.g., ifKptThemedoesn't provide an emphasized variant) or if it should also be migrated for consistency.#!/bin/bash # Search for other MifosTypography usages to determine if this is a pattern or an outlier rg -n --type=kotlin 'MifosTypography\.' -C2feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/CropContent.kt (1)
36-36: LGTM! Theme migration implemented correctly.The migration from MaterialTheme to KptTheme is properly executed:
- KptTheme import added correctly
- Hardcoded padding (16.dp) replaced with design token (DesignToken.padding.large)
- Typography style migrated from MaterialTheme to KptTheme
The changes align with the PR objective and maintain consistency with existing DesignToken usage in the file.
Also applies to: 66-66, 89-89
feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/LoanCard.kt (4)
39-39: LGTM! Theme import successfully updated.The KptTheme import has been correctly added to support the theme migration.
92-92: Excellent migration to design tokens!All spacing and padding values have been correctly migrated from hardcoded dp values to DesignToken properties. The token names are semantically appropriate and consistent with the design system.
Also applies to: 101-101, 118-118, 170-170, 178-178, 186-186, 202-202
134-134: Theme migration successfully applied.The default
backgroundColorparameter has been correctly updated fromMaterialTheme.colorScheme.primarytoKptTheme.colorScheme.primary, maintaining consistency with the design system migration.
116-125: Verify that preview functions render correctly with KptTheme.The preview functions don't explicitly wrap their content with
KptTheme(or any theme provider). SinceLoanCardCustomnow defaults toKptTheme.colorScheme.primaryat line 134, ensure that the preview infrastructure providesKptThemeautomatically, or the previews may fail to render correctly.If previews don't work, consider wrapping the preview content:
@Preview @Composable fun LoanCardPreviewCustom() { KptTheme { LoanCardCustom( modifier = Modifier.padding(DesignToken.padding.large), cardImage = UiRes.drawable.ic_icon_dashboard, onClick = {}, title = "title", amount = "amount", interestRate = "7.5 %", ) } }Also applies to: 200-209
feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/Dialogs.kt (3)
31-32: LGTM: Theme system imports added correctly.The new imports for
DesignTokenandKptThemeproperly support the migration fromMaterialThemeto the custom theme system.
59-59: LGTM: Shape reference migrated to KptTheme.The shape reference has been correctly updated from
MaterialTheme.shapes.smalltoKptTheme.shapes.small, aligning with the PR's theme migration objective.
63-63: Padding change verified and correct.The migration from hardcoded
16.dptoDesignToken.padding.largeis accurate.DesignToken.padding.largeequals16.dp(line 136 in DesignToken.kt), ensuring no visual regression.feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/ConfirmDetailsCard.kt (2)
44-44: LGTM: Color scheme migration is correct.The migration from
MaterialTheme.colorScheme.secondaryContainertoKptTheme.colorScheme.secondaryContainercorrectly aligns with the PR objective to replace MaterialTheme with KptTheme throughout the codebase.
29-29: No issues found with the KptTheme import.The import
template.core.base.designsystem.theme.KptThemeis correct and valid. Thetemplate.core.basepackage structure is the legitimate location of the design system module, not a placeholder. This import pattern is consistently used across the entire codebase in 20+ files. The KptTheme object is properly defined incore-base/designsystem/src/commonMain/kotlin/template/core/base/designsystem/theme/KptColorSchemeImpl.kt.Likely an incorrect or invalid review comment.
feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/makeTransfer/MakeTransferScreen.kt (2)
194-194: LGTM! Consistent theming migration for error states.The migration from
MaterialTheme.colorScheme.errortoKptTheme.colorScheme.erroris correctly applied to both error indicator icons, maintaining consistent error styling across the form validation.Also applies to: 222-222
58-58: The review comment regarding the KptTheme import package naming cannot be verified without access to the repository source code. To proceed, the actual package structure and KptTheme definition location in the codebase must be confirmed.feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessScreen.kt (1)
170-170: LGTM! Consistent typography migration.The migration from
MaterialTheme.typography.labelLargetoKptTheme.typography.labelLargecorrectly updates the button text styling to use the custom design system.feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/component/AccountSummaryCard.kt (2)
51-51: LGTM! KptTheme import added correctly.The import addition supports the theming migration throughout this file.
78-78: Theme migration to KptTheme needs manual verification.The migration from
MaterialTheme.colorSchemetoKptTheme.colorSchemeappears consistent across all five identified lines (78, 98, 104, 124, 133), using semantic color tokenssecondaryContainer,onSurface, andsecondary. However, verification requires access to the repository to confirm:
- KptTheme is properly defined and available in module dependencies
- KptTheme.colorScheme provides the expected color tokens
- No remaining MaterialTheme.colorScheme references exist in the file
feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccount/SavingsAccountScreen.kt (2)
225-225: LGTM! Text color migrations are semantically correct.The migration to
KptTheme.colorScheme.onBackgroundfor the title andKptTheme.colorScheme.secondaryfor the label appropriately uses semantic color roles from the design system.Also applies to: 233-233
278-283: Review status colors for consistent theming approach.The
whenexpression at lines 279-282 mixesAppColors(customEnable, customYellow) withKptTheme.colorScheme(error, onSurface). Verify whetherAppColors.customEnableandAppColors.customYellowshould migrate to KptTheme equivalents for consistency, or if these custom colors are intentionally preserved due to missing KptTheme definitions.feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/uploadDocs/component/BottomSheetContent.kt (2)
74-74: LGTM! Import addition supports the theming migration.The
KptThemeimport is correctly added to support the migration fromMaterialThemeto the custom theme system.
152-152: LGTM! Theming migration is consistent and correct.The color scheme references have been properly migrated from
MaterialThemetoKptThemeacross all three locations:
- Border color in
BottomSheetIconContainer(line 152)- Text color in
BottomSheetIconContainer(line 170)- Border color in
SignatureContentcard (line 220)No
MaterialThemereferences remain in the file. The changes maintain the same semantic color roles and align with the PR's objective to standardize on the custom theme system.feature/share-account/src/commonMain/kotlin/org/mifos/mobile/feature/shareaccount/shareAccount/ShareAccountScreen.kt (2)
223-223: Theme migration verified as correct.All color properties referenced in ShareAccountScreen.kt are properly defined in the KptColorScheme interface and implemented in KptColorSchemeImpl. The migration from MaterialTheme.colorScheme to KptTheme.colorScheme maintains Material3 compatibility and semantic correctness for:
onBackgroundfor primary text (line 223)secondaryfor secondary text (line 231)errorfor error states (line 289)onSurfacefor default surface content (line 290)
59-59: The import is correct.KptThemeis properly defined in thetemplate.core.base.designsystem.themepackage, which is the designated design system module for this Kotlin Multiplatform project. The package naming is consistent with the project's architecture, andKptThemeis used throughout the codebase with standard Material 3 color scheme properties (colorScheme, typography, shapes). No changes needed.Likely an incorrect or invalid review comment.
feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountUpdate/AccountUpdateScreen.kt (1)
50-50: Theme migration correctly applied.The import addition (
template.core.base.designsystem.theme.KptTheme) and the button color change fromMaterialTheme.colorScheme.primarytoKptTheme.colorScheme.primaryare aligned with the codebase's migration pattern and are correctly implemented.feature/onboarding-language/src/commonMain/kotlin/org/mifos/mobile/feature/onboarding/language/SetOnboardingLanguageScreen.kt (2)
53-53: LGTM! Import aligns with the theme migration.The KptTheme import is correctly added to support the MaterialTheme → KptTheme migration throughout this file.
125-125: LGTM! Theme color migration is correct.The Text color properties have been correctly migrated from
MaterialTheme.colorSchemetoKptTheme.colorScheme, maintaining the same semantic color roles (onSurfaceandsecondary).Also applies to: 135-135
feature/third-party-transfer/src/commonMain/kotlin/org/mifos/mobile/feature/third/party/transfer/thirdPartyTransfer/TptScreen.kt (2)
62-62: LGTM! Import added correctly.The KptTheme import has been added appropriately to support the theme migration.
167-167: Theme migration is complete and correct.All
MaterialTheme.colorSchemereferences have been consistently replaced withKptTheme.colorScheme. TheKptThemeobject properly exposes thecolorSchemeproperty via Compose'sCompositionLocalpattern, and all required color properties (primary,secondary,error,onSurface) are correctly defined inKptColorSchemeImpl. No remainingMaterialTheme.colorSchemereferences exist in the file.feature/share-application/src/commonMain/kotlin/org/mifos/mobile/feature/share/application/fillApplication/FillApplicationScreen.kt (2)
228-228: LGTM! Color scheme migration is consistent.The replacement of
MaterialTheme.colorScheme.onSurfacewithKptTheme.colorScheme.onSurfacefor the section header Text components is correct and consistent across all three occurrences. The semantic meaning (text color on surface) is preserved.Also applies to: 287-287, 318-318
63-63: The KptTheme import path at line 63 is correct and standard across the codebase. The packagetemplate.core.base.designsystem.themeis the proper location for KptTheme, which is defined in the core-base designsystem module. This import pattern is consistently used in 50+ files throughout the project, including other feature modules. No compilation or correctness issues exist here.Likely an incorrect or invalid review comment.
feature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/components/ClientChargeItem.kt (4)
42-42: LGTM!The KptTheme import is properly added to support the theming migration.
147-151: Duplicate concern: AppColors usage.Same observation as lines 122-126—
AppColors.customEnableremains unmigrated whileKptTheme.colorScheme.erroris correctly used.
75-82: Theme migration and design token usage are correct.The migration from
MaterialThemetoKptThemeis consistent, andDesignToken.sizes.iconExtraLargecorrectly resolves to 36.dp, maintaining the original icon size.
163-172: Verify that MifosMobileTheme provides KptTheme.The component now depends on
KptTheme, but the preview wraps it withMifosMobileTheme. Ensure thatMifosMobileThemeproperly provides theKptThemecomposition local; otherwise, the preview (and component usage) will fail at runtime.#!/bin/bash # Description: Check if MifosMobileTheme provides KptTheme composition local # Search for MifosMobileTheme definition and check if it provides KptTheme rg -nP --type=kt -A20 'fun MifosMobileTheme\s*\('feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorList/GuarantorListScreen.kt (2)
150-167: LGTM! Complete theme migration for GuarantorListItem.The component has been successfully migrated:
- Card colors now use
KptTheme.colorScheme.background- Spacing now uses
DesignToken.padding.*instead of hardcoded values- Typography now uses
KptTheme.typography.*The migration is mechanically correct and consistent with the PR objectives.
Consider performing visual regression testing to confirm that:
- The card background color matches the previous appearance
- The padding values (
DesignToken.padding.largeandDesignToken.padding.small) produce the expected spacing- The typography styles (
bodyMediumandlabelMedium) render as intended
102-102: Theme migration for FAB contentColor is correct.The migration from
MaterialThemetoKptThemeat line 102 is properly applied.KptThemeis correctly imported and used as the color source for thecontentColorparameter.feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/passcode/UpdatePasscodeScreen.kt (1)
133-133: Incomplete design token migration requires correction with proper token mapping.Lines 133 and 186 use hardcoded
24.dpvalues that should be replaced with design tokens. However, the suggested tokenDesignToken.spacing.largeis incorrect—it equals16.dp, not24.dp. No existing spacing token matches24.dpexactly. The closest alternatives areDesignToken.spacing.largeIncreased(20.dp) orDesignToken.spacing.extraLarge(28.dp). Verify the intended spacing value and apply the appropriate token, or add a new token for24.dpif it's a deliberate design value.feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryList/BeneficiaryListScreen.kt (4)
293-350: Theme migration looks correct, pending import fix.The ActionBar color changes from MaterialTheme to KptTheme are consistent and correct. All text and icon colors properly use
KptTheme.colorScheme.primary. However, this depends on fixing the import issue flagged above.
469-527: Color scheme migration is correct.The FilterSection text color changes to
KptTheme.colorScheme.onBackgroundandKptTheme.colorScheme.secondaryare appropriate and consistent with the theme migration.
76-77: No action needed. The importtemplate.core.base.designsystem.theme.KptThemeis correct and consistent with the project's architecture. Thetemplate.core.basepackage is the legitimate designation for the core-base module, not a placeholder. This import pattern is used consistently across 20+ files throughout the codebase.Likely an incorrect or invalid review comment.
402-402: Hardcoded divider height lacks a corresponding design token. Lines 402 and 537 use1.dpfor HorizontalDivider height, but DesignToken does not currently define a divider thickness constant. Consider adding a divider thickness property toAppSizes(or a new design token class) before replacing these hardcoded values.Likely an incorrect or invalid review comment.
feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/loanApplication/LoanApplyScreen.kt (2)
252-264: Theme migration applied correctly.The typography reference has been successfully migrated from MaterialTheme to KptTheme for the "Continue" button label. The change is consistent with the PR objective and maintains the same typography style reference (labelLarge).
Since the PR description indicates no screenshots were provided, consider visually verifying that the button text renders as expected with the new KptTheme typography to ensure UI consistency.
66-66: The import pathtemplate.core.base.designsystem.theme.KptThemeis correct and intentional. This refers to thecore-base/designsystemmodule, which uses thetemplate.*package prefix as part of the project's architectural separation between shared template libraries (template.core.base.*) and application-specific modules (org.mifos.mobile.*). KptTheme is properly defined in the codebase and the import is valid.Likely an incorrect or invalid review comment.
feature/location/src/commonMain/kotlin/org/mifos/mobile/feature/location/LocationScreen.kt (3)
39-40: No issues found. The design tokens are correctly configured withpadding.large = 16.dpandspacing.large = 16.dp, matching the original hardcoded values.
23-24: Imports are properly cleaned. The oldMaterialThemeanddpimports have been completely removed, and the newDesignTokenandKptThemeimports are correctly in place. Design token values (DesignToken.padding.largeandDesignToken.spacing.largeboth equal 16.dp) match the original hardcoded values, andKptTheme.typographyprovides the required typography styles. The migration is clean and complete.
44-48: No changes needed — KptTheme typography styles are equivalent to Material 3 defaults. ThebodyLargeandbodyMediumtext styles in KptTheme match their Material 3 counterparts exactly (bodyLarge: 16sp/Normal/24sp line height/0.5sp letter spacing; bodyMedium: 14sp/Normal/20sp line height/0.25sp letter spacing), ensuring no visual regressions from this design system migration.feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorDetails/GuarantorDetailContent.kt (2)
37-37: Design token migration verified—values match original dimensions.The replacement of hardcoded padding dimensions with DesignToken values is correct:
DesignToken.padding.large= 16.dp andDesignToken.padding.small= 8.dp preserve the original visual behavior across all five locations (lines 37, 45, 53, 61, 69).
27-27: No issues found. The import pathtemplate.core.base.designsystem.theme.KptThemeis correct and follows the established package structure used consistently throughout the codebase.Likely an incorrect or invalid review comment.
feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountWithdraw/AccountWithdrawScreen.kt (2)
48-48: The import is correct.template.core.base.designsystem.theme.KptThemeis the actual package where KptTheme is defined in the designsystem module, and this import pattern is used consistently throughout the codebase.
187-189: Theme migration to KptTheme is correct.KptTheme.colorScheme provides all required Material3 ColorScheme properties including
secondaryContainer,error, andprimary. The implementation uses appropriate color values (secondaryContainer: #E8DEF8, error: #BA1A1A, primary: #6750A4) for these UI states.feature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/fillApplication/FillApplicationScreen.kt (2)
217-217: LGTM! Theme migration is consistent.All three Text composables have been correctly updated to use
KptTheme.colorScheme.onSurfaceinstead ofMaterialTheme.colorScheme.onSurface. The changes are mechanically correct and maintain visual consistency.Also applies to: 247-247, 279-279
64-64: No action needed—the import is correct.The import
template.core.base.designsystem.theme.KptThemeis valid. TheKptThemeobject is properly defined in that package, and "template" is the project's intentional package prefix used consistently throughout the codebase (confirmed across core-base modules). The theme migration fromMaterialTheme.colorScheme.onSurfacetoKptTheme.colorScheme.onSurfaceis mechanically sound and uses the correct API.feature/notification/src/commonMain/kotlin/org/mifos/mobile/feature/notification/NotificationScreen.kt (5)
191-191: LGTM: Divider color migration is consistent.The
HorizontalDividercolor correctly usesKptTheme.colorScheme.surfaceDim, maintaining the same semantic color property from the previousMaterialThemeimplementation.
224-228: LGTM: Icon tint colors migrated correctly.The conditional icon tint logic correctly uses
KptTheme.colorScheme.onSurfaceVariantandKptTheme.colorScheme.primary, maintaining the same color semantics as before.
233-242: LGTM: Typography styles migrated correctly.The text styles correctly use
KptTheme.typography.bodyMediumandKptTheme.typography.labelMedium, maintaining the same typography semantics from the previousMaterialThemeimplementation.
218-219: Design token migration verified—no visual regressions.The design token values match the previous hardcoded dimensions:
DesignToken.padding.small = 8.dpandDesignToken.spacing.large = 16.dp. The changes are safe.
47-52: KptTheme and DesignToken are properly configured with required properties.Verification confirms both KptTheme and DesignToken are correctly available and provide all required properties:
- KptTheme.colorScheme: Includes surfaceDim, onSurfaceVariant, and primary
- KptTheme.typography: Includes bodyMedium and labelMedium
- DesignToken.padding: Includes small (8.dp, matching the original hardcoded value)
- DesignToken.spacing: Includes large (16.dp, matching the original hardcoded value)
The theming migration is properly configured.
feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/uploadDocs/UploadDocsScreen.kt (2)
53-53: The import is correct. Thetemplate.core.base.designsystem.theme.KptThemepackage is the intentional and consistent naming convention used throughout the core-base modules in the codebase (designsystem, platform, ui, common, database, datastore, analytics). This is not a placeholder.
202-202: No action needed. The typography migration fromMaterialTheme.typography.labelLargetoKptTheme.typography.labelLargeis correctly implemented.KptTheme.typography.labelLargeis properly defined with appropriate Material 3 design tokens (Medium weight, 14sp font size), and the change is consistent throughout the codebase.feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryDetail/BeneficiaryDetailContent.kt (1)
44-44: No issues found. The import pathtemplate.core.base.designsystem.theme.KptThemeis correct and consistent with the project's package structure, which uses "template" as the top-level package prefix throughout the codebase.feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accountTransactions/TransactionScreen.kt (1)
68-68: LGTM! Theme migration executed correctly.The import of
KptThemeand replacement ofMaterialTheme.colorScheme.primarywithKptTheme.colorScheme.primaryaligns with the PR's design-system migration objectives. The changes maintain visual consistency while adopting the custom theme.Also applies to: 302-311
feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryApplication/BeneficiaryApplicationContent.kt (1)
51-51: LGTM! Themed text styling updated correctly.The
KptTheme.colorScheme.primarynow provides the accent color for the annotated string, maintaining the visual emphasis while aligning with the new design system.Also applies to: 175-175
feature/qr/src/commonMain/kotlin/org/mifos/mobile/feature/qr/qrCodeDisplay/QrCodeDisplayScreen.kt (1)
50-50: LGTM! Color scheme migration applied consistently.Both the title and footer text now use
KptTheme.colorScheme.primary, ensuring consistent theming across the QR code display screen.Also applies to: 128-128, 160-160
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/componenets/MifosLogoutDilaog.kt (1)
38-38: LGTM! Color scheme references migrated consistently.All text color assignments in the logout dialog now use
KptTheme.colorScheme, maintaining visual hierarchy with appropriate primary and secondary colors.Also applies to: 87-87, 93-93, 115-115
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/registration/RegistrationScreen.kt (1)
85-85: LGTM! Comprehensive theming and spacing migration.The registration screen successfully migrates to
KptTheme.colorSchemefor all color references (onSurface, secondary, primary, error) and replaces the hardcoded8.dpwithDesignToken.padding.smallat line 334, demonstrating consistent adoption of the design-system tokens.Also applies to: 213-213, 221-221, 266-266, 309-311, 322-322, 334-334
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/appInfo/AppInfoScreen.kt (1)
49-49: LGTM! Theme migration applied to app title.The "Mifos" title text now uses
KptTheme.colorScheme.primary, ensuring brand consistency with the new design system.Also applies to: 139-139
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginScreen.kt (1)
71-71: LGTM! Comprehensive theming and sizing migration.The login screen successfully migrates to
KptTheme.colorSchemefor all color references (onSurface, secondary, error, primary) and replaces the hardcoded48.dpwithDesignToken.sizes.avatarMediumat line 207, ensuring consistent sizing across the design system.Also applies to: 207-207, 219-219, 227-227, 258-258, 291-291, 328-328
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/about/AboutScreen.kt (1)
50-50: LGTM! Theming migration looks good.The replacement of
MaterialTheme.colorScheme.primarywithKptTheme.colorScheme.primaryis consistent with the broader theming migration across the codebase.Also applies to: 123-123
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/uploadId/UploadIdScreen.kt (1)
76-76: LGTM! Consistent theming migration.All color references have been successfully migrated from
MaterialTheme.colorSchemetoKptTheme.colorScheme. The changes maintain visual consistency across onBackground, secondary, primary, and border colors.Also applies to: 162-162, 170-170, 275-275, 337-339
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/faq/FaqScreen.kt (1)
48-48: LGTM! Theming and design token migration is well-executed.The migration successfully replaces hardcoded spacing (
8.dp→DesignToken.spacing.small) and color references (MaterialTheme.colorScheme.primary→KptTheme.colorScheme.primary), aligning with the design system standards.Also applies to: 156-156, 160-160
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/settings/SettingsScreen.kt (1)
58-58: LGTM! Profile card theming updated correctly.The color scheme migration in
SettingsProfileCardmaintains visual consistency by updating border color and text colors (primary, secondary) to useKptTheme.colorScheme.Also applies to: 223-223, 240-240, 249-249
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (1)
70-70: LGTM! Comprehensive theming migration across HomeScreen.All color references have been successfully migrated to
KptTheme.colorScheme, including:
- Icon color filters (onSurface, tertiary)
- Text colors (onSurface)
- Border colors (secondaryContainer)
- Container colors (surface)
The changes maintain consistent visual theming throughout the home screen.
Also applies to: 143-143, 183-183, 209-209, 278-278, 284-284, 291-291, 328-328
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/recoverPassword/RecoverPasswordScreen.kt (1)
60-60: LGTM! RecoverPasswordScreen theming migration is complete.All theming updates are consistent:
- Text colors migrated to
KptTheme.colorScheme(onBackground, secondary, primary)- Error icon tints updated to
KptTheme.colorScheme.error- Spacing standardized with
DesignToken.spacing.mediumAlso applies to: 130-130, 133-133, 138-138, 186-186, 211-211, 241-241, 250-250
feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/component/FilterSection.kt (1)
46-46: LGTM! FilterSection theming updated correctly.Text colors for filter titles, counts, and labels have been consistently migrated to use
KptTheme.colorScheme(onBackground, secondary).Also applies to: 95-95, 105-105, 205-205
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt (1)
60-60: SetPasswordScreen theming migration is incomplete — hardcoded spacing value requires standardization.Color references have been successfully migrated to
KptTheme.colorScheme(lines 147, 155, 263, 272). However, line 158 contains a hardcoded24.dpspacing value that breaks the migration. ReplaceSpacer(modifier = Modifier.height(24.dp))with an appropriateDesignToken.spacingvalue (e.g.,DesignToken.spacing.extraLargeat 28.dp is the closest match).Likely an incorrect or invalid review comment.
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt (1)
60-67: LGTM! Clean design system migration.The import changes properly introduce
KptThemeandDesignTokento support the design system migration. The overall approach of systematically replacingMaterialThemereferences withKptThemethroughout this file is consistent and well-executed.feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/otpAuthentication/OtpAuthenticationScreen.kt (1)
66-66: LGTM! Comprehensive theming migration.The file has been thoroughly migrated from
MaterialThemetoKptTheme. All color references (onBackground, secondary, error, primary) and typography are consistently updated throughout the authentication screen. The changes maintain the existing functionality while adopting the new design system.feature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/PasscodeScreen.kt (1)
54-317: LGTM! Excellent migration with design tokens.This file demonstrates a complete migration from
MaterialThemetoKptThemewith proper adoption of design tokens:
- All color references migrated to
KptTheme.colorScheme- Fixed dimensions replaced with
DesignToken.sizesandDesignToken.spacing- Consistent theming across all UI elements (backgrounds, borders, text, icons)
The implementation is clean and maintains existing functionality while improving design system compliance.
feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/component/RepaymentPeriodCard.kt (1)
48-172: LGTM! Clean theming update.The migration updates all
MaterialThemereferences toKptThemeand replaces the hardcoded 36.dp icon size withDesignToken.sizes.iconExtraLarge. All color references (secondaryContainer, primary, outline, onSurface, error) are consistently updated to use the new theme, maintaining the component's functionality.feature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/verifyPasscode/VerifyPasscodeScreen.kt (1)
53-306: LGTM! Comprehensive migration.This file mirrors the excellent migration pattern seen in
PasscodeScreen.kt:
- Complete transition from
MaterialThemetoKptTheme- Hardcoded dimensions (16.dp, 24.dp, 32.dp) properly replaced with
DesignTokenequivalents- All color, surface, and spacing references consistently use the new design system
The verification screen maintains full functionality while adopting the standardized theming approach.
feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccount/LoanAccountScreen.kt (1)
61-293: LGTM! Targeted theming update.The migration updates the essential
MaterialTheme.colorSchemereferences toKptTheme.colorSchemefor text colors (onBackground, secondary) and account status colors (error, onSurface). The changes are minimal but complete for this screen's theming needs.feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountDetails/LoanAccountDetailsScreen.kt (1)
58-284: LGTM! Clean theming update.The file properly updates
MaterialTheme.colorSchemereferences toKptTheme.colorSchemefor text colors in the account details grid and actions section. The migration is complete and consistent with the PR's design system objectives.feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountDetails/SavingsAccountDetailsScreen.kt (4)
249-267: Theme migration applied correctly with conditional logic preserved.The replacement of
MaterialTheme.colorSchemewithKptTheme.colorSchememaintains the existing conditional color logic. However, ensure the color values inKptThemematch the expected design to avoid visual regressions.Consider adding a visual comparison (screenshots) to the PR description to verify the theme colors render as expected.
299-299: Theme migration applied correctly.The color scheme updates maintain the existing conditional logic for status highlighting. The use of
AppColors.customEnablefor active status andKptTheme.colorScheme.onBackgroundfor other cases is consistent with the refactoring approach.Also applies to: 316-322
350-350: Theme migration applied correctly.The color scheme replacement for the section header is consistent with the overall refactoring.
60-60: No issues found. The import pathtemplate.core.base.designsystem.theme.KptThemeis legitimate and correct. The package exists atcore-base/designsystem/src/commonMain/kotlin/template/core/base/designsystem/KptTheme.ktand follows the established naming convention of the core-base module. This import is used consistently across 58+ files in the project as the standard way to reference the shared theming system. The changes to useKptTheme.colorSchemeare valid.Likely an incorrect or invalid review comment.
| title = stringResource(Res.string.first_name), | ||
| description = data.firstname ?: "", | ||
| descriptionStyle = MaterialTheme.typography.bodyLarge, | ||
| descriptionStyle = KptTheme.typography.bodyLarge, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for MifosTextTitleDescDoubleLine function definition
find . -name "MifosTexts.kt" -type fRepository: openMF/mifos-mobile
Length of output: 143
🏁 Script executed:
#!/bin/bash
# Search for MifosTextTitleDescDoubleLine using ast-grep
ast-grep --pattern 'fun MifosTextTitleDescDoubleLine($$$) {
$$$
}'Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Also try a simpler ripgrep search for the component
rg -n "MifosTextTitleDescDoubleLine" --type=kotlinRepository: openMF/mifos-mobile
Length of output: 1585
🏁 Script executed:
#!/bin/bash
# Read the MifosTexts.kt file to check the MifosTextTitleDescDoubleLine implementation
cat -n ./core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/MifosTexts.kt | head -100Repository: openMF/mifos-mobile
Length of output: 3904
🏁 Script executed:
#!/bin/bash
# Read GuarantorDetailContent.kt to see how MifosTextTitleDescDoubleLine is called
cat -n ./feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorDetails/GuarantorDetailContent.kt | head -80Repository: openMF/mifos-mobile
Length of output: 3374
🏁 Script executed:
#!/bin/bash
# Check for other usages of MifosTextTitleDescDoubleLine in the codebase
rg -n "MifosTextTitleDescDoubleLine" --type=kotlin -B2 -A5 | head -80Repository: openMF/mifos-mobile
Length of output: 9172
Update MifosTextTitleDescDoubleLine to accept titleStyle parameter instead of hardcoding MaterialTheme.
The component currently uses MaterialTheme.typography.labelMedium for the title (line 72 of MifosTexts.kt) while the description accepts a parameterized style. This creates a mixed-theme state where callers pass KptTheme.typography.bodyLarge for descriptions but the title remains bound to MaterialTheme. If the objective is to fully migrate away from MaterialTheme, add a titleStyle parameter with a default value and update the component definition accordingly. Also update the preview at line 171 to use KptTheme for consistency.
| unselectedTextStyle = MifosTypography.titleSmallEmphasized.copy( | ||
| MaterialTheme.colorScheme.onSurface, | ||
| KptTheme.colorScheme.onSurface, | ||
| ), |
There was a problem hiding this comment.
Fix compilation error: missing parameter name in copy().
The copy() method requires the parameter name color = before the value. Compose's TextStyle.copy() uses named parameters only.
🔎 Apply this diff to fix the compilation error:
unselectedTextStyle = MifosTypography.titleSmallEmphasized.copy(
- KptTheme.colorScheme.onSurface,
+ color = KptTheme.colorScheme.onSurface,
),📝 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.
| unselectedTextStyle = MifosTypography.titleSmallEmphasized.copy( | |
| MaterialTheme.colorScheme.onSurface, | |
| KptTheme.colorScheme.onSurface, | |
| ), | |
| unselectedTextStyle = MifosTypography.titleSmallEmphasized.copy( | |
| color = KptTheme.colorScheme.onSurface, | |
| ), |
🤖 Prompt for AI Agents
In
feature/onboarding-language/src/commonMain/kotlin/org/mifos/mobile/feature/onboarding/language/SetOnboardingLanguageScreen.kt
around lines 170 to 172, the call to TextStyle.copy(...) is missing the named
parameter for the color; update the copy call to use the named argument (e.g.,
color = KptTheme.colorScheme.onSurface) so the code compiles with Compose's
named-parameter copy signature.
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/about/AboutScreen.kt
Outdated
Show resolved
Hide resolved
...ngs/src/commonMain/kotlin/org/mifos/mobile/feature/settings/passcode/UpdatePasscodeScreen.kt
Show resolved
Hide resolved
| unselectedTextStyle = MifosTypography.titleSmallEmphasized.copy( | ||
| MaterialTheme.colorScheme.onSurface, | ||
| KptTheme.colorScheme.onSurface, | ||
| ), |
There was a problem hiding this comment.
Critical: Missing parameter name in .copy() call.
Line 112 is missing the color = parameter name. The .copy() method on TextStyle requires named parameters in Compose. This will likely cause a compilation error.
Compare with line 109 where selectedTextStyle correctly uses color = AppColors.primaryBlue.
🔎 Apply this diff to fix the syntax error:
unselectedTextStyle = MifosTypography.titleSmallEmphasized.copy(
- KptTheme.colorScheme.onSurface,
+ color = KptTheme.colorScheme.onSurface,
),📝 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.
| unselectedTextStyle = MifosTypography.titleSmallEmphasized.copy( | |
| MaterialTheme.colorScheme.onSurface, | |
| KptTheme.colorScheme.onSurface, | |
| ), | |
| unselectedTextStyle = MifosTypography.titleSmallEmphasized.copy( | |
| color = KptTheme.colorScheme.onSurface, | |
| ), |
🤖 Prompt for AI Agents
In
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/theme/ChangeThemeScreen.kt
around lines 111 to 113, the unselectedTextStyle.copy(...) call omits the
required named parameter; change the call to use the named parameter (color =
KptTheme.colorScheme.onSurface) so the TextStyle.copy invocation matches
Compose's named-parameter requirement, mirroring how selectedTextStyle uses
color = AppColors.primaryBlue.
biplab1
force-pushed
the
MM-389-replace-materialtheme-with-kptmaterialtheme
branch
from
7e10ac7 to
0ab48f7
Compare
There was a problem hiding this comment.
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 (3)
feature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/components/ClientChargeItem.kt (1)
157-157: Replace hardcoded size with DesignToken.Line 157 uses a hardcoded
20.dpvalue, which is inconsistent with the PR objective to use DesignToken for sizes. Line 77 already demonstrates the preferred approach usingDesignToken.sizes.iconExtraLarge.🔎 Apply this diff to use DesignToken:
Icon( imageVector = MifosIcons.ChevronRight, contentDescription = "", - modifier = Modifier.size(20.dp), + modifier = Modifier.size(DesignToken.sizes.iconSmall), )Note: Verify that
DesignToken.sizes.iconSmall(or another appropriate size token) matches the desired 20.dp size.feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt (2)
319-348: Inconsistent padding usage.Line 322 mixes hardcoded
24.dpwithDesignToken.padding.large, which is inconsistent with the migration goals.🔎 Use DesignToken for all padding values
-.padding(horizontal = 24.dp, vertical = DesignToken.padding.large) +.padding(horizontal = DesignToken.padding.extraLarge, vertical = DesignToken.padding.large)Verify that
DesignToken.padding.extraLargeprovides the appropriate 24.dp equivalent, or use the closest available token.
355-391: Multiple hardcoded colors in account selector card.Lines 361, 362, 382, and 388 use hardcoded colors (
Color.Gray,Color.White,Color.Black) instead of KptTheme color tokens, which is inconsistent with the theming migration.🔎 Migrate hardcoded colors to KptTheme
-border = BorderStroke(1.dp, Color.Gray.copy(alpha = 0.5f)), -colors = CardDefaults.cardColors(containerColor = Color.White), +border = BorderStroke(1.dp, KptTheme.colorScheme.outline.copy(alpha = 0.5f)), +colors = CardDefaults.cardColors(containerColor = KptTheme.colorScheme.surface),-style = KptTheme.typography.bodySmall.copy(color = Color.Gray), +style = KptTheme.typography.bodySmall.copy(color = KptTheme.colorScheme.onSurfaceVariant),-tint = Color.Black, +tint = KptTheme.colorScheme.onSurface,
♻️ Duplicate comments (1)
feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorDetails/GuarantorDetailContent.kt (1)
42-42: Incomplete theme migration - MifosTextTitleDescDoubleLine still uses MaterialTheme internally.While the
descriptionStyleparameter is correctly updated toKptTheme.typography.bodyLarge, theMifosTextTitleDescDoubleLinecomponent still usesMaterialTheme.typography.labelMediuminternally for the title text (see line 72 of MifosTexts.kt in relevant snippets). This creates a mixed-theme state that doesn't fully achieve the PR's objective of replacing MaterialTheme with KptTheme.As noted in the previous review, the component should accept a
titleStyleparameter to allow full theme migration. Based on learnings, this issue was already flagged in a previous review.Also applies to: 50-50, 58-58, 66-66
🧹 Nitpick comments (9)
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/recoverPassword/RecoverPasswordScreen.kt (1)
141-141: Consider migrating remaining hardcoded spacing values for consistency.For consistency with the spacing migration at line 133, consider replacing the remaining hardcoded dp values with DesignToken equivalents (e.g.,
24.dpmight map toDesignToken.spacing.largeor similar, and6.dpmight map toDesignToken.spacing.small).This would complete the spacing migration and improve maintainability.
Also applies to: 244-244
feature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/components/ClientChargeItem.kt (2)
75-79: Consider consolidating the theming approach.The file now uses three different theme systems:
KptTheme.colorScheme.*(lines 75, 79, 125, 150)MifosTypography.*(lines 91, 96, 104, 121, 146)AppColors.*(lines 123, 148)While this may be intentional during a gradual migration, consider whether typography and custom colors should also be migrated to KptTheme for consistency, or document the intended theming strategy.
Also applies to: 91-91, 121-126, 146-150
163-172: Consider updating the preview theme wrapper.The preview wraps the component in
MifosMobileTheme, while the component itself now usesKptThemefor color scheme properties. This may cause the preview to display differently from the actual runtime appearance.Verify whether the preview should use a KptTheme-compatible wrapper to ensure accurate preview rendering.
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt (2)
186-230: Consider using semantic color tokens for transaction amounts.The hardcoded colors
Color(0xFF0A7E48)(credit/green) andColor.Red(debit) should ideally use KptTheme color tokens to maintain theming consistency.💡 Consider this approach if semantic tokens exist
-val amountColor = if (isCredit) Color(0xFF0A7E48) else Color.Red +val amountColor = if (isCredit) KptTheme.colorScheme.success else KptTheme.colorScheme.errorIf
successcolor is not available inKptTheme.colorScheme, verify whether these semantic colors should be added to the design system or if they're intentionally custom to this feature.
393-415: Hardcoded background color in dropdown menu.Line 398 uses
Color.Whitefor the dropdown background instead of a KptTheme color token.🔎 Use theme color for dropdown background
-.background(Color.White), +.background(KptTheme.colorScheme.surface),feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt (1)
158-158: Consider using DesignToken for remaining hardcoded spacing.Line 158 still uses a hardcoded
24.dpvalue. If a DesignToken spacing equivalent exists (e.g.,DesignToken.spacing.largeor similar), consider replacing it for consistency with the migration at line 150.feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/component/RepaymentPeriodCard.kt (1)
84-88: Consider using DesignToken for border width.The border width uses hardcoded
1.dpwhile other sizing/spacing values have been migrated to DesignToken. If a suitable DesignToken for border width exists (e.g.,DesignToken.borderWidth.thinor similar), consider using it for consistency.feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/LoanCard.kt (1)
106-107: Consider resolving the TODO comment as part of this theming migration.Since this PR migrates to
KptThemefor color scheme, this might be an opportunity to address the TODO comment about finding the appropriate color from the theme instead of hardcodingColor.White.Would you like me to help identify the appropriate color token from
KptTheme.colorSchemeorAppColorsthat matches the Figma design? I can open a new issue to track this if you'd prefer to address it separately.feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/CropContent.kt (1)
78-78: Consider applying KptTheme typography for consistency.The "No image selected !" text doesn't have a style applied. For consistent theming across the UI, consider using an appropriate KptTheme typography style (e.g.,
bodyMediumorbodyLarge).🔎 Suggested change:
- Text("No image selected !") + Text( + text = "No image selected !", + style = KptTheme.typography.bodyMedium, + )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginScreen.kt(6 hunks)feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/recoverPassword/RecoverPasswordScreen.kt(6 hunks)feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/registration/RegistrationScreen.kt(6 hunks)feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt(4 hunks)feature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/components/ClientChargeItem.kt(4 hunks)feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorAdd/AddGuarantorScreen.kt(2 hunks)feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorDetails/GuarantorDetailContent.kt(2 hunks)feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorList/GuarantorListScreen.kt(3 hunks)feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/component/RepaymentPeriodCard.kt(5 hunks)feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/LoanCard.kt(7 hunks)feature/location/src/commonMain/kotlin/org/mifos/mobile/feature/location/LocationScreen.kt(2 hunks)feature/notification/src/commonMain/kotlin/org/mifos/mobile/feature/notification/NotificationScreen.kt(4 hunks)feature/onboarding-language/src/commonMain/kotlin/org/mifos/mobile/feature/onboarding/language/SetOnboardingLanguageScreen.kt(5 hunks)feature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/PasscodeScreen.kt(8 hunks)feature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/verifyPasscode/VerifyPasscodeScreen.kt(7 hunks)feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/CropContent.kt(3 hunks)feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/Dialogs.kt(2 hunks)feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt(15 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/faq/FaqScreen.kt(2 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/help/HelpScreen.kt(1 hunks)feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/passcode/UpdatePasscodeScreen.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- feature/location/src/commonMain/kotlin/org/mifos/mobile/feature/location/LocationScreen.kt
- feature/onboarding-language/src/commonMain/kotlin/org/mifos/mobile/feature/onboarding/language/SetOnboardingLanguageScreen.kt
- feature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/PasscodeScreen.kt
- feature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/verifyPasscode/VerifyPasscodeScreen.kt
- feature/notification/src/commonMain/kotlin/org/mifos/mobile/feature/notification/NotificationScreen.kt
- feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/registration/RegistrationScreen.kt
- feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/help/HelpScreen.kt
- feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorList/GuarantorListScreen.kt
- feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/faq/FaqScreen.kt
- feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/passcode/UpdatePasscodeScreen.kt
- feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/Dialogs.kt
- feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorAdd/AddGuarantorScreen.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorDetails/GuarantorDetailContent.kt (1)
core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/MifosTexts.kt (1)
MifosTextTitleDescDoubleLine(62-83)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR Checks KMP / Static Analysis Check
🔇 Additional comments (24)
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/recoverPassword/RecoverPasswordScreen.kt (3)
60-60: LGTM! Import correctly added.The KptTheme import is properly added to support the theming migration throughout the file.
130-130: LGTM! Color scheme migration completed correctly.All MaterialTheme color references have been successfully migrated to KptTheme with appropriate semantic color properties (onBackground for title, secondary for body/labels, error for validation icons, primary for emphasized links).
Also applies to: 138-138, 186-186, 211-211, 241-241, 250-250
133-133: Token migration is correct — DesignToken.spacing.medium equals 12.dp.The change from hardcoded
12.dpto the design token maintains the exact same spacing value, ensuring no visual changes to the layout.feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorDetails/GuarantorDetailContent.kt (3)
24-24: LGTM! Necessary imports added.The imports for
DesignTokenandKptThemeare correctly added to support the theme migration.Also applies to: 27-27
37-37: Change is correct and maintains visual consistency.The migration from hardcoded
16.dptoDesignToken.padding.largeis safe; the token is defined as exactly16.dpin the AppPadding data class. No visual regression will occur.
45-45: DesignToken.padding.small migration confirmed correct.The divider padding has been properly migrated from hardcoded
8.dptoDesignToken.padding.smallacross all four locations (lines 45, 53, 61, 69). The design token is correctly defined as8.dp, ensuring no visual regression. Migration follows the established pattern and is complete.feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginScreen.kt (2)
71-71: LGTM! Clean migration to design tokens.The KptTheme import and avatar size migration to
DesignToken.sizes.avatarMediumare consistent with the PR objectives.Also applies to: 207-207
219-219: LGTM! Consistent color scheme migration.All color references have been cleanly migrated to
KptTheme.colorSchemewith appropriate semantic tokens.Also applies to: 227-227, 258-258, 291-291, 328-328
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt (3)
60-60: LGTM! Clean theming migration.The imports and theming updates in the top app bar and filter sheet are consistent with the migration to KptTheme and DesignToken.
Also applies to: 67-67, 95-96, 102-104, 122-124, 164-164
257-297: LGTM! Consistent design token usage.The transaction list divider styling and error screen color correctly use KptTheme and DesignToken.
417-501: LGTM! Consistent design token usage.The Apply Filters button and filter chips correctly use KptTheme color schemes and DesignToken shapes/spacing throughout.
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt (4)
60-60: LGTM! Import addition is necessary for KptTheme usage.The import correctly supports the migration from MaterialTheme to KptTheme throughout this file.
155-155: LGTM! Color scheme migrations are consistent.The replacements of
MaterialTheme.colorScheme.secondary(lines 155, 263) andMaterialTheme.colorScheme.primary(line 272) with their KptTheme equivalents are correctly applied. Ensure color compatibility as noted in the earlier comment for line 147.Also applies to: 263-263, 272-272
150-150: DesignToken.spacing.medium correctly equals 12.dp. The replacement maintains visual consistency with the original hardcoded value and aligns with the PR's migration to design tokens.
147-147: KptTheme.colorScheme.onBackground is properly defined and compatible.The color value
Color(0xFF1C1B1F)matches Material 3 standards and is consistently applied throughout the codebase. The migration fromMaterialTheme.colorScheme.onBackgroundtoKptTheme.colorScheme.onBackgroundis correct with no visual regression risk.feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/component/RepaymentPeriodCard.kt (3)
48-48: LGTM: Import statement correctly added for KptTheme.The import aligns with the PR objective to migrate theming from MaterialTheme to KptTheme.
99-99: No action needed. The change from hardcoded36.dptoDesignToken.sizes.iconExtraLargeis correct—the design token is defined as36.dpand maintains the intended size for the period number indicator.
86-86: Color scheme migration verified and correct.KptTheme.colorScheme properly provides all referenced color properties (
secondaryContainer,primary,outline,onSurface,error) with Material3-compatible semantics. The migration from MaterialTheme.colorScheme is consistent throughout the file.feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/LoanCard.kt (4)
39-39: LGTM! Import addition supports the theming migration.The KptTheme import is appropriately added to support the migration from MaterialTheme to the design system's theming.
92-92: Verify DesignToken spacing values match original hardcoded values.The spacing token replacements require verification of the underlying DesignToken definitions to ensure visual consistency with original hardcoded values. Confirm that:
extraSmall= 4.dpsmall= 8.dplarge= 16.dpextraLargeIncreased= 24.dpThis applies to lines 92, 101, 178, and 186 in LoanCard.kt.
82-82: No action needed; preview modifier padding is separate from component content padding.Lines 82 and 170 (the actual component implementations) both consistently use
DesignToken.padding.largeIncreased(20.dp) for internal content padding. Lines 118 and 202 are in @Preview functions and useDesignToken.padding.large(16.dp) for the preview wrapper modifier—this is intentional preview display spacing, not component content padding. The apparent inconsistency does not exist in the production component code.Likely an incorrect or invalid review comment.
134-134: No issues identified. The default parameter change fromMaterialTheme.colorScheme.primarytoKptTheme.colorScheme.primaryis consistent with the app's design system.KptThemeis the established custom theme used throughout the codebase and is properly initialized in all contexts. The only usage ofLoanCardCustomis in the preview function, which correctly uses the new default without requiring any changes.feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/CropContent.kt (2)
87-90: Typography migration looks good.The change from
MaterialTheme.typography.labelLargetoKptTheme.typography.labelLargecorrectly follows the theming migration pattern.
66-66: Padding value migration is correct.The change from
16.dptoDesignToken.padding.largeis verified. The design token defineslargeas16.dp, maintaining the original spacing with no visual changes.
| import com.attafitamim.krop.ui.ImageCropperDialog | ||
| import org.mifos.mobile.core.designsystem.component.MifosButton | ||
| import org.mifos.mobile.core.designsystem.theme.DesignToken | ||
| import template.core.base.designsystem.theme.KptTheme |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the feature/qr module exists and its structure
find . -type f -name "CropContent.kt" 2>/dev/null | head -5Repository: openMF/mifos-mobile
Length of output: 141
🏁 Script executed:
# Find build.gradle files in feature/qr module
fd -t f 'build.gradle' feature/qr 2>/dev/null | head -10Repository: openMF/mifos-mobile
Length of output: 90
🏁 Script executed:
# Check the actual CropContent.kt file
fd -t f 'CropContent.kt' 2>/dev/null | xargs cat -n 2>/dev/nullRepository: openMF/mifos-mobile
Length of output: 4127
🏁 Script executed:
# Search for KptTheme definition in the codebase
rg -t kotlin 'KptTheme' --max-count 20 2>/dev/nullRepository: openMF/mifos-mobile
Length of output: 50375
🏁 Script executed:
# Read the feature/qr/build.gradle.kts file to verify dependencies
cat feature/qr/build.gradle.ktsRepository: openMF/mifos-mobile
Length of output: 1755
🏁 Script executed:
# Check another feature module's build.gradle.kts for comparison
cat feature/auth/build.gradle.kts | head -50Repository: openMF/mifos-mobile
Length of output: 1269
🏁 Script executed:
# Check core/qrcode/build.gradle.kts for dependencies
cat core/qrcode/build.gradle.kts | head -60Repository: openMF/mifos-mobile
Length of output: 1882
🏁 Script executed:
# Check core/ui/build.gradle.kts for dependencies
cat core/ui/build.gradle.kts | head -60Repository: openMF/mifos-mobile
Length of output: 2027
🏁 Script executed:
# Search for where core.designsystem or template.core.base.designsystem is explicitly used
rg "projects\.core\.designsystem" --type gradle | head -20Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
# Check if there's a convention plugin that adds designsystem automatically
fd -t f 'convention.gradle' -o -name '*convention*.gradle.kts' | xargs cat | grep -A5 -B5 designsystem | head -50Repository: openMF/mifos-mobile
Length of output: 293
🏁 Script executed:
# Fix the fd command - search for convention gradle files
fd 'convention' -t f | grep gradle | head -20Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
# Search for core-base/designsystem references
rg "core-base" --type gradle | head -20Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
# Check gradle settings file for module definitions
cat settings.gradle.kts | grep -i designsystemRepository: openMF/mifos-mobile
Length of output: 127
🏁 Script executed:
# Find which modules depend on core-base:designsystem
rg "projects\.core\.base\.designsystem\|projects\.coreba" --type gradle | head -20Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
# Check if cmp.feature.convention plugin includes core-base:designsystem
find . -name '*.gradle.kts' -path '*/buildLogic/*' -o -name '*.gradle.kts' -path '*/build/*' | xargs grep -l "cmp.feature" 2>/dev/null | head -5Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
# List all gradle plugin definitions
find . -type f -name '*.gradle.kts' -path '*/buildLogic/*' 2>/dev/null | head -10Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
# Check the build-logic directory structure
find . -type d -name buildLogic -o -name build-logic 2>/dev/nullRepository: openMF/mifos-mobile
Length of output: 76
🏁 Script executed:
# Look for gradle files in common locations
find . -path "*/gradle/*" -name "*.gradle.kts" 2>/dev/null | head -20Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
# Check settings.gradle.kts to see if it includes buildLogic
cat settings.gradle.kts | head -30Repository: openMF/mifos-mobile
Length of output: 842
🏁 Script executed:
# List contents of build-logic directory
ls -la build-logic/Repository: openMF/mifos-mobile
Length of output: 435
🏁 Script executed:
# Find gradle files in build-logic
find build-logic -name "*.gradle.kts" | head -20Repository: openMF/mifos-mobile
Length of output: 134
🏁 Script executed:
# Check the convention build.gradle.kts
cat build-logic/convention/build.gradle.ktsRepository: openMF/mifos-mobile
Length of output: 3978
🏁 Script executed:
# Look for plugin definitions in convention directory
find build-logic/convention -type f -name "*.kt" | head -20Repository: openMF/mifos-mobile
Length of output: 1531
🏁 Script executed:
# Check the CMPFeatureConventionPlugin implementation
cat build-logic/convention/src/main/kotlin/CMPFeatureConventionPlugin.ktRepository: openMF/mifos-mobile
Length of output: 3409
🏁 Script executed:
# Check if core-base:designsystem is included somewhere or if it's missing
rg "core.base.designsystem\|core-base.*designsystem" --type gradle | head -20Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
# Verify the package path of KptTheme definition
cat core-base/designsystem/src/commonMain/kotlin/template/core/base/designsystem/theme/KptColorSchemeImpl.kt | head -30Repository: openMF/mifos-mobile
Length of output: 1329
Add explicit dependency for KptTheme module.
The feature/qr module imports KptTheme from template.core.base.designsystem.theme, but the core-base:designsystem module is not declared as a dependency in build.gradle.kts. Add the explicit dependency to ensure proper module resolution and avoid relying on transitive dependencies:
implementation(projects.core.base.designsystem)🤖 Prompt for AI Agents
In feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/CropContent.kt
around line 36, the file imports KptTheme from
template.core.base.designsystem.theme but the feature/qr module doesn't declare
the core-base:designsystem dependency; open feature/qr/build.gradle.kts and add
implementation(projects.core.base.designsystem) inside the dependencies block
(alongside other implementation entries) so the module is explicitly referenced
and resolves correctly.
DesignToken ↔ KptTheme Parameter MappingThis document maps DesignToken parameters to their closest equivalents in KptTheme, focusing on Shapes, Spacing, Padding, Sizes and Elevations.
1. Shapes Mapping
Conclusion: 2. Spacing Mapping (Primary Shared Axis)
Conclusion: 3. Padding Mapping (DesignToken → KptTheme.spacing)
Conclusion: 4. Sizes Mapping
Conclusion: 5. Elevation MappingBoth systems define elevation, but they differ conceptually and semantically.
Elevation Token Comparison
Overall Summary
This explains why mixing |
biplab1
force-pushed
the
MM-389-replace-materialtheme-with-kptmaterialtheme
branch
from
a1e4296 to
2236195
Compare
biplab1
force-pushed
the
MM-389-replace-materialtheme-with-kptmaterialtheme
branch
from
60b5e8d to
74fd263
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/otpAuthentication/OtpAuthenticationScreen.kt (1)
265-265: Complete the spacing token migration.This line still uses
DesignToken.spacing.mediumwhile the rest of the file has migrated to KptTheme tokens. For consistency, useKptTheme.spacing.md.🔎 Proposed fix
Column( - verticalArrangement = Arrangement.spacedBy(DesignToken.spacing.medium), + verticalArrangement = Arrangement.spacedBy(KptTheme.spacing.md), ) {core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/DesignToken.kt (1)
426-441: MissingLocalStrokesinDesignTokenThemeprovider.
LocalStrokesis declared at line 386 but is not provided inDesignTokenTheme. This meansDesignToken.strokeswill always return the defaultAppStrokes()and cannot be customized via the theme. For consistency with other tokens, addLocalStrokesto the provider.🔎 Proposed fix
@Composable internal fun DesignTokenTheme( spacing: AppSpacing = AppSpacing(), padding: AppPadding = AppPadding(), shapes: AppShapes = AppShapes(), elevation: AppElevation = AppElevation(), sizes: AppSizes = AppSizes(), + strokes: AppStrokes = AppStrokes(), content: @Composable () -> Unit, ) { CompositionLocalProvider( LocalSpacing provides spacing, LocalPadding provides padding, LocalShapes provides shapes, LocalElevation provides elevation, LocalSizes provides sizes, + LocalStrokes provides strokes, content = content, ) }
♻️ Duplicate comments (3)
feature/onboarding-language/src/commonMain/kotlin/org/mifos/mobile/feature/onboarding/language/SetOnboardingLanguageScreen.kt (1)
169-171: Fix compilation error: missing parameter name in copy().The
copy()method requires the parameter namecolor =before the value. This issue was already flagged in a previous review.🔎 Apply this diff to fix the compilation error:
unselectedTextStyle = MifosTypography.titleSmallEmphasized.copy( - KptTheme.colorScheme.onSurface, + color = KptTheme.colorScheme.onSurface, ),feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/theme/ChangeThemeScreen.kt (1)
111-113: Critical: Missing parameter name in.copy()call.This issue was already flagged in a previous review: Line 112 is missing the
color =parameter name. The.copy()method onTextStylerequires named parameters in Compose, which will cause a compilation error.🔎 Apply this diff to fix the syntax error
unselectedTextStyle = MifosTypography.titleSmallEmphasized.copy( - KptTheme.colorScheme.onSurface, + color = KptTheme.colorScheme.onSurface, ),feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/CropContent.kt (1)
36-36: Dependency issue already flagged in previous review.As noted in the prior review, the
feature/qrmodule importsKptThemebut lacks an explicit dependency oncore-base:designsystemin itsbuild.gradle.kts. Please ensure this dependency is added.
🧹 Nitpick comments (17)
feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/TermsAndConditionItem.kt (1)
36-50: Consider simplifying the spacing pattern.The Column uses
Arrangement.spacedByfor inter-child spacing and then adds a trailingSpacerfor bottom padding. While functional, this pattern mixes two spacing approaches.Consider one of these alternatives for clarity:
- Apply bottom padding directly to the Column modifier:
modifier.padding(bottom = KptTheme.spacing.sm)- Let the parent layout handle spacing between terms items
🔎 Alternative approach using padding
Column( - modifier = modifier, + modifier = modifier.padding(bottom = KptTheme.spacing.sm), verticalArrangement = Arrangement.spacedBy(KptTheme.spacing.sm), ) { Text( text = stringResource(title), style = MifosTypography.titleSmallEmphasized, ) Text( text = stringResource(description), style = MifosTypography.bodySmall, ) - Spacer(modifier = Modifier.height(KptTheme.spacing.sm)) }feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountUpdate/AccountUpdateScreen.kt (1)
192-194: Consider aligning padding tokens for consistency.Line 192 migrates to
KptTheme.spacing.md, but line 193 still usesDesignToken.padding.mediumfor top padding. This mixes both token systems in sequential modifiers. Since the PR objectives indicate padding should be mapped toKptTheme.spacing(as KptTheme lacks a dedicated padding system), consider migrating line 193 as well for consistency.🔎 Suggested alignment
If the intent is to apply uniform spacing on all sides plus additional top padding, and both values can be expressed via KptTheme.spacing:
.padding(KptTheme.spacing.md) -.padding(top = DesignToken.padding.medium) +.padding(top = KptTheme.spacing.md)Alternatively, if the top padding intentionally requires a value not available in KptTheme (e.g., 12dp or 20dp per PR constraints), then retaining
DesignToken.padding.mediumis appropriate and this comment can be disregarded.feature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/verifyPasscode/VerifyPasscodeScreen.kt (1)
111-111: Consider usingKptTheme.spacing.lgfor consistency.Per PR objectives,
KptTheme.spacing.lgmaps to 24dp. UsingKptTheme.spacing.lghere would maintain consistency with other spacing tokens in this file that use KptTheme.🔎 Suggested change
- Spacer(modifier = Modifier.height(DesignToken.spacing.dp24)) + Spacer(modifier = Modifier.height(KptTheme.spacing.lg))feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/otpAuthentication/OtpAuthenticationScreen.kt (1)
282-282: Inconsistent typography token usage.This is the only text style in the file that uses
KptTheme.typography.labelLargewhile all other text styles useMifosTypography.*(lines 159, 167, 175, 214, 243, 298, 310, 322). Consider usingMifosTypography.labelLargefor consistency unless there's a specific reason to prefer KptTheme here.🔎 Proposed fix for consistency
Text( text = stringResource(Res.string.feature_common_cancel), - style = KptTheme.typography.labelLarge, + style = MifosTypography.labelLarge, )feature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/PasscodeScreen.kt (1)
118-118: UseKptTheme.spacing.lgfor consistency.The rest of the file uses
KptTheme.spacing.*tokens, but this line usesDesignToken.spacing.dp24. Per the mapping discussion,KptTheme.spacing.lgequals 24dp, so this can be unified.🔎 Suggested fix
- Spacer(modifier = Modifier.height(DesignToken.spacing.dp24)) + Spacer(modifier = Modifier.height(KptTheme.spacing.lg))feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryApplication/BeneficiaryApplicationContent.kt (1)
159-159: Optional: Consider typography consistency.Line 159 correctly retains
DesignToken.sizes.buttonHeightsince KptTheme lacks size tokens (as documented in PR comments).However, line 182 still uses
MifosTypographyrather thanKptTheme.typography. If the PR goal includes migrating typography references to KptTheme, this instance may have been missed. IfMifosTypographyalready delegates to KptTheme or if typography migration is out of scope here, this can be ignored.Also applies to: 182-182
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt (4)
211-211: Inconsistent token usage for padding.The horizontal padding uses
KptTheme.spacing.md(16dp) while the vertical padding usesDesignToken.padding.medium(12dp). While this may be intentional for asymmetric padding, mixing token sources for the same modifier type can reduce maintainability. Consider using consistent token sources or adding a comment clarifying the intent.
377-377: Hardcoded colors may break dark mode support.
Color.Whiteis hardcoded for the card container. This could cause visual issues in dark mode and is inconsistent with the theming migration. Consider usingKptTheme.colorScheme.surfaceorKptTheme.colorScheme.surfaceContainerinstead.🔎 Proposed fix
- colors = CardDefaults.cardColors(containerColor = Color.White), + colors = CardDefaults.cardColors(containerColor = KptTheme.colorScheme.surface),
403-403: HardcodedColor.Blackfor icon tint.Similar to the card color, using a hardcoded black tint may cause accessibility issues in dark mode. Consider using
KptTheme.colorScheme.onSurfacefor proper theme adaptation.🔎 Proposed fix
- tint = Color.Black, + tint = KptTheme.colorScheme.onSurface,
413-413: Hardcoded background color in DropdownMenu.The
Color.Whitebackground should be replaced with a theme-aware color for dark mode compatibility.🔎 Proposed fix
- .background(Color.White), + .background(KptTheme.colorScheme.surface),core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/DesignToken.kt (2)
330-330: Unusual fractional dp value.
boxDp107 = 107.33333.dpis an unusual fractional value. While Compose handles this, fractional dp values can cause subpixel rendering inconsistencies across different screen densities. Consider rounding to a whole number (107.dp) unless there's a specific design requirement.
99-104: Consider semantic naming for custom tokens.The new custom tokens use numeric naming (e.g.,
dp6,dp24,imageDp48). While this is clear for values, it couples the token name to its implementation. If the design system evolves and values change, the names become misleading. Consider adding semantic aliases for frequently-used custom tokens (e.g.,formFieldSpacingfordp24) in future iterations.Also applies to: 152-156, 212-215, 254-255, 316-332
feature/share-account/src/commonMain/kotlin/org/mifos/mobile/feature/shareaccount/shareAccount/ShareAccountScreen.kt (1)
254-254: Consider using KptTheme.spacing.md for consistency.Line 254 uses
DesignToken.spacing.medium(16dp), while line 200 migrated toKptTheme.spacing.md(also 16dp). For consistency with the KptTheme migration strategy in this file, consider replacing this withKptTheme.spacing.md.🔎 Proposed change
- Spacer(modifier = Modifier.height(DesignToken.spacing.medium)) + Spacer(modifier = Modifier.height(KptTheme.spacing.md))feature/qr/src/commonMain/kotlin/org/mifos/mobile/feature/qr/qr/QrCodeReaderScreen.kt (3)
148-149: Consider consolidating padding modifiers for clarity.The current pattern applies padding to all sides, then adds additional top padding via a second modifier. While functionally correct, it can be less obvious at a glance. Consider using a single padding call with explicit side parameters if the intent is asymmetric padding.
🔎 Alternative approach
- .padding(KptTheme.spacing.md) - .padding(top = KptTheme.spacing.md), + .padding( + horizontal = KptTheme.spacing.md, + top = KptTheme.spacing.md * 2, + bottom = KptTheme.spacing.md + ),Note: This assumes the intent is 32dp top and 16dp on other sides. Adjust if different spacing is needed.
172-172: Transparent border appears unnecessary.The border is styled with
Color.Transparent, which renders it invisible and without functional purpose. Consider removing the border modifier entirely, or clarify the intent if this is needed for layout calculations or future styling.🔎 Proposed simplification
.clip(KptTheme.shapes.medium) .drawQrCorners() - .border(DesignToken.strokes.thin, Color.Transparent, KptTheme.shapes.medium),
244-253: TODO: Hardcoded dp values remain in drawQrCorners.The TODO comment indicates that replacing these hardcoded values (5dp, 40dp, 50dp, 200dp) with DesignToken values encountered "some error." These custom values don't align with standard design tokens and leave technical debt.
Consider either:
- Debugging the error to complete the token migration
- Adding custom tokens to DesignToken for these specific QR corner drawing values
- Documenting why these specific pixel values are required for QR code rendering
Do you want me to help investigate potential token replacements or create a follow-up issue to track this work?
feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccount/LoanAccountScreen.kt (1)
285-293: Reconsider color semantics for withdrawn and matured loan statuses.The mapping of
KptTheme.colorScheme.error(Material Design red) toWITHDRAWNandMATUREDstatuses is semantically problematic:
WITHDRAWN("Withdrawn by applicant") represents a user action, not a system errorMATUREDrepresents normal loan completion, not a failure stateSimilarly,
KptTheme.colorScheme.onSurfaceis a text color meant for foreground content, not a status indicator. Consider using dedicated status colors (AppColors.customEnableor a neutral variant) that semantically convey withdrawn/matured as non-error states.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (71)
core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/DesignToken.ktfeature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accountTransactions/TransactionScreen.ktfeature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.ktfeature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/component/FilterSection.ktfeature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginScreen.ktfeature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/otpAuthentication/OtpAuthenticationScreen.ktfeature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/recoverPassword/RecoverPasswordScreen.ktfeature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/registration/RegistrationScreen.ktfeature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.ktfeature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/uploadId/UploadIdScreen.ktfeature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryApplication/BeneficiaryApplicationContent.ktfeature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryApplicationConfirmation/BeneficiaryApplicationConfirmationScreen.ktfeature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryDetail/BeneficiaryDetailContent.ktfeature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryList/BeneficiaryListScreen.ktfeature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/chargeDetails/ChargeDetailScreen.ktfeature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/charges/ClientChargeScreen.ktfeature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/components/ClientChargeItem.ktfeature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorAdd/AddGuarantorScreen.ktfeature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorDetails/GuarantorDetailContent.ktfeature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorList/GuarantorListScreen.ktfeature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.ktfeature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/components/BottomSheetContent.ktfeature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/component/AccountSummaryCard.ktfeature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/component/RepaymentPeriodCard.ktfeature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccount/LoanAccountScreen.ktfeature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountDetails/LoanAccountDetailsScreen.ktfeature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountRepaymentSchedule/RepaymentScheduleScreen.ktfeature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountSummary/AccountSummaryScreen.ktfeature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/ApplyLoanBottomBar.ktfeature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/ConfirmDetailsCard.ktfeature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/LoanCard.ktfeature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/component/TermsAndConditionItem.ktfeature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/confirmDetails/ConfirmDetailsScreen.ktfeature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/loanApplication/LoanApplyScreen.ktfeature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/loanProductDescription/LoanProductDetailsScren.ktfeature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/loanType/SelectLoanTypeSceen.ktfeature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/uploadDocs/UploadDocsScreen.ktfeature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/uploadDocs/component/BottomSheetContent.ktfeature/location/src/commonMain/kotlin/org/mifos/mobile/feature/location/LocationScreen.ktfeature/notification/src/commonMain/kotlin/org/mifos/mobile/feature/notification/NotificationScreen.ktfeature/onboarding-language/src/commonMain/kotlin/org/mifos/mobile/feature/onboarding/language/SetOnboardingLanguageScreen.ktfeature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/PasscodeScreen.ktfeature/passcode/src/commonMain/kotlin/org/mifos/mobile/feature/passcode/verifyPasscode/VerifyPasscodeScreen.ktfeature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/CropContent.ktfeature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/Dialogs.ktfeature/qr/src/commonMain/kotlin/org/mifos/mobile/feature/qr/qr/QrCodeReaderScreen.ktfeature/qr/src/commonMain/kotlin/org/mifos/mobile/feature/qr/qrCodeDisplay/QrCodeDisplayScreen.ktfeature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.ktfeature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccount/SavingsAccountScreen.ktfeature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountDetails/SavingsAccountDetailsScreen.ktfeature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountUpdate/AccountUpdateScreen.ktfeature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountWithdraw/AccountWithdrawScreen.ktfeature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/fillApplication/FillApplicationScreen.ktfeature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/savingsApplication/SavingsApplyScreen.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/about/AboutScreen.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/appInfo/AppInfoScreen.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/componenets/MifosLogoutDilaog.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/faq/FaqScreen.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/help/HelpScreen.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/language/LanguageScreen.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/passcode/UpdatePasscodeScreen.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/password/ChangePasswordScreen.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/settings/SettingsScreen.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/theme/ChangeThemeScreen.ktfeature/share-account/src/commonMain/kotlin/org/mifos/mobile/feature/shareaccount/shareAccount/ShareAccountScreen.ktfeature/share-application/src/commonMain/kotlin/org/mifos/mobile/feature/share/application/fillApplication/FillApplicationScreen.ktfeature/share-application/src/commonMain/kotlin/org/mifos/mobile/feature/share/application/shareApplication/ShareApplyScreen.ktfeature/status/src/commonMain/kotlin/org/mifos/mobile/feature/status/Components.ktfeature/third-party-transfer/src/commonMain/kotlin/org/mifos/mobile/feature/third/party/transfer/thirdPartyTransfer/TptScreen.ktfeature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/makeTransfer/MakeTransferScreen.ktfeature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessScreen.kt
🚧 Files skipped from review as they are similar to previous changes (26)
- feature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/chargeDetails/ChargeDetailScreen.kt
- feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/registration/RegistrationScreen.kt
- feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountDetails/LoanAccountDetailsScreen.kt
- feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryDetail/BeneficiaryDetailContent.kt
- feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/help/HelpScreen.kt
- feature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/fillApplication/FillApplicationScreen.kt
- feature/client-charge/src/commonMain/kotlin/org/mifos/mobile/feature/charge/components/ClientChargeItem.kt
- feature/location/src/commonMain/kotlin/org/mifos/mobile/feature/location/LocationScreen.kt
- feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/language/LanguageScreen.kt
- feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountWithdraw/AccountWithdrawScreen.kt
- feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccount/SavingsAccountScreen.kt
- feature/qr/src/commonMain/kotlin/org/mifos/mobile/feature/qr/qrCodeDisplay/QrCodeDisplayScreen.kt
- feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/uploadId/UploadIdScreen.kt
- feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorAdd/AddGuarantorScreen.kt
- feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryList/BeneficiaryListScreen.kt
- feature/share-application/src/commonMain/kotlin/org/mifos/mobile/feature/share/application/fillApplication/FillApplicationScreen.kt
- feature/third-party-transfer/src/commonMain/kotlin/org/mifos/mobile/feature/third/party/transfer/thirdPartyTransfer/TptScreen.kt
- feature/savings-account/src/commonMain/kotlin/org/mifos/mobile/feature/savingsaccount/savingsAccountDetails/SavingsAccountDetailsScreen.kt
- feature/guarantor/src/commonMain/kotlin/org/mifos/mobile/feature/guarantor/screens/guarantorList/GuarantorListScreen.kt
- feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/makeTransfer/MakeTransferScreen.kt
- feature/qr/src/androidMain/kotlin/org/mifos/mobile/feature/qr/Dialogs.kt
- feature/notification/src/commonMain/kotlin/org/mifos/mobile/feature/notification/NotificationScreen.kt
- feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/uploadDocs/component/BottomSheetContent.kt
- feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/about/AboutScreen.kt
- feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/component/RepaymentPeriodCard.kt
- feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/component/FilterSection.kt
| .padding(KptTheme.spacing.md) | ||
| .padding(top = KptTheme.spacing.md), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git log -p --all -- feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.kt | grep -B 5 -A 15 "padding" | head -100Repository: openMF/mifos-mobile
Length of output: 182
🏁 Script executed:
cat -n feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.kt | sed -n '140,160p'Repository: openMF/mifos-mobile
Length of output: 885
🏁 Script executed:
# Also search for similar double-padding patterns in the codebase
rg '\.padding\(' feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.kt -A 1 -B 1Repository: openMF/mifos-mobile
Length of output: 212
🏁 Script executed:
# Find KptTheme.spacing definitions to confirm spacing values
fd -e kt -e kts | xargs grep -l "spacing\s*=" | head -5Repository: openMF/mifos-mobile
Length of output: 448
🏁 Script executed:
grep -n "spacing" core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Theme.kt | head -20Repository: openMF/mifos-mobile
Length of output: 168
🏁 Script executed:
grep -n "val md\|spacing.md\|\.md\s*=" core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/DesignToken.kt | head -30Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
# Search for the specific pattern of double padding in the codebase
rg '\.padding\([^)]+\)\s*\.\s*padding\(' feature/ core/ -A 2 -B 2 | head -50Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
cat -n core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/DesignToken.kt | head -150Repository: openMF/mifos-mobile
Length of output: 6504
🏁 Script executed:
# Look for where spacing values are defined
rg "val\s+spacing|object.*spacing" core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/ -A 5Repository: openMF/mifos-mobile
Length of output: 772
🏁 Script executed:
# Search for similar Column padding patterns in feature/accounts
rg "Column\(" feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/ -A 8 | head -80Repository: openMF/mifos-mobile
Length of output: 10825
🏁 Script executed:
rg "object KptTheme|val spacing" core/designsystem/ -A 3 | head -40Repository: openMF/mifos-mobile
Length of output: 532
🏁 Script executed:
# Search for KptTheme in the entire codebase
fd -e kt | xargs grep -l "object KptTheme" | head -5Repository: openMF/mifos-mobile
Length of output: 167
🏁 Script executed:
# Look for md property definition
rg "\.md\s*=" core/ feature/ -B 2 -A 1 | grep -A 3 "spacing"Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
find . -name "*.kt" -type f | xargs grep -l "KptTheme\s*=" | head -10Repository: openMF/mifos-mobile
Length of output: 45
🏁 Script executed:
# Search for KptTheme directly
rg "KptTheme\s*=" -t kt | head -20Repository: openMF/mifos-mobile
Length of output: 89
🏁 Script executed:
# Look for where md is defined in spacing context
rg "val md\|\.md\s*=" -t kt -B 2 | grep -B 5 "spacing" | head -40Repository: openMF/mifos-mobile
Length of output: 89
🏁 Script executed:
# Find where KptTheme is imported/defined
grep -r "KptTheme" feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.kt | head -5Repository: openMF/mifos-mobile
Length of output: 207
🏁 Script executed:
# Get imports from AccountsScreen.kt
head -50 feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.kt | grep -i "import"Repository: openMF/mifos-mobile
Length of output: 2392
🏁 Script executed:
# Find KptTheme definition anywhere in the repo
find . -type f -name "*.kt" | xargs grep "object KptTheme\|val KptTheme\|KptTheme =" 2>/dev/null | head -10Repository: openMF/mifos-mobile
Length of output: 187
🏁 Script executed:
cat core-base/designsystem/src/commonMain/kotlin/template/core/base/designsystem/theme/KptColorSchemeImpl.kt | head -200Repository: openMF/mifos-mobile
Length of output: 7654
Simplify redundant padding modifier.
The chained padding() calls result in inconsistent spacing: 32dp on top (16dp + 16dp) and 16dp on other sides. This pattern doesn't appear elsewhere in the codebase. Use a single .padding(KptTheme.spacing.md) call or explicitly specify all sides if 32dp top was intended: .padding(horizontal = KptTheme.spacing.md, vertical = KptTheme.spacing.xl).
🤖 Prompt for AI Agents
In
feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.kt
around lines 149-150, the chained .padding(KptTheme.spacing.md).padding(top =
KptTheme.spacing.md) creates inconsistent top spacing (double 16dp) — replace
the chained calls with a single .padding(KptTheme.spacing.md) to make all sides
consistent, or if the intent was a larger top padding, replace with an explicit
call such as .padding(horizontal = KptTheme.spacing.md, vertical =
KptTheme.spacing.xl) so spacing is unambiguous.
| Spacer(Modifier.height(DesignToken.spacing.largeIncreased)) | ||
|
|
||
| HorizontalDivider(modifier = Modifier.height(1.dp)) | ||
| HorizontalDivider(modifier = Modifier.height(DesignToken.strokes.thin)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find DesignToken file
fd -e kt -e kts | xargs rg -l "DesignToken" | grep -i "design\|token"Repository: openMF/mifos-mobile
Length of output: 758
🏁 Script executed:
#!/bin/bash
# Search for strokes definition in Kotlin files
rg -n "strokes" --type kotlin -C 3Repository: openMF/mifos-mobile
Length of output: 26698
🏁 Script executed:
#!/bin/bash
# Look for DesignToken object/class definition
fd -e kt | xargs rg -n "object DesignToken|class DesignToken" -A 20Repository: openMF/mifos-mobile
Length of output: 2589
🏁 Script executed:
cat -n core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/DesignToken.ktRepository: openMF/mifos-mobile
Length of output: 19868
DesignToken.strokes.thin is properly defined with value 1.dp. The migration from hard-coded sizing is a good semantic improvement.
However, note that LocalStrokes is not provided in the DesignTokenTheme composable (lines 434-441), unlike other design tokens. This means stroke values cannot be customized through the theme, though the defaults will still function correctly.
🤖 Prompt for AI Agents
In
feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.kt
around line 167, you use DesignToken.strokes.thin but the DesignTokenTheme
(lines 434-441) does not provide LocalStrokes, so stroke values can't be
overridden via the theme; fix by updating the DesignTokenTheme composable to
provide LocalStrokes (backed by the current theme strokes instance), add a
strokes parameter/default to DesignTokenTheme so callers can override strokes,
and wrap content with CompositionLocalProvider(LocalStrokes provides strokes)
while keeping the existing defaults unchanged.
| style = MifosTypography.headlineMedium, | ||
| ) | ||
|
|
||
| Spacer(modifier = Modifier.height(DesignToken.spacing.medium)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inconsistent spacing token usage.
These lines still use DesignToken.spacing.medium while the rest of the file has been migrated to KptTheme.spacing.md for equivalent spacing. This creates inconsistency in the design system migration.
🔎 Proposed fix
- Spacer(modifier = Modifier.height(DesignToken.spacing.medium))
+ Spacer(modifier = Modifier.height(KptTheme.spacing.md))Apply the same change to both occurrences at lines 162 and 170.
Also applies to: 170-170
🤖 Prompt for AI Agents
In
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/otpAuthentication/OtpAuthenticationScreen.kt
around lines 162 and 170, the Spacer usages still reference
DesignToken.spacing.medium while the rest of the file uses KptTheme.spacing.md;
update both occurrences to use KptTheme.spacing.md to make spacing tokens
consistent with the migrated design system.
| ) | ||
|
|
||
| Spacer(modifier = Modifier.height(24.dp)) | ||
| Spacer(modifier = Modifier.height(DesignToken.spacing.dp24)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use KptTheme.spacing.lg for 24dp spacing.
According to the PR objectives, DesignToken.spacing.dp24 (24dp) maps directly to KptTheme.spacing.lg. This line should be migrated to use the KptTheme equivalent.
🔎 Proposed fix
- Spacer(modifier = Modifier.height(DesignToken.spacing.dp24))
+ Spacer(modifier = Modifier.height(KptTheme.spacing.lg))📝 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.
| Spacer(modifier = Modifier.height(DesignToken.spacing.dp24)) | |
| Spacer(modifier = Modifier.height(KptTheme.spacing.lg)) |
🤖 Prompt for AI Agents
In
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/otpAuthentication/OtpAuthenticationScreen.kt
around line 178, replace the hardcoded 24dp spacing that uses
DesignToken.spacing.dp24 with the theme token KptTheme.spacing.lg; update the
Spacer modifier to use KptTheme.spacing.lg so the component follows the theme
spacing mapping instead of the DesignToken constant.
| ) | ||
|
|
||
| Spacer(modifier = Modifier.height(12.dp)) | ||
| Spacer(modifier = Modifier.height(DesignToken.spacing.medium)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find DesignToken and KptTheme spacing definitions to verify consistency
echo "=== Looking for DesignToken definitions (any file) ==="
fd -e kt | head -20 | xargs rg -l "DesignToken" | head -5
echo -e "\n=== Looking for spacing token definitions ==="
rg -nP --type=kt "object\s+DesignToken|class\s+DesignToken" -A 50 | head -100
echo -e "\n=== Searching for 'spacing' property in design tokens ==="
rg -nP --type=kt "val spacing|object.*[Ss]pacing" | head -20
echo -e "\n=== Looking for KptTheme definition ==="
rg -nP --type=kt "object\s+KptTheme|class\s+KptTheme" -A 30 | head -80Repository: openMF/mifos-mobile
Length of output: 492
🏁 Script executed:
#!/bin/bash
# Description: Find DesignToken and KptTheme spacing definitions
echo "=== Finding all DesignToken files ==="
fd "\.kt$" | xargs rg -l "object DesignToken|class DesignToken" 2>/dev/null | head -5
echo -e "\n=== Finding all KptTheme files ==="
fd "\.kt$" | xargs rg -l "object KptTheme|class KptTheme" 2>/dev/null | head -5
echo -e "\n=== Searching for spacing definitions in common theme/design ==="
fd "\.kt$" "design\|theme" | head -10
echo -e "\n=== Looking for SetPasswordScreen.kt file ==="
fd "SetPasswordScreen.kt"
echo -e "\n=== Searching for 'spacing' definitions across codebase ==="
fd "\.kt$" | xargs rg -l "val spacing" 2>/dev/null | head -10Repository: openMF/mifos-mobile
Length of output: 1124
🏁 Script executed:
#!/bin/bash
# Read DesignToken.kt to see spacing definitions
echo "=== Reading DesignToken.kt ==="
cat core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/DesignToken.kt
echo -e "\n=== Searching for KptTheme object (not just KptColorSchemeImpl) ==="
fd "\.kt$" | xargs rg -l "object KptTheme" 2>/dev/null
echo -e "\n=== Searching for Spacing definitions in KptTheme area ==="
fd "\.kt$" "core-base/designsystem" | xargs rg "val spacing|object.*Spacing" 2>/dev/null | head -20Repository: openMF/mifos-mobile
Length of output: 17709
🏁 Script executed:
#!/bin/bash
# Find KptSpacing definitions to see what spacing values are available
echo "=== Reading KptColorSchemeImpl.kt to see spacing implementation ==="
cat core-base/designsystem/src/commonMain/kotlin/template/core/base/designsystem/theme/KptColorSchemeImpl.kt | head -200
echo -e "\n=== Searching for KptSpacingImpl ==="
fd "\.kt$" | xargs rg -l "class KptSpacingImpl|object KptSpacingImpl|data class KptSpacing"
echo -e "\n=== Looking for KptSpacing definition ==="
fd "\.kt$" | xargs rg "interface KptSpacing|class KptSpacing|data class KptSpacing" -A 30 | head -100Repository: openMF/mifos-mobile
Length of output: 19849
🏁 Script executed:
#!/bin/bash
# Read SetPasswordScreen.kt to verify the exact spacing usage at lines 149, 157, 188, 265
echo "=== Reading SetPasswordScreen.kt lines around 149, 157, 188, 265 ==="
wc -l feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt
echo -e "\n=== Checking lines 140-160 (covering 149, 157) ==="
sed -n '140,160p' feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt
echo -e "\n=== Checking lines 180-195 (covering 188) ==="
sed -n '180,195p' feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt
echo -e "\n=== Checking lines 260-270 (covering 265) ==="
sed -n '260,270p' feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.ktRepository: openMF/mifos-mobile
Length of output: 2503
🏁 Script executed:
#!/bin/bash
# Final verification: confirm the exact spacing values match our understanding
echo "=== Summary of spacing definitions ==="
echo "DesignToken.spacing values:"
grep -A2 "val medium:\|val dp24:\|val largeIncreased:" core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/DesignToken.kt | head -10
echo -e "\nKptTheme.spacing values:"
grep "override val xs:\|override val sm:\|override val md:\|override val lg:" core-base/designsystem/src/commonMain/kotlin/template/core/base/designsystem/theme/KptColorSchemeImpl.kt | head -10
echo -e "\n=== Exact lines from SetPasswordScreen.kt ==="
echo "Line 149:"
sed -n '149p' feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt
echo "Line 157:"
sed -n '157p' feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt
echo "Line 188:"
sed -n '188p' feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt
echo "Line 265:"
sed -n '265p' feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.ktRepository: openMF/mifos-mobile
Length of output: 928
Migrate line 157 to KptTheme.spacing.lg for consistency with other spacing token usage.
Line 157 uses DesignToken.spacing.dp24 (24.dp), but KptTheme.spacing.lg also equals 24.dp and is already used elsewhere in the file (lines 188, 265). For consistency with the KptTheme migration pattern, change line 157 to KptTheme.spacing.lg.
Line 149 correctly retains DesignToken.spacing.medium (12.dp) since KptTheme lacks a 12dp equivalent.
🤖 Prompt for AI Agents
In
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/setNewPassword/SetPasswordScreen.kt
around lines 149-157, replace the spacing token at line 157 that currently uses
DesignToken.spacing.dp24 (24.dp) with KptTheme.spacing.lg to match the rest of
the file and KptTheme usage; retain DesignToken.spacing.medium at line 149 as-is
since KptTheme has no 12dp equivalent.
| modifier = Modifier.padding(horizontal = KptTheme.spacing.md), | ||
| verticalArrangement = Arrangement.spacedBy(DesignToken.spacing.medium), |
There was a problem hiding this comment.
Inconsistent spacing token usage.
Line 72 uses KptTheme.spacing.md while line 73 still uses DesignToken.spacing.medium. For consistency with the migration pattern applied throughout this file, line 73 should also use KptTheme spacing.
🔎 Proposed fix for consistency
Column(
modifier = Modifier.padding(horizontal = KptTheme.spacing.md),
- verticalArrangement = Arrangement.spacedBy(DesignToken.spacing.medium),
+ verticalArrangement = Arrangement.spacedBy(KptTheme.spacing.md),
) {📝 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.
| modifier = Modifier.padding(horizontal = KptTheme.spacing.md), | |
| verticalArrangement = Arrangement.spacedBy(DesignToken.spacing.medium), | |
| Column( | |
| modifier = Modifier.padding(horizontal = KptTheme.spacing.md), | |
| verticalArrangement = Arrangement.spacedBy(KptTheme.spacing.md), | |
| ) { |
🤖 Prompt for AI Agents
In
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/components/BottomSheetContent.kt
around lines 72 to 73, the spacing tokens are inconsistent: the modifier uses
KptTheme.spacing.md while verticalArrangement uses DesignToken.spacing.medium;
update the verticalArrangement spacing to use KptTheme.spacing.md (or the
corresponding KptTheme.spacing value matching DesignToken.spacing.medium) so
both lines use the KptTheme spacing tokens consistently.
| label = stringResource(Res.string.feature_apply_loan_label_applicant_name), | ||
| shape = DesignToken.shapes.medium, | ||
| shape = KptTheme.shapes.medium, | ||
| textStyle = MifosTypography.bodyLarge, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for MifosTypography usages in feature modules
echo "=== MifosTypography usages in feature modules ==="
rg -nP 'MifosTypography\.' --type=kt -g 'feature/**' | head -30
echo -e "\n=== MifosTypography definition ==="
rg -nP 'object MifosTypography|class MifosTypography' --type=kt -A 10
echo -e "\n=== KptTheme typography definition ==="
rg -nP 'object KptTheme|class KptTheme' --type=kt -A 15 | head -50
echo -e "\n=== Check if MifosTypography imports or references KptTheme ==="
rg -nP 'MifosTypography' --type=kt -B 5 -A 5 | grep -E '(import|KptTheme|MaterialTheme)' | head -20Repository: openMF/mifos-mobile
Length of output: 378
🏁 Script executed:
#!/bin/bash
# Search for MifosTypography definition
echo "=== MifosTypography definition ==="
fd -e kt | xargs rg -l 'object MifosTypography|class MifosTypography'
fd -e kt | xargs rg -B 2 -A 15 'object MifosTypography|class MifosTypography'
echo -e "\n=== MifosTypography usages in feature modules ==="
fd -e kt feature/ | xargs rg 'MifosTypography\.' -c | head -20
echo -e "\n=== Check imports in LoanApplyScreen.kt ==="
fd -e kt -p 'LoanApplyScreen.kt' | xargs cat -n | head -100
echo -e "\n=== Search for KptTheme typography ==="
fd -e kt | xargs rg 'KptTheme\.typography|object KptTheme' -B 2 -A 10 | head -50Repository: openMF/mifos-mobile
Length of output: 17297
🏁 Script executed:
#!/bin/bash
# Get the file path for LoanApplyScreen.kt
FILE=$(fd -e kt -p 'LoanApplyScreen.kt')
echo "=== Lines around 216 in LoanApplyScreen.kt ==="
sed -n '210,225p' "$FILE" | cat -n
echo -e "\n=== Lines around 287 in LoanApplyScreen.kt ==="
sed -n '280,295p' "$FILE" | cat -n
echo -e "\n=== All typography usages in LoanApplyScreen.kt ==="
rg 'MifosTypography\.|KptTheme\.typography' "$FILE" -nRepository: openMF/mifos-mobile
Length of output: 2216
🏁 Script executed:
#!/bin/bash
# Get MifosTypography definition
echo "=== MifosTypography.bodyLarge definition ==="
fd -e kt 'Type.kt' core/designsystem | xargs grep -A 10 'val bodyLarge'
echo -e "\n=== KptTypography definition and bodyLarge ==="
fd -e kt | xargs grep -l 'class KptTypography\|object KptTypography' | head -3
fd -e kt | xargs grep -B 5 -A 10 'val bodyLarge' | grep -E '(KptTypography|bodyLarge|fontSize|lineHeight)' | head -20Repository: openMF/mifos-mobile
Length of output: 2045
🏁 Script executed:
#!/bin/bash
# Get KptTheme.typography.labelLarge definition
echo "=== KptTheme.typography.labelLarge definition ==="
fd -e kt | xargs grep -B 2 -A 5 'val labelLarge' | grep -E '(labelLarge|fontSize|lineHeight)' | head -15
# Get MifosTypography.labelLarge definition
echo -e "\n=== MifosTypography.labelLarge definition ==="
fd -e kt 'Type.kt' core/designsystem | xargs grep -A 5 'val labelLarge'
# Check if bodyLarge and labelLarge are different
echo -e "\n=== Compare MifosTypography styles ==="
fd -e kt 'Type.kt' core/designsystem | xargs grep -E 'val (bodyLarge|labelLarge|labelSmall)' -A 6 | grep -E '(val|fontSize|lineHeight)' | head -20Repository: openMF/mifos-mobile
Length of output: 2405
Migrate MifosTypography to KptTheme typography for consistency.
Line 216 uses MifosTypography.bodyLarge while line 287 uses KptTheme.typography.labelLarge. The file mixes typography from two different design systems. Per the PR objectives to replace MaterialTheme with KptTheme, update line 216 to use KptTheme.typography.bodyLarge to maintain consistency within the file.
🤖 Prompt for AI Agents
In
feature/loan-application/src/commonMain/kotlin/org/mifos/mobile/feature/loan/application/loanApplication/LoanApplyScreen.kt
around line 216, replace the direct use of MifosTypography.bodyLarge with
KptTheme.typography.bodyLarge to keep typography consistent with the rest of the
file; update the import if necessary and ensure any references to
MifosTypography are swapped to KptTheme.typography so the component uses the
KptTheme design system.
| style = MifosTypography.bodySmall, | ||
| ) | ||
| Spacer(modifier = Modifier.height(8.dp)) | ||
| Spacer(modifier = Modifier.height(KptTheme.spacing.sm)) |
There was a problem hiding this comment.
Fix incorrect Spacer modifier in Row layout.
The Spacer uses height modifier inside a Row, which creates vertical spacing rather than horizontal spacing. To add horizontal space between the two Text elements, use width instead.
🔎 Proposed fix
- Spacer(modifier = Modifier.height(KptTheme.spacing.sm))
+ Spacer(modifier = Modifier.width(KptTheme.spacing.sm))🤖 Prompt for AI Agents
In
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/faq/FaqScreen.kt
around line 156, the Spacer inside a Row is using Modifier.height which creates
vertical space; replace Modifier.height(KptTheme.spacing.sm) with
Modifier.width(KptTheme.spacing.sm) so the spacing between the two Text
composables is horizontal; ensure the correct width modifier is used from
androidx.compose.ui.Modifier (no other changes needed).
3 participants
Fixes - Jira-#MM-389
Didn't create a Jira ticket, click here to create new.
Summary of changes:
Please make sure these boxes are checked before submitting your pull request - thanks!
Run the static analysis check
./gradlew checkorci-prepush.shto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.
Summary by CodeRabbit
Style
✏️ Tip: You can customize this high-level summary in your review settings.