-
Notifications
You must be signed in to change notification settings - Fork 671
fix(client): Added Create Account buttons for all types of accounts. #2583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a createAccount callback across multiple account screens/routes, introduces AddAccount actions/events in several ViewModels, wires header Add buttons and empty-state CTA to dispatch AddAccount, and routes those events to navigation helpers that open the appropriate create-account flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Screen as AccountScreen
participant VM as ViewModel
participant Nav as NavController
User->>Screen: tap Add button / empty-state CTA
Screen->>VM: onAction(AddAccount)
VM->>VM: emit Event.AddAccount(clientId)
VM->>Screen: sendEvent(AddAccount(clientId))
Screen->>Nav: createAccount(clientId)
Nav->>Nav: navigateToCreate<X>AccountRoute(clientId)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/recurringDepositAccount/RecurringDepositAccountScreen.kt (1)
244-248: Pre-existing bug:ApproveAccountaction is not dispatched.The
ApproveAccountaction is constructed but never sent viaonAction(), so the approve functionality won't work. Compare withViewAccount(lines 237-243) which correctly callsonAction(...).🐛 Proposed fix
is Actions.ApproveAccount -> { - RecurringDepositAccountAction.ApproveAccount( + onAction(RecurringDepositAccountAction.ApproveAccount( recurringDeposit.accountNo ?: "", - ) + )) }
🤖 Fix all issues with AI agents
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.kt`:
- Around line 242-249: Replace the null content description on the add-icon
button so screen readers can announce it: in the Icon inside IconButton (the one
invoking onAction(ClientLoanAccountsAction.AddAccount) and using
painterResource(Res.drawable.add_icon)), set contentDescription to a meaningful
string (preferably via a string resource, e.g.
stringResource(R.string.add_account) or a localized label like "Add account")
instead of null so the button is accessible.
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/fixedDepositAccount/FixedDepositAccountScreen.kt`:
- Around line 13-14: The Add icon in FixedDepositAccountScreen (the clickable
add icon near where the composable shows an empty state) has a null
contentDescription, making it inaccessible; update the composable that renders
the add_icon (in FixedDepositAccountScreen) to pass a meaningful, localized
contentDescription string (create a new string resource like "add_fixed_deposit"
or reuse an appropriate existing resource) and wire it via stringResource(...)
so screen readers announce the action; ensure the contentDescription is non-null
for the Image/Icon composable and update any related preview/tests to reflect
the new resource.
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/savingsAccounts/SavingsAccounts.kt`:
- Around line 254-262: Remove the no-op DesignToken.padding expression and
replace it with a proper Spacer using Modifier.width or Modifier.padding as
appropriate (replace the standalone DesignToken.padding near the IconButton);
also fix accessibility by providing a meaningful contentDescription for the Icon
(do not use contentDescription = null) — either hardcode a short label like "Add
account" or reference a string resource, and ensure the IconButton callback
remains onAction.invoke(SavingsAccountAction.AddAccount).
🧹 Nitpick comments (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/shareAccounts/ShareAccountsScreen.kt (1)
186-202: Consider adding meaningfulcontentDescriptionfor accessibility.The new Add icon (and the existing Search/Filter icons) have
contentDescription = null, which makes them inaccessible to screen readers. For interactive icons, providing descriptive labels improves accessibility.♿ Suggested improvement for accessibility
Icon( modifier = Modifier.onClick { onAction.invoke(ShareAccountsAction.ToggleSearchBar) }, painter = painterResource(Res.drawable.search), - contentDescription = null, + contentDescription = "Search accounts", ) Spacer(modifier = Modifier.width(DesignToken.spacing.largeIncreased)) Icon( modifier = Modifier.onClick { onAction.invoke(ShareAccountsAction.AddAccount) }, painter = painterResource(Res.drawable.add_icon), - contentDescription = null, + contentDescription = "Add account", ) Spacer(modifier = Modifier.width(DesignToken.spacing.largeIncreased)) Icon( modifier = Modifier.onClick { onAction.invoke(ShareAccountsAction.ToggleFiler) }, painter = painterResource(Res.drawable.filter), - contentDescription = null, + contentDescription = "Filter accounts", )
| IconButton( | ||
| onClick = { onAction.invoke(ClientLoanAccountsAction.AddAccount) }, | ||
| ) { | ||
| Icon( | ||
| painter = painterResource(Res.drawable.add_icon), | ||
| contentDescription = null, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add content description for accessibility.
The contentDescription = null makes this button invisible to screen readers.
Proposed fix
IconButton(
onClick = { onAction.invoke(ClientLoanAccountsAction.AddAccount) },
) {
Icon(
painter = painterResource(Res.drawable.add_icon),
- contentDescription = null,
+ contentDescription = "Add Loan Account",
)
}📝 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.
| IconButton( | |
| onClick = { onAction.invoke(ClientLoanAccountsAction.AddAccount) }, | |
| ) { | |
| Icon( | |
| painter = painterResource(Res.drawable.add_icon), | |
| contentDescription = null, | |
| ) | |
| } | |
| IconButton( | |
| onClick = { onAction.invoke(ClientLoanAccountsAction.AddAccount) }, | |
| ) { | |
| Icon( | |
| painter = painterResource(Res.drawable.add_icon), | |
| contentDescription = "Add Loan Account", | |
| ) | |
| } |
🤖 Prompt for AI Agents
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.kt`
around lines 242 - 249, Replace the null content description on the add-icon
button so screen readers can announce it: in the Icon inside IconButton (the one
invoking onAction(ClientLoanAccountsAction.AddAccount) and using
painterResource(Res.drawable.add_icon)), set contentDescription to a meaningful
string (preferably via a string resource, e.g.
stringResource(R.string.add_account) or a localized label like "Add account")
instead of null so the button is accessible.
| import androidclient.feature.client.generated.resources.add_icon | ||
| import androidclient.feature.client.generated.resources.client_empty_card_message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a meaningful contentDescription for the new Add icon (accessibility).
The new clickable Add icon (Line 303) has a null contentDescription, which makes the action invisible to screen readers. Please add an accessible label (add a string resource if needed).
✅ Suggested fix
Icon(
painter = painterResource(Res.drawable.add_icon),
- contentDescription = null,
+ contentDescription = stringResource(Res.string.client_add_account),
modifier = Modifier.clickable {
addAccount.invoke()
},
)Also applies to: 303-309
🤖 Prompt for AI Agents
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/fixedDepositAccount/FixedDepositAccountScreen.kt`
around lines 13 - 14, The Add icon in FixedDepositAccountScreen (the clickable
add icon near where the composable shows an empty state) has a null
contentDescription, making it inaccessible; update the composable that renders
the add_icon (in FixedDepositAccountScreen) to pass a meaningful, localized
contentDescription string (create a new string resource like "add_fixed_deposit"
or reuse an appropriate existing resource) and wire it via stringResource(...)
so screen readers announce the action; ensure the contentDescription is non-null
for the Image/Icon composable and update any related preview/tests to reflect
the new resource.
| DesignToken.padding | ||
| IconButton( | ||
| onClick = { onAction.invoke(SavingsAccountAction.AddAccount) }, | ||
| ) { | ||
| Icon( | ||
| painter = painterResource(Res.drawable.add_icon), | ||
| contentDescription = null, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dead code and accessibility concerns.
-
Line 254 has a standalone
DesignToken.paddingexpression that has no effect. If spacing is intended, useSpacer(modifier = Modifier.width(...)). -
The
contentDescription = nullon the add icon button makes this control invisible to screen readers, which is an accessibility issue.
Proposed fix
- DesignToken.padding
IconButton(
onClick = { onAction.invoke(SavingsAccountAction.AddAccount) },
) {
Icon(
painter = painterResource(Res.drawable.add_icon),
- contentDescription = null,
+ contentDescription = "Add Savings Account",
)
}🤖 Prompt for AI Agents
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/savingsAccounts/SavingsAccounts.kt`
around lines 254 - 262, Remove the no-op DesignToken.padding expression and
replace it with a proper Spacer using Modifier.width or Modifier.padding as
appropriate (replace the standalone DesignToken.padding near the IconButton);
also fix accessibility by providing a meaningful contentDescription for the Icon
(do not use contentDescription = null) — either hardcode a short label like "Add
account" or reference a string resource, and ensure the IconButton callback
remains onAction.invoke(SavingsAccountAction.AddAccount).
niyajali
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these require changes in all places
| ) { | ||
| composable<ClientLoanAccountsRoute> { | ||
| composable<ClientLoanAccountsRoute> { backStackEntry -> | ||
| val route = backStackEntry.toRoute<ClientLoanAccountsRoute>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of collecting navigation data in this manner, leverage SavedStateHandle within your ViewModel. Retrieve the necessary data and transmit it via an event.
| is ClientLoanAccountsEvent.MakeRepayment -> makeRepayment(event.id) | ||
| ClientLoanAccountsEvent.NavigateBack -> navigateBack() | ||
| is ClientLoanAccountsEvent.ViewAccount -> viewAccount(event.id) | ||
| ClientLoanAccountsEvent.AddAccount -> createAccount() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of "Data Object" use "Data class AddAccount(val clientId: Int)" and pass in the lambda
| makeRepayment: (Int) -> Unit, | ||
| viewAccount: (Int) -> Unit, | ||
| navController: NavController, | ||
| createAccount: () -> Unit, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update this to "createAccount: (Int) -> Unit" to accept the Int parameter
|
@techsavvy185 could you also post images/videos of the outcome |
| ) | ||
| } | ||
|
|
||
| IconButton( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this toolbar icon instead update the card with a proper message with a button to create a new account
|
@niyajali Thanks for the review! I'll make the required changes. I've already attached a screen recording in the PR, is there anything specific I should cover? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/recurringDepositAccount/RecurringDepositAccountScreen.kt (1)
270-310: Add an accessible contentDescription for the new add button.The new clickable icon has
contentDescription = null, so screen readers can’t discover or announce the action. Please provide a localized label.♿ Suggested fix
Icon( painter = painterResource(Res.drawable.add_icon), - contentDescription = null, + contentDescription = stringResource(Res.string.client_add_account), modifier = Modifier.clickable { addAccount.invoke() }, )
🧹 Nitpick comments (4)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/shareAccounts/ShareAccountsViewModel.kt (1)
36-36: Minor style inconsistency:iskeyword used for data object.Other
data objectactions in thiswhenblock (e.g.,ToggleFiler,ToggleSearchBar,Refresh) use direct matching withoutis. For consistency, consider removing theiskeyword here.Suggested change
- is ShareAccountsAction.AddAccount -> sendEvent(ShareAccountsEvent.AddAccount(route.clientId)) + ShareAccountsAction.AddAccount -> sendEvent(ShareAccountsEvent.AddAccount(route.clientId))feature/client/src/commonMain/kotlin/com/mifos/feature/client/savingsAccounts/SavingsAccountsRoute.kt (1)
35-35: Simplify the pass-through lambda.The lambda wrapping is unnecessary since it just forwards the argument directly.
♻️ Suggested simplification
- createAccount = { clientId -> createAccount(clientId) }, + createAccount = createAccount,feature/client/src/commonMain/kotlin/com/mifos/feature/client/fixedDepositAccount/FixedDepositAccountScreen.kt (1)
267-274: Consider reordering parameters for consistency.The
addAccountparameter (without default) is placed aftermodifier(with default). While Kotlin's named arguments make this work, grouping parameters without defaults before those with defaults improves readability and aligns with common Compose conventions.♻️ Suggested parameter order
`@Composable` fun FixedDepositAccountHeader( totalItem: String, onToggleFilter: () -> Unit, + addAccount: () -> Unit, onToggleSearch: () -> Unit, modifier: Modifier = Modifier, - addAccount: () -> Unit, - onToggleSearch: () -> Unit, )feature/client/src/commonMain/kotlin/com/mifos/feature/client/shareAccounts/ShareAccountsScreen.kt (1)
192-196: Consider addingcontentDescriptionfor accessibility.Setting
contentDescription = nullmeans screen readers cannot describe this button. For better accessibility, provide a meaningful description.This also applies to the existing search and filter icons in this header.
♻️ Suggested improvement
Icon( modifier = Modifier.onClick { onAction.invoke(ShareAccountsAction.AddAccount) }, painter = painterResource(Res.drawable.add_icon), - contentDescription = null, + contentDescription = "Add Share Account", )
…ndition and also made adjustments in the account screens for the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/recurringDepositAccount/RecurringDepositAccountScreen.kt`:
- Around line 12-21: The icon usage (add_icon) in RecurringDepositAccountScreen
currently sets contentDescription = null and always renders a Spacer after the
icon, which removes the action from screen readers and leaves a gap when the
icon is hidden; update the composable(s) that render add_icon (e.g., the
icon/Image composable and its surrounding layout in
RecurringDepositAccountScreen and the similar block around the code referenced
at lines ~307-318) to: 1) provide a meaningful contentDescription using an
accessible string resource (e.g.,
Res.client_profile_recurring_deposit_account_title or a dedicated "add" label)
instead of null, and 2) render the Spacer conditionally only when the icon is
actually shown (wrap the Spacer in the same if that shows the icon) so no extra
gap appears when the icon is omitted. Ensure the accessible label is localized
via the existing resources import.
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/shareAccounts/ShareAccountsScreen.kt`:
- Around line 12-17: The use of add_icon currently sets contentDescription =
null (making it invisible to screen readers) and always renders a Spacer even
when the icon is hidden; update the Icon/Image usage for add_icon in
ShareAccountsScreen (and the similar nodes around the 192-206 block) to provide
an accessible label by passing a meaningful string resource (e.g., from Res like
feature_share_account_empty_list_message or a new localized string such as "add
share" via Res) into contentDescription instead of null, and render the Spacer
conditionally only when the add_icon is actually visible (wrap Spacer rendering
with the same visibility condition used for the icon).
🧹 Nitpick comments (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/fixedDepositAccount/FixedDepositAccountScreen.kt (1)
301-328: Consider aligning icon button patterns with SavingsAccountsHeader.The header icons use
Iconwithclickablemodifier, whileSavingsAccountsHeaderusesIconButton. For consistency across the codebase and better accessibility (proper touch targets), consider refactoring all interactive icons to useIconButton.Also note that search icon (line 303) and filter icon (line 324) have
contentDescription = null, though these lines aren't part of this PR's changes.
| import androidclient.feature.client.generated.resources.Res | ||
| import androidclient.feature.client.generated.resources.client_empty_card_message | ||
| import androidclient.feature.client.generated.resources.add_icon | ||
| import androidclient.feature.client.generated.resources.client_product_recurring_deposit_account | ||
| import androidclient.feature.client.generated.resources.client_profile_recurring_deposit_account_title | ||
| import androidclient.feature.client.generated.resources.client_savings_item | ||
| import androidclient.feature.client.generated.resources.client_savings_not_avilable | ||
| import androidclient.feature.client.generated.resources.client_savings_pending_approval | ||
| import androidclient.feature.client.generated.resources.feature_client_dialog_action_ok | ||
| import androidclient.feature.client.generated.resources.feature_recurring_account_empty_list_message | ||
| import androidclient.feature.client.generated.resources.filter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add icon needs an accessible label and conditional spacing.
contentDescription = null hides the action from screen readers, and the spacer after the conditional icon leaves a gap when the icon isn’t shown.
🔧 Suggested fix
import androidclient.feature.client.generated.resources.Res
import androidclient.feature.client.generated.resources.add_icon
+import androidclient.feature.client.generated.resources.client_apply_new_applications_recurring_account
import androidclient.feature.client.generated.resources.client_product_recurring_deposit_account
import androidclient.feature.client.generated.resources.client_profile_recurring_deposit_account_title
@@
- if (!isRecurringDepositScreenEmpty) {
- Icon(
- painter = painterResource(Res.drawable.add_icon),
- contentDescription = null,
- modifier = Modifier.clickable {
- addAccount.invoke()
- },
- )
- }
-
- Spacer(modifier = Modifier.width(DesignToken.spacing.largeIncreased))
+ if (!isRecurringDepositScreenEmpty) {
+ Icon(
+ painter = painterResource(Res.drawable.add_icon),
+ contentDescription = stringResource(
+ Res.string.client_apply_new_applications_recurring_account,
+ ),
+ modifier = Modifier.clickable { addAccount.invoke() },
+ )
+ Spacer(modifier = Modifier.width(DesignToken.spacing.largeIncreased))
+ }Also applies to: 307-318
🤖 Prompt for AI Agents
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/recurringDepositAccount/RecurringDepositAccountScreen.kt`
around lines 12 - 21, The icon usage (add_icon) in RecurringDepositAccountScreen
currently sets contentDescription = null and always renders a Spacer after the
icon, which removes the action from screen readers and leaves a gap when the
icon is hidden; update the composable(s) that render add_icon (e.g., the
icon/Image composable and its surrounding layout in
RecurringDepositAccountScreen and the similar block around the code referenced
at lines ~307-318) to: 1) provide a meaningful contentDescription using an
accessible string resource (e.g.,
Res.client_profile_recurring_deposit_account_title or a dedicated "add" label)
instead of null, and 2) render the Spacer conditionally only when the icon is
actually shown (wrap the Spacer in the same if that shows the icon) so no extra
gap appears when the icon is omitted. Ensure the accessible label is localized
via the existing resources import.
| import androidclient.feature.client.generated.resources.Res | ||
| import androidclient.feature.client.generated.resources.add_icon | ||
| import androidclient.feature.client.generated.resources.client_product_shares_account | ||
| import androidclient.feature.client.generated.resources.client_savings_item | ||
| import androidclient.feature.client.generated.resources.feature_share_account_empty_list_message | ||
| import androidclient.feature.client.generated.resources.filter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add icon needs an accessible label and conditional spacing.
contentDescription = null hides the action from screen readers, and the spacer should only appear when the icon is visible.
🔧 Suggested fix
import androidclient.feature.client.generated.resources.Res
import androidclient.feature.client.generated.resources.add_icon
+import androidclient.feature.client.generated.resources.client_apply_new_applications_share_account
import androidclient.feature.client.generated.resources.client_product_shares_account
import androidclient.feature.client.generated.resources.client_savings_item
@@
Spacer(modifier = Modifier.width(DesignToken.spacing.largeIncreased))
if (!isShareAccountsEmpty) {
Icon(
modifier = Modifier.onClick { onAction.invoke(ShareAccountsAction.AddAccount) },
painter = painterResource(Res.drawable.add_icon),
- contentDescription = null,
+ contentDescription = stringResource(
+ Res.string.client_apply_new_applications_share_account,
+ ),
)
+ Spacer(modifier = Modifier.width(DesignToken.spacing.largeIncreased))
}
- Spacer(modifier = Modifier.width(DesignToken.spacing.largeIncreased))Also applies to: 192-206
🤖 Prompt for AI Agents
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/shareAccounts/ShareAccountsScreen.kt`
around lines 12 - 17, The use of add_icon currently sets contentDescription =
null (making it invisible to screen readers) and always renders a Spacer even
when the icon is hidden; update the Icon/Image usage for add_icon in
ShareAccountsScreen (and the similar nodes around the 192-206 block) to provide
an accessible label by passing a meaningful string resource (e.g., from Res like
feature_share_account_empty_list_message or a new localized string such as "add
share" via Res) into contentDescription instead of null, and render the Spacer
conditionally only when the add_icon is actually visible (wrap Spacer rendering
with the same visibility condition used for the icon).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/recurringDepositAccount/RecurringDepositAccountScreen.kt (1)
249-255: Bug:ApproveAccountaction is created but never dispatched.The
ApproveAccountaction is instantiated but not passed toonAction(), so clicking "Approve Account" does nothing. Compare withViewAccountat line 243 which correctly callsonAction(...).🐛 Proposed fix
is Actions.ApproveAccount -> { - RecurringDepositAccountAction.ApproveAccount( + onAction( + RecurringDepositAccountAction.ApproveAccount( - recurringDeposit.accountNo ?: "", + recurringDeposit.accountNo ?: "", + ), ) }
🧹 Nitpick comments (3)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/recurringDepositAccount/RecurringDepositAccountScreen.kt (1)
66-74: Parameter ordering violates Compose conventions.The
createAccountparameter is required (no default value) but is placed aftermodifierwhich has a default. Compose API guidelines recommend placing required parameters before optional ones, withmodifiertypically being the last optional parameter.♻️ Suggested fix
`@Composable` fun RecurringDepositAccountScreen( navController: NavController, navigateBack: () -> Unit, onApproveAccount: (String) -> Unit, onViewAccount: (String) -> Unit, + createAccount: (Int) -> Unit, modifier: Modifier = Modifier, - createAccount: (Int) -> Unit, viewModel: RecurringDepositAccountViewModel = koinViewModel(), ) {feature/client/src/commonMain/kotlin/com/mifos/feature/client/fixedDepositAccount/FixedDepositAccountScreen.kt (2)
66-74: Consider reordering parameters for better API consistency.The
createAccountparameter (required, no default) is placed aftermodifier(optional with default). Kotlin/Compose conventions typically place required parameters before optional ones. Consider movingcreateAccountbeforemodifierfor consistency.♻️ Suggested reordering
`@Composable` fun FixedDepositAccountScreen( navController: NavController, navigateBack: () -> Unit, onApproveAccount: (String) -> Unit, onViewAccount: (String) -> Unit, + createAccount: (Int) -> Unit, modifier: Modifier = Modifier, - createAccount: (Int) -> Unit, viewModel: FixedDepositAccountViewModel = koinViewModel(), )
273-281: Consider consistent parameter ordering in header function.Required parameters (
onToggleFilter,addAccount,onToggleSearch,isFixedDepositScreenEmpty) are interleaved around the optionalmodifierparameter. For consistency, group all required parameters before optional ones.♻️ Suggested reordering
`@Composable` fun FixedDepositAccountHeader( totalItem: String, onToggleFilter: () -> Unit, + addAccount: () -> Unit, onToggleSearch: () -> Unit, + isFixedDepositScreenEmpty: Boolean, modifier: Modifier = Modifier, - addAccount: () -> Unit, - onToggleSearch: () -> Unit, - isFixedDepositScreenEmpty: Boolean, )
Fixes - Jira-#629
Screen_recording_20260127_184125.webm
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
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.