Collection Sheet Enhancement #2406
Conversation
# Conflicts: # cmp-android/dependencies/demoDebugRuntimeClasspath.txt # cmp-android/dependencies/prodDebugRuntimeClasspath.txt
# Conflicts: # feature/collectionSheet/src/commonMain/kotlin/com/mifos/feature/individualCollectionSheet/paymentDetails/PaymentDetailsScreen.kt
|
@Aditya3815 can you replace if Total Charges , and other fields if null show zero on screen as showing null on screen to user is not appropriate |
| @@ -25,11 +26,16 @@ kotlin { | |||
| implementation(compose.ui) | |||
| implementation(projects.core.domain) | |||
| implementation(libs.kotlinx.serialization.json) | |||
| implementation(compose.components.uiToolingPreview) | |||
There was a problem hiding this comment.
Remove this as u already defined in dependencies {} block
There was a problem hiding this comment.
i mentioned remove it from commonMain but keep the dependencies{}:
- cause when we create a project from
KMP wizardthis way it is done by default maybe they have sth in mind where in future the previews will work for all platform if we have it here
dependencies {
debugImplementation(compose.uiTooling)
}
| @@ -210,6 +210,8 @@ private fun IndividualCollectionSheetItem( | |||
| modifier: Modifier = Modifier, | |||
| onClick: () -> Unit, | |||
| ) { | |||
| val loan = client.loans?.getOrNull(index) | |||
There was a problem hiding this comment.
i see that this index is not use anywhere except finding the loan. so instead of passing index why not passing the loan itself from place this composable is called
| state.officeList[index].id.let { | ||
| getStaffList(it) | ||
| officeId = it | ||
| println("DEBUG: Office selection - Index: $index, List size: ${state.officeList.size}, Value: $value") |
There was a problem hiding this comment.
remove all of these println
| ) | ||
| Spacer(modifier = Modifier.width(16.dp)) | ||
| Text( | ||
| text = client.loans?.get(index)?.totalDue.toString(), | ||
| text = (loan?.totalDue ?: 0).toString(), |
There was a problem hiding this comment.
- the zero should be in this format
0.0 - i see this screen there are things that need to be taken care of at viewmodel and the screen should only display to user, but no worries for now we should come back for all of these
| state.staffList[index].id?.let { | ||
| staffId = it | ||
| try { | ||
| if (index >= 0 && index < state.staffList.size) { |
There was a problem hiding this comment.
having these codes like try/cathc is highly discouraged in UI. use build it extension functions: sth like below change accordingly
state.officeList.getOrNull(index)?.let { selectedOfficeEntity ->
selectedOfficeEntity.id?.let {
getStaffList(it)
officeId = it
}
selectedOffice = selectedOfficeEntity.name.toString()
}
There was a problem hiding this comment.
@HekmatullahAmin as you requested I have done changes can you do review now
There was a problem hiding this comment.
After that I will run the spotlessApply
Removed duplicate depndencies from build.gradle.kts change the Int to Double type 0 -> 0.0
| onClick = { | ||
| sheet.paymentTypeOptions?.let { paymentTypeOptions -> | ||
| submit( | ||
| index, | ||
| 0, |
There was a problem hiding this comment.
- By removing index i meant to remove it from composable and just pass the loan as a parameter instead of going to composable and finding again using the index. where the index is not used in that composable at any line.
- but you removed the index from submit and other areas an hardcoded it.
- when i review the code i compare it to the previous code cause i am not fully aware of each variable and what specefic screen do. so keep the things as before just inside the
IndividualCollectionSheetIteminstead of passing index pass the loan using maybeclient.loans?.get(index). - so have
itemsIndexedas before
| @@ -178,17 +180,17 @@ internal fun IndividualCollectionSheetDetailsScreen( | |||
| } else { | |||
| LazyColumn { | |||
There was a problem hiding this comment.
whenever u use LazyColumn use key for better performance
| @@ -25,11 +26,16 @@ kotlin { | |||
| implementation(compose.ui) | |||
| implementation(projects.core.domain) | |||
| implementation(libs.kotlinx.serialization.json) | |||
| implementation(compose.components.uiToolingPreview) | |||
There was a problem hiding this comment.
i mentioned remove it from commonMain but keep the dependencies{}:
- cause when we create a project from
KMP wizardthis way it is done by default maybe they have sth in mind where in future the previews will work for all platform if we have it here
dependencies {
debugImplementation(compose.uiTooling)
}
|
@Aditya3815 are you working on this pr, if not can you close this |
Screen_recording_20250625_225800.mp4
Fixes - Jira-#MIFOSAC-453
Didn't create a Jira ticket, click here to create new.
Please Add Screenshots If there are any UI 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.