-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Prevent double Discard Changes dialog on Android back swipe gesture #17907
base: main
Are you sure you want to change the base?
Conversation
First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible. |
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.
Heya! Thanks
Check if it can be further simplified
isCancellable: Boolean = true, | ||
negativeMethod: () -> Unit = {}, | ||
positiveMethod: () -> Unit, | ||
) = AlertDialog.Builder(context).show { | ||
setCancelable(isCancellable) | ||
Timber.i("showing 'discard changes' dialog") | ||
message(text = message) | ||
positiveButton(text = positiveButtonText) { positiveMethod() } | ||
negativeButton(text = negativeButtonText) | ||
negativeButton(text = negativeButtonText) { negativeMethod() } |
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.
You can do the same without changing the original code here, I think that the Discard dialog should be kept simple
DiscardChangesDialog.showDialog(this, isCancellable = false, negativeMethod = { | ||
isDialogShowing = false | ||
}) { | ||
isDialogShowing = false |
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.
Can use dismiss listener
Purpose / Description
Instant Card: Double "discard changes" window appears on Android "Back" swipe gesture #17864
Fixes
Approach
To handle the issue effectively, we introduce a flag (
isDialogShowing
) to monitor the visibility of the AlertDialog.By default, the AlertDialog gets dismissed when clicked outside, but the flag (
isDialogShowing
) remainstrue
. This prevents the dialog from appearing again when needed.To address this, we modify the
showDialog
function in theDiscardChangesDialog.kt
file to accept anisCancellable
parameter. SettingsetCancelable(false)
ensures that the dialog is not dismissed when clicking outside, allowing us to manage the flag correctly.This update changes the default behavior of this particular AlertDialog, preventing it from being dismissed when clicking outside the dialog. Seeking guidance on whether this change is acceptable.
This approach ensures that
isDialogShowing
is updated properly when the dialog is dismissed, preventing unexpected behavior.How Has This Been Tested?
Tested
on Android device and attached screen recording.Checklist
Please, go through these checks before submitting the PR.
Uploading screen-rec.mp4…