-
Notifications
You must be signed in to change notification settings - Fork 109
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
Fix double back navigation in roles and permissions screen #2506
Fix double back navigation in roles and permissions screen #2506
Conversation
This happened after saving changes and pressing the back navigation button in the top app bar.
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
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.
LGTM, I propose an alternative fix, but your proposal is OK!
The screenshot test are not passing, did not check yet what is failing. EDIT: I guess updating the branch should fix it. I do it.
@@ -189,7 +188,7 @@ fun ChangeRolesView( | |||
) | |||
} | |||
is AsyncAction.Success -> { | |||
SideEffect { updatedOnBackPressed() } | |||
LaunchedEffect(action) { updatedOnBackPressed() } |
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.
Another possible fix is to use AsyncActionView:
AsyncActionView(
async = state.exitState,
onSuccess = { updatedOnBackPressed() },
confirmationDialog = {
ConfirmationDialog(
title = stringResource(CommonStrings.dialog_unsaved_changes_title),
content = stringResource(CommonStrings.dialog_unsaved_changes_description_android),
onSubmitClicked = { state.eventSink(ChangeRolesEvent.Exit) },
onDismiss = { state.eventSink(ChangeRolesEvent.CancelExit) }
)
},
onErrorDismiss = { /* Cannot happen */ },
)
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.
Oh true, it's the perfect place to use it. Thanks!
Done in 3037b77
…ng-roles-and-permission-changes
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2506 +/- ##
===========================================
- Coverage 72.97% 72.97% -0.01%
===========================================
Files 1394 1394
Lines 33511 33512 +1
Branches 6493 6492 -1
===========================================
Hits 24455 24455
- Misses 5671 5672 +1
Partials 3385 3385 ☔ View full report in Codecov by Sentry. |
Quality Gate passedIssues Measures |
Type of change
Content
Change use of
SideEffect
forLaunchedEffect
, which prevents the action from being called twice when recomposing.Motivation and context
After saving changes in the 'edit admins' or 'edit moderators' screens, tapping the back navigation arrow went back twice.
Tests
Tested devices
Checklist