Skip to content
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

Center Module clean up #2226

Merged
merged 16 commits into from
Nov 13, 2024

Conversation

Darkeye14
Copy link
Contributor

Fixes #Issue_Number

The required changes have been made.
The commits have been squashed.

Please Add Screenshots If there are any UI changes.

WhatsApp.Video.2024-11-08.at.13.48.55_a6d4853c.mp4

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Apply the MifosStyle.xml style template to your code in Android Studio.

  • Run the unit tests with ./gradlew check to make sure you didn't break anything

  • If you have multiple commits please combine them into one commit by squashing them.

@Darkeye14
Copy link
Contributor Author

The conflict can be ignored as in a PR that got merged previously, a section of the code was commented out, that fragment has been removed completely here without any effect to the working.

…nto Feature-Center-Module-CleanUp

# Conflicts:
#	feature/center/src/main/java/com/mifos/feature/center/navigation/CenterNavigation.kt
#	feature/center/src/main/java/com/mifos/feature/center/sync_centers_dialog/SyncCentersDialogScreen.kt
@therajanmaurya
Copy link
Member

Fix conflicts

@@ -149,8 +153,9 @@ fun CenterListScreen(
actions = {
FilledTonalButton(
onClick = {
sync.value = true
syncClicked(selectedItems.toList())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are not using it anymore. Remove it.
line no 114 as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, ill make those necessary changes

syncClicked(selectedItems.toList())
resetSelectionMode()
// resetSelectionMode()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment

@@ -121,6 +122,9 @@ fun CenterListScreen(
isInSelectionMode = false
selectedItems.clear()
}
val sync = remember {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use rememberSavable instead!?

Comment on lines 17 to 19
data object SyncCenterPayloadsScreen : CenterScreens("sync_center_payloads_screen"){
fun arguments(centers :List<Center>) = "sync_center_payloads_screen/${centers}"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did we add it? we are not using it right...

@@ -174,14 +184,14 @@ fun SyncGroupDialogContent(
Row(
modifier = Modifier.fillMaxWidth()
) {
if (uiData.isSyncSuccess) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we deleting this logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Cancel and hide button doesnt show and hence its implementation goes for a waste.
Screenshot 2024-11-09 193339
Do you want me to add it back @itsPronay

Copy link
Contributor

@itsPronay itsPronay Nov 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hide and cancel should show when uiData.isSyncSuccess is false. If its not showing when syncing, the problem must be somewhere else. Note it down so that we can fix it later.

Copy link
Contributor

@itsPronay itsPronay Nov 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and keep the logic as it was

@Darkeye14
Copy link
Contributor Author

Current logic:
https://github.com/user-attachments/assets/56539c06-b80f-4993-93c5-cbffbe7630cd

Previous Working:

WhatsApp.Video.2024-11-09.at.20.06.45_489187ba.mp4

Other changes

Center sync issues resolved
…nto Feature-Center-Module-CleanUp

# Conflicts:
#	feature/center/src/main/java/com/mifos/feature/center/centerList/ui/CenterListScreen.kt
#	feature/center/src/main/java/com/mifos/feature/center/navigation/CenterNavigation.kt
#	feature/center/src/main/java/com/mifos/feature/center/navigation/CenterScreens.kt
#	feature/center/src/main/java/com/mifos/feature/center/syncCentersDialog/SyncCentersDialogScreen.kt
@Darkeye14
Copy link
Contributor Author

Spotless has been Applied
Screenshot 2024-11-10 103541

@Darkeye14
Copy link
Contributor Author

@therajanmaurya ji, the conflicts have been resolved

@therajanmaurya therajanmaurya merged commit 2aa1103 into openMF:master Nov 13, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants